mirror of
https://github.com/unshackle-dl/unshackle.git
synced 2026-11-11 19:45:06 +00:00
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.
This commit is contained in:
@@ -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 |
|
||||
| --- | --- | --- |
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user