mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-15 12:08:31 +00:00
fix(queue): a late progress update no longer reopens a finished task (#1538)
#1429 made the first terminal status win, but the guard only refused terminal to terminal changes. A worker's poll that reports DOWNLOADING after a cancel still overwrote CANCELLED (the poll loop calls the status callback before it waits on the cancel flag), and the COMPLETE or ERROR that followed was then accepted as a first terminal status, so the terminal hook ran a second time. This refuses any status change away from a terminal status. `add()` and `enqueue_existing()` reset the status themselves, so retries are unaffected. The new test cancels a downloading task, sends DOWNLOADING and then COMPLETE, and expects CANCELLED and one hook call. It fails on main and passes here. Full non-e2e suite passes, ruff and basedpyright are clean. Found in a Claude pass over 1.5.0, same as my other small fixes from it.
This commit is contained in:
1 parent
395c331b4a
commit
e27fed1d40
2 files changed
+22
-6
No files matched your search
@@ -168,14 +168,13 @@ class BookQueue:
|
||||
hook: Callable[[str, QueueStatus, DownloadTask], None] | None = None
|
||||
hook_task: DownloadTask | None = None
|
||||
previous_status = self._status.get(book_id)
|
||||
if (
|
||||
previous_status in TERMINAL_QUEUE_STATUSES
|
||||
and status in TERMINAL_QUEUE_STATUSES
|
||||
and previous_status != status
|
||||
):
|
||||
if previous_status in TERMINAL_QUEUE_STATUSES and previous_status != status:
|
||||
# The first terminal status wins. A worker that checked its cancel flag just before
|
||||
# a cancel landed would otherwise overwrite CANCELLED with COMPLETE and fire the
|
||||
# terminal hook a second time.
|
||||
# terminal hook a second time. A late progress update (DOWNLOADING) is refused for
|
||||
# the same reason: it would reopen the task and let the next terminal status
|
||||
# through. A retry goes through add() or enqueue_existing(), which reset the
|
||||
# status themselves.
|
||||
return None, None
|
||||
self._update_status(book_id, status)
|
||||
|
||||
|
||||
@@ -88,6 +88,23 @@ def test_a_late_complete_does_not_overwrite_a_cancel():
|
||||
assert terminal_events == [QueueStatus.CANCELLED]
|
||||
|
||||
|
||||
def test_a_late_progress_update_does_not_reopen_a_cancelled_task():
|
||||
"""A poll that reports DOWNLOADING after the cancel must not reopen the task.
|
||||
|
||||
Without this the next COMPLETE or ERROR is accepted as a first terminal status and
|
||||
the terminal hook fires a second time.
|
||||
"""
|
||||
queue = BookQueue()
|
||||
terminal_events = _downloading_task(queue, "race-task")
|
||||
|
||||
assert queue.cancel_download("race-task") is True
|
||||
queue.update_status("race-task", QueueStatus.DOWNLOADING)
|
||||
queue.update_status("race-task", QueueStatus.COMPLETE)
|
||||
|
||||
assert queue.get_task_status("race-task") == QueueStatus.CANCELLED
|
||||
assert terminal_events == [QueueStatus.CANCELLED]
|
||||
|
||||
|
||||
def test_a_task_that_ended_in_error_is_not_completed_afterwards():
|
||||
queue = BookQueue()
|
||||
terminal_events = _downloading_task(queue, "race-task")
|
||||
|
||||
Reference in new issue
Block a user