Skip to content

fix: allow slow CPU transcription through the proxy - #452

Open
cswaney wants to merge 3 commits into
mainfrom
fix-transcription-cpu-timeout
Open

fix: allow slow CPU transcription through the proxy#452
cswaney wants to merge 3 commits into
mainfrom
fix-transcription-cpu-timeout

Conversation

@cswaney

@cswaney cswaney commented Aug 12, 2026

Copy link
Copy Markdown
Member

Summary

  • Non-streaming proxied requests inherited the shared client's 30s read timeout, cutting off CPU transcription mid-request. Add a bounded PROXY_TIMEOUT (60s read — generous for the short-clip service UI, still bounded) for the non-streaming proxy path, and convert an httpx.ReadTimeout into a 504 with actionable CPU/GPU guidance instead of a generic 500.
  • Web: surface the 504 as a clear "Transcription timed out" toast (was a generic failure), and preserve the backend's status/detail through callSpeechRecognitionInference.
  • Web: fix a latent race where a cancelled request's late success repopulated the output box — guard the success path with controller.signal.aborted before writing output.

Test plan

  • PROXY_TIMEOUT value locked by a unit test in lib/tests/unit/test_http_client.py.
  • New web/src/routes/speech-recognition/lib/requests.test.js covers the success body, the 504 carrying status + backend detail, and the generic fallback when the error body has no JSON.
  • Manually verified locally on a CPU-only service: a 45s clip returns successfully, and a >60s clip surfaces the timeout toast. Separately, a cancelled request whose fetch resolves successfully after abort() no longer writes its stale transcript to the output box.
  • Backend suite: 760 passed. Frontend suite: 513 passed.

Follow-ups

Heavier tests deferred to backlog issues: #449 (proxy ReadTimeout → 504 route test) and #450 (container stale-response guard test).

Non-streaming proxied requests inherited the shared client's 30s read
timeout, cutting off CPU transcription. Add a bounded PROXY_TIMEOUT (60s)
and surface a read timeout as a 504 with CPU/GPU guidance.
Surface the backend 504 as a clear timeout toast, and guard against a
cancelled request's late success repopulating the output box.
@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review

Solid, well-scoped fix -- good docs on the new timeout constant (mirrors the existing STREAM_TIMEOUT comment style), and the controller.signal.aborted guard closes a real race (closure-captured controller correctly ignores a newer abortRef -- nice touch called out in the comment).

A few things worth a look:

1. Duplicated/drifting 504 copy (minor)
callSpeechRecognitionInference already threads the backend's exact detail through as err.message (that is what the generic-failure else branch reuses). But the 504 branch in SpeechRecognitionContainer.jsx hardcodes its own copy of the guidance text instead of reusing err.message:

  • Backend (asgi.py): "...This can happen when running inference on CPU (no GPU); try a shorter audio clip or a GPU-backed service."
  • Frontend (SpeechRecognitionContainer.jsx): "...This is common on CPU (no GPU) -- try a shorter audio clip or a GPU-backed service."

These already differ slightly and will keep drifting since nothing ties them together. Consider detail: err.message || "The service took too long to respond..." to match the pattern used in the else branch right below it.

2. Reimplements an existing error-parsing helper (code reuse)
web/src/routes/text-generation/lib/requests.js already has a parseErrorResponse(res) helper that does the same thing this PR adds to speech-recognition/lib/requests.js -- parse the JSON body, fall back to a generic message on parse failure, attach .status. The text-generation version also falls back to errorBody.message in addition to .detail. Might be worth lifting this into a shared helper (e.g. web/src/lib/requests.js) both routes import from, so the two do not diverge over time the way the copy in point 1 already has.

3. Core behavior path still untested (acknowledged in PR body)
The unit test only locks PROXY_TIMEOUT.read == 60.0; the actual proxy_service route's httpx.ReadTimeout -> 504 conversion (the main behavior change) has no route-level test yet. Issue #449 is already filed for this -- given it is a new exception-to-status mapping in production code (vs. the harder-to-test frontend timing race in #450), it might be worth bumping this one up in priority.

4. Non-streaming proxy path still does not check upstream status (pre-existing, adjacent)
asyncpost calls response.json() unconditionally with no is_success/status check, unlike the streaming branch just above it which explicitly forwards the upstream status code and error message. So a downstream service error on the non-streaming path (e.g. a malformed audio file) currently comes back to the frontend looking like a success. Not introduced by this PR, but since this PR is specifically about improving proxy error surfacing, closing this gap would make the "Transcription failed" vs "Transcription timed out" distinction more reliable end-to-end.

5. Nit: exception chaining
raise HTTPException(...) inside except httpx.ReadTimeout: does not use raise ... from e. Python still sets __context__ implicitly so this is not a functional issue, just a bit less explicit than from e for anyone reading a future traceback.

Nothing here blocks merging -- points 1 and 2 are the ones worth actually acting on; 3-5 are minor/follow-up material.

Lift parseErrorResponse into the shared lib so speech-recognition and
text-generation don't diverge; reuse the backend's 504 detail in the
timeout toast. Chain the proxy ReadTimeout -> 504 with `from e`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant