From fdcc653b6665f9f95c00c7f71260243b0a915f14 Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Tue, 29 Sep 2026 15:53:10 -0600 Subject: [PATCH] fix(api): keep the worker's error on failed jobs, gate tracebacks behind --debug-api Failed jobs now keep the worker's traceback, error_details and error_code instead of the "Worker exited" wrapper. error_traceback and worker_stderr only reach API clients under --debug-api. --- docs/dev/rest-api/endpoints.md | 2 +- docs/dev/rest-api/job-lifecycle.md | 4 +- tests/remote/unit/test_download_manager.py | 52 ++++++++++++++++++++++ unshackle/core/api/download_manager.py | 10 +++-- unshackle/core/api/handlers.py | 16 +++++-- 5 files changed, 74 insertions(+), 10 deletions(-) diff --git a/docs/dev/rest-api/endpoints.md b/docs/dev/rest-api/endpoints.md index 6f66997f..6806c4c1 100644 --- a/docs/dev/rest-api/endpoints.md +++ b/docs/dev/rest-api/endpoints.md @@ -559,7 +559,7 @@ Show download jobs, with optional filtering and sorting. } ``` -With `full=true`, each job additionally includes `parameters` (with secrets redacted), `started_time`, `completed_time`, `output_files`, `error_message`, `error_details`, `error_code`, `error_traceback`, and `worker_stderr`. +With `full=true`, each job additionally includes `parameters` (with secrets redacted), `started_time`, `completed_time`, `output_files`, `error_message`, `error_details`, and `error_code`. When you start the server with `--debug-api`, each job also includes `error_traceback` and `worker_stderr`. | Status | Error code | Meaning | | --- | --- | --- | diff --git a/docs/dev/rest-api/job-lifecycle.md b/docs/dev/rest-api/job-lifecycle.md index d5a88324..3d6a8913 100644 --- a/docs/dev/rest-api/job-lifecycle.md +++ b/docs/dev/rest-api/job-lifecycle.md @@ -165,8 +165,8 @@ full detail: | `input_prompt` | string or `null` | The prompt the service waits on. Answer it with `POST /api/download/jobs/{job_id}/input`. | The full detail adds `started_time`, `completed_time`, `output_files`, the redacted -`parameters`, and, for failed jobs, `error_message`, `error_details`, `error_code`, -`error_traceback`, and `worker_stderr`. +`parameters`, and, for failed jobs, `error_message`, `error_details`, and `error_code`. +With `--debug-api`, it also adds `error_traceback` and `worker_stderr`. !!! example "A typical polling loop" ```shell diff --git a/tests/remote/unit/test_download_manager.py b/tests/remote/unit/test_download_manager.py index 2fbbb147..fd235865 100644 --- a/tests/remote/unit/test_download_manager.py +++ b/tests/remote/unit/test_download_manager.py @@ -440,3 +440,55 @@ async def test_input_handler_rejects_bad_requests( assert await error_code({"response": "1"}, job.job_id) is APIErrorCode.CONFLICT job.owner_key = "another-key" assert await error_code({"response": "1"}, job.job_id) is APIErrorCode.JOB_NOT_FOUND + + +FAKE_FAILING_WORKER = """ +import sys +from unshackle.core.api import download_worker + + +def fail(*args, **kwargs): + {}[[1]] = 2 + + +download_worker.perform_download = fail +sys.exit(download_worker.main(["worker"] + sys.argv[1:])) +""" + + +async def test_a_failed_job_keeps_the_worker_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, manager: DownloadQueueManager +) -> None: + """The job records the worker's traceback and error, not the parent's "Worker exited" wrapper.""" + import asyncio + + from unshackle.core.api import download_manager + + worker = tmp_path / "worker.py" + worker.write_text(FAKE_FAILING_WORKER, encoding="utf-8") + spawn = asyncio.create_subprocess_exec + + async def spawn_fake_worker(executable: str, *args: str, **kwargs: Any) -> Any: + return await spawn(executable, str(worker), *args[2:], **kwargs) + + monkeypatch.setattr(download_manager.asyncio, "create_subprocess_exec", spawn_fake_worker) + job = manager.create_job("EXAMPLE", "t") + with pytest.raises(Exception, match="Worker exited"): + await asyncio.wait_for(manager.execute_download(job), timeout=60) + + assert job.status is JobStatus.FAILED + assert "{}[[1]] = 2" in (job.error_traceback or "") + assert not (job.error_details or "").startswith("Worker exited") + + +def test_client_job_view_hides_debug_fields_without_debug_api() -> None: + from types import SimpleNamespace + + from unshackle.core.api.handlers import client_job_view + + job = {"job_id": "j", "error_traceback": "tb", "worker_stderr": "err"} + off: Any = SimpleNamespace(app={"debug_api": False}) + on: Any = SimpleNamespace(app={"debug_api": True}) + assert client_job_view(job, off) == {"job_id": "j"} + assert client_job_view(job, None) == {"job_id": "j"} + assert client_job_view(job, on) == job diff --git a/unshackle/core/api/download_manager.py b/unshackle/core/api/download_manager.py index 875ab2c0..03a30b1a 100644 --- a/unshackle/core/api/download_manager.py +++ b/unshackle/core/api/download_manager.py @@ -1005,14 +1005,13 @@ class DownloadQueueManager: job.status = JobStatus.FAILED job.error_message = str(e) - job.error_details = str(e) api_error = categorize_exception( e, context={"service": job.service, "title_id": job.title_id, "job_id": job.job_id} ) - job.error_code = api_error.error_code.value - - job.error_traceback = traceback.format_exc() + job.error_details = job.error_details or str(e) + job.error_code = job.error_code or api_error.error_code.value + job.error_traceback = job.error_traceback or traceback.format_exc() log.error(f"Download failed for job {job.job_id}: {e}") raise @@ -1165,6 +1164,9 @@ class DownloadQueueManager: log.error(f"Failed to parse worker result for job {job.job_id}: {exc}") break + if result_data: + job.error_traceback = result_data.get("traceback") + if returncode != 0: message = result_data.get("message") if result_data else "unknown error" if result_data: diff --git a/unshackle/core/api/handlers.py b/unshackle/core/api/handlers.py index 40a97a75..09bd63fc 100644 --- a/unshackle/core/api/handlers.py +++ b/unshackle/core/api/handlers.py @@ -2141,7 +2141,7 @@ async def list_download_jobs_handler(data: Dict[str, Any], request: Optional[web jobs = sorted(jobs, key=get_sort_key, reverse=reverse) include_full = str(data.get("full") or "").lower() == "true" - job_list = [job.to_dict(include_full_details=include_full) for job in jobs] + job_list = [client_job_view(job.to_dict(include_full_details=include_full), request) for job in jobs] return web.json_response({"jobs": job_list}) @@ -2157,6 +2157,16 @@ async def list_download_jobs_handler(data: Dict[str, Any], request: Optional[web ) +def client_job_view(job: Dict[str, Any], request: Optional[web.Request]) -> Dict[str, Any]: + """Drop a job's traceback and stderr for API clients unless the server runs with ``--debug-api``. + + Redaction masks only the job's own secrets, so these fields can still carry server-side values. + """ + if request is not None and request.app.get("debug_api", False): + return job + return {k: v for k, v in job.items() if k not in ("error_traceback", "worker_stderr")} + + async def get_download_job_handler(job_id: str, request: Optional[web.Request] = None) -> web.Response: """Answer the request for one specific download job.""" from unshackle.core.api.download_manager import get_download_manager @@ -2172,7 +2182,7 @@ async def get_download_job_handler(job_id: str, request: Optional[web.Request] = details={"job_id": job_id}, ) - return web.json_response(job.to_dict(include_full_details=True)) + return web.json_response(client_job_view(job.to_dict(include_full_details=True), request)) except APIError: raise @@ -2214,7 +2224,7 @@ async def download_job_events_handler(job_id: str, request: web.Request) -> web. await response.prepare(request) async def send(event: str, data: Dict[str, Any]) -> None: - payload = json.dumps(data, separators=(",", ":"), default=str) + payload = json.dumps(client_job_view(data, request), separators=(",", ":"), default=str) await response.write(f"event: {event}\ndata: {payload}\n\n".encode()) queue: Optional[asyncio.Queue] = None