generated from john/python-template
Review log [8]. classify_unexpected_error already returned retriable=False and the verdict was logged and then thrown away. Measured across src/: retriable was assigned in 9 places and read in none. The plan asks for a test that a programming error "does not silently retry". Probing with an injected AttributeError showed that is not what happens, and the two real failure modes need different fixes. Mode A, raised after the claim commits (inside advance_job): raised exactly once, job left at PROCESSING, retry_count 0, never re-claimed, because claim_next_queued_job filters status == QUEUED. A permanently stranded job with one swallowed log line, not a retry. advance_job's PROCESSING branch, commented "Recover mid-flight jobs", is unreachable from the worker for the same reason. Mode B, raised before or during the claim: 20 raises in 1.2s, an unbounded hot spin at the poll interval. It never reaches the per-job retry machinery, so WORKER_MAX_RETRIES does not cap it and the plan's 60s worst case understates this path. services/workflows.py _advance_job_with_containment wraps advance_job. Any escaping exception is classified and the job driven to terminal FAILED, which is visible in the UI and resubmittable. The caller session is rolled back first and the terminal write runs in its own transaction, so it stays atomic even when the failure left that session dirty (plan task 3). The loop continues, so one poison job cannot halt transcription for every other job. worker.py handle_worker_exceptions re-raises non-retriable faults rather than suppressing them; retriable ones are still suppressed so transient conditions do not stop work. run_worker_loop catches that, logs CRITICAL and returns cleanly. Returning rather than propagating matters: the exception would otherwise surface only at app shutdown, through the wait_for in worker_consumer_lifespan. tests test_run_worker_loop_survives_process_next_exception asserted the loop SURVIVES a RuntimeError and continues, which is the Mode B defect written down as an expectation. Replaced by test_run_worker_loop_stops_on_non_retriable_exception, with a new test_run_worker_loop_survives_retriable_exception so suppression of genuinely transient faults stays covered, and test_error_after_claim_fails_the_job_instead_of_stranding_it for Mode A. All three were verified to fail on pre-fix code. The Mode B guard fails by timing out, which is the infinite spin made visible. Verified: 295 passed, 4 skipped, 0 ruff, 0 ty. Co-authored-by: Copilot App <[email protected]>
154 lines
5.0 KiB
Python
154 lines
5.0 KiB
Python
import asyncio
|
|
import logging
|
|
from typing import cast
|
|
|
|
import pytest
|
|
|
|
from transcription.errors import AppError
|
|
from transcription.errors import ErrorCategory
|
|
from transcription.services import ServiceBundle
|
|
from transcription.services.sources import SourceService
|
|
from transcription.worker import process_next_queued_job
|
|
from transcription.worker import run_worker_loop
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_run_worker_loop_stops_on_non_retriable_exception(monkeypatch, caplog):
|
|
"""A programming error before a job is claimed stops the loop instead of spinning.
|
|
|
|
Regression guard for review log [8]. This previously spun at the poll interval
|
|
forever: the fault was classified non-retriable, logged, and then discarded, and
|
|
it never reached the per-job retry machinery so nothing capped it. Measured at 20
|
|
iterations in 1.2s before the fix.
|
|
"""
|
|
calls = 0
|
|
stop_event = asyncio.Event()
|
|
|
|
async def _fake_process_next_queued_job(*, session=None, session_factory=None, services=None):
|
|
nonlocal calls
|
|
_ = (session, session_factory, services)
|
|
calls += 1
|
|
raise RuntimeError("boom")
|
|
|
|
monkeypatch.setattr("transcription.worker.process_next_queued_job", _fake_process_next_queued_job)
|
|
|
|
with caplog.at_level(logging.CRITICAL):
|
|
await asyncio.wait_for(
|
|
run_worker_loop(stop_event=stop_event, poll_interval_seconds=0),
|
|
timeout=5,
|
|
)
|
|
|
|
assert calls == 1
|
|
assert "Worker loop stopped after a non-retriable error" in caplog.text
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_run_worker_loop_survives_retriable_exception(monkeypatch, caplog):
|
|
"""A retriable fault is still suppressed so transient conditions do not stop work."""
|
|
calls = 0
|
|
stop_event = asyncio.Event()
|
|
|
|
async def _fake_process_next_queued_job(*, session=None, session_factory=None, services=None):
|
|
nonlocal calls
|
|
_ = (session, session_factory, services)
|
|
calls += 1
|
|
if calls == 1:
|
|
raise AppError(
|
|
"transient",
|
|
category=ErrorCategory.EXTERNAL_PROVIDER,
|
|
suggestion="retry",
|
|
retriable=True,
|
|
)
|
|
stop_event.set()
|
|
return False
|
|
|
|
monkeypatch.setattr("transcription.worker.process_next_queued_job", _fake_process_next_queued_job)
|
|
|
|
with caplog.at_level(logging.ERROR):
|
|
await run_worker_loop(stop_event=stop_event, poll_interval_seconds=0)
|
|
|
|
assert calls == 2
|
|
assert "Worker loop exception" in caplog.text
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_run_worker_loop_reuses_one_bundle_across_jobs(monkeypatch):
|
|
"""HIGH-02: the provider client is built once per loop, not once per job."""
|
|
stop_event = asyncio.Event()
|
|
seen: list[object] = []
|
|
closed = False
|
|
|
|
class _Sources:
|
|
async def aclose(self):
|
|
nonlocal closed
|
|
closed = True
|
|
|
|
bundle = ServiceBundle(sources=cast("SourceService", _Sources()))
|
|
monkeypatch.setattr(
|
|
"transcription.worker.ServiceBundle.from_session_factory",
|
|
classmethod(lambda _cls, _factory=None, **_kwargs: bundle),
|
|
)
|
|
|
|
async def _fake_process_next_queued_job(*, session=None, session_factory=None, services=None):
|
|
_ = (session, session_factory)
|
|
seen.append(services)
|
|
if len(seen) >= 3:
|
|
stop_event.set()
|
|
return False
|
|
return True
|
|
|
|
monkeypatch.setattr("transcription.worker.process_next_queued_job", _fake_process_next_queued_job)
|
|
|
|
await run_worker_loop(stop_event=stop_event, poll_interval_seconds=0)
|
|
|
|
assert len(seen) == 3
|
|
assert all(item is bundle for item in seen)
|
|
assert closed is True
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_process_next_closes_provider_for_the_bundle_it_owns(monkeypatch):
|
|
closed = False
|
|
|
|
class _Sources:
|
|
async def aclose(self):
|
|
nonlocal closed
|
|
closed = True
|
|
|
|
bundle = ServiceBundle(sources=cast("SourceService", _Sources()))
|
|
monkeypatch.setattr(
|
|
"transcription.worker.ServiceBundle.from_session_factory",
|
|
classmethod(lambda _cls, _factory=None, **_kwargs: bundle),
|
|
)
|
|
|
|
async def _no_job(*, services, session):
|
|
_ = (services, session)
|
|
return False
|
|
|
|
monkeypatch.setattr("transcription.worker.process_next_queued_job_workflow", _no_job)
|
|
|
|
assert await process_next_queued_job() is False
|
|
assert closed is True
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_process_next_leaves_a_caller_owned_bundle_open(monkeypatch):
|
|
"""A bundle passed in belongs to the caller and must outlive one job."""
|
|
closed = False
|
|
|
|
class _Sources:
|
|
async def aclose(self):
|
|
nonlocal closed
|
|
closed = True
|
|
|
|
bundle = ServiceBundle(sources=cast("SourceService", _Sources()))
|
|
|
|
async def _no_job(*, services, session):
|
|
_ = (services, session)
|
|
return False
|
|
|
|
monkeypatch.setattr("transcription.worker.process_next_queued_job_workflow", _no_job)
|
|
|
|
assert await process_next_queued_job(services=bundle) is False
|
|
assert closed is False
|