Reset file worker status when a synchronous refresh is cancelled (#775)

* Reset file worker status when a synchronous refresh is cancelled

refresh_sync runs inside request handlers.  When Sanic cancelled one on
response timeout, the phase flag cleared as its `with` block unwound but
the shared status did not, so the worker reported 'modeling' forever while
nothing ran.  Reset to idle on cancellation.

Deleting outdated Zims also ran the global modelers inside the request,
which is what timed out.  Skip post-processing there and update the
outdated_zims flag directly, the only cleanup step it relied on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* tests: observe the modeling flag in the cancelled refresh_sync test

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

modules/zim/lib.py

@@ -586,8 +586,10 @@ async def remove_outdated_zim_files(path: pathlib.Path = None) -> int:
         deleted_count += 1
 
     if deleted_count:
-        # Refresh synchronously to clean up DB before returning.
-        await file_worker.refresh_sync(list(outdated))
+        # Refresh synchronously to clean up DB before returning.  Deleting needs no modeling, and the
+        # global modelers would work through the whole unindexed backlog inside this request.
+        await file_worker.refresh_sync(list(outdated), post_processing=False)
+        flag_outdated_zim_files()
 
         # Trigger Kiwix restart so the deleted zim files are no longer served.
         restart_kiwix_handler.activate_switch()

modules/zim/test/test_restart_kiwix.py

@@ -7,6 +7,7 @@ from modules.zim import lib
 from modules.zim.models import Zim
 from wrolpi.api_utils import api_app
 from wrolpi.files.models import FileGroup
+from wrolpi.files.worker import file_worker
 
 
 @pytest.mark.asyncio
@@ -145,3 +146,28 @@ async def test_remove_outdated_zim_files_no_restart_when_nothing_deleted(async_c
     assert deleted_count == 0
     switches = dict(api_app.shared_ctx.switches) if api_app.shared_ctx.switches else {}
     assert 'restart_kiwix' not in switches
+
+
+@pytest.mark.asyncio
+async def test_remove_outdated_zim_files_skips_post_processing(async_client, test_session, test_directory,
+                                                               zim_path_factory):
+    """Deleting outdated Zims must not run the global modelers inside the request.
+
+    `DELETE /api/zim/outdated` awaits this; modeling the whole unindexed backlog there exceeded the
+    response timeout and the cancelled request stranded the file worker status."""
+    zim_dir = test_directory / 'zims'
+    zim_dir.mkdir(exist_ok=True)
+    test_zim_bytes = zim_path_factory().read_bytes()
+    paths = [zim_dir / 'wikipedia_en_all_maxi_2020-01.zim', zim_dir / 'wikipedia_en_all_maxi_2020-02.zim']
+    for path in paths:
+        path.write_bytes(test_zim_bytes)
+    await file_worker.refresh_sync(paths)
+    assert test_session.query(FileGroup).count() == 2
+
+    with mock.patch('wrolpi.files.worker.apply_modelers') as apply_modelers:
+        deleted_count = await lib.remove_outdated_zim_files(zim_dir)
+
+    assert deleted_count == 1
+    apply_modelers.assert_not_called()
+    # The deleted Zim is still removed from the DB.
+    assert test_session.query(FileGroup).count() == 1

wrolpi/files/test/test_worker.py

@@ -7,6 +7,7 @@ from unittest import mock
 import pytest
 from sqlalchemy import text
 
+from wrolpi import flags
 from wrolpi.conftest import await_switches
 from wrolpi.dates import from_timestamp
 from wrolpi.vars import IS_MACOS
@@ -637,6 +638,25 @@ async def test_refresh_sync_records_error_status_on_failure(async_client, test_s
     assert 'boom' in (file_worker.status['error'] or '')
 
 
+@pytest.mark.asyncio
+async def test_refresh_sync_resets_status_when_cancelled(async_client, test_session, test_directory,
+                                                         make_files_structure, flags_lock):
+    """A cancelled synchronous refresh must not strand the status at its last phase.
+
+    `refresh_sync` runs inside request handlers, which Sanic cancels on response timeout.  The
+    phase flags clear when their `with` block unwinds, but the shared status does not, so the
+    status endpoint kept reporting 'modeling' while nothing ran.
+    """
+    paths = make_files_structure(['docs/file1.txt'])
+
+    with mock.patch('wrolpi.files.worker.apply_modelers', side_effect=asyncio.CancelledError):
+        with pytest.raises(asyncio.CancelledError):
+            await file_worker.refresh_sync(paths)
+
+    assert file_worker.status['status'] == 'idle'
+    assert not flags.file_worker_modeling.is_set()
+
+
 @pytest.mark.asyncio
 async def test_process_queue_resets_status_on_handler_error(async_client, test_session, test_directory,
                                                             make_files_structure):

wrolpi/files/worker.py

@@ -346,9 +346,9 @@ class FileWorker(FileMoveMixin, FileReorganizeMixin):
         # `_upsert_file_groups`/`_delete_file_groups`/`_apply_post_processing` all advance the
         # shared status (upserting/deleting/modeling/indexing/cleanup).  Unlike the queued
         # handlers this path has no `process_queue` wrapper to reset it, so reset here or the UI
-        # is stranded at the last phase (e.g. 'cleanup' after deleting files).  Mirror
-        # `process_queue`: reset to idle only on success, leave a terminal 'error' status on
-        # failure (and re-raise so the synchronous caller still sees it).
+        # is stranded at the last phase (e.g. 'cleanup' after deleting files).  Reset to idle on
+        # success or cancellation, leave a terminal 'error' status on failure (and re-raise so the
+        # synchronous caller still sees it).
         try:
             result = await self._refresh_files_directly(paths)
             self._cleanup_modified_models(result.modified)
@@ -361,6 +361,9 @@ class FileWorker(FileMoveMixin, FileReorganizeMixin):
                 # unindexed files and could exceed Sanic's RESPONSE_TIMEOUT.
                 await self._apply_post_processing(is_global_refresh=False)
         except asyncio.CancelledError:
+            # Sanic cancels the request handler on response timeout.  Nothing is running anymore, so
+            # report idle; the unfinished modeling is picked up by the next refresh.
+            self.reset_status()
             raise
         except Exception as e:
             logger.error(f'refresh_sync failed for {len(paths)} paths', exc_info=e)