Skip to content

fix(audio): carry CORS on the fetch-failure 500 from speech, transcription and translation - #17938

Merged
diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.53from
AndresDiagoM:fix/audio-fetch-failure-cors
Oct 26, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.53from
AndresDiagoM:fix/audio-fetch-failure-cors

Conversation

@AndresDiagoM

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #17219

Closes #17937. Follow-up suggested by the maintainer in the review of #17844.

Summary

  • Problem. When the upstream fetch itself throws (DNS failure, connection refused, TLS error), the generic catch in audioSpeech.ts, audioTranscription.ts and audioTranslation.ts returned errorResponse(500, ...) without CORS. Every !res.ok path on these routes re-applies CORS_HEADERS since fix(backend): whole-repository audit findings for v3.8.52 — 78 items (14 critical · 21 high · 29 medium · 14 keep) #15159 (fix(audio): sanitize upstream error bodies on /v1/audio/translations #17844 for translations), because errorResponse does not emit them and the routes set CORS only on the OPTIONS preflight.
  • Fix. The "canonical error + CORS" step that upstreamErrorResponse already did inline is now the exported helper audioErrorResponse(status, message) in open-sse/utils/audioResponse.ts. upstreamErrorResponse calls it, and so do the three catch blocks. Messages still go through errorResponse, so sanitization is unchanged.
  • Reachability. The three routes return error responses from the handlers unchanged (attachOmniRouteMetaToResponse only wraps response.ok), so the headers reach the client.

Not changed on purpose

Validation (TDD)

tests/unit/audio-fetch-failure-cors.test.ts, 3 tests (one per handler). Each stubs fetch to throw an error whose message carries a credential and an absolute path, then asserts a 500 with CORS and neither secret in the body.

# base (release/v3.8.53)
✖ translation: ... lost CORS on fetch-failure response
✖ transcription: ... lost CORS on fetch-failure response
✖ speech: ... lost CORS on fetch-failure response
ℹ tests 3  pass 0  fail 3
# with the fix
ℹ tests 3  pass 3  fail 0

All tests/unit/audio-*.test.ts plus api-error-response-sanitization-15159: 162/163. The one failure, audio-transcriptions-combo-resolution "a combo name is expanded to its target...", fails identically on the untouched base (ReferenceError: Cannot access 'updateSettings' before initialization inside the test file).

Also green: typecheck:core, eslint on the touched files, check:file-size, check-complexity-ratchets --base-ref origin/release/v3.8.53, lint-staged / check:ai-attribution hooks.

…ption and translation

When the upstream fetch itself throws (DNS failure, connection reset, TLS
error), the generic catch in the three audio handlers returned
errorResponse(500, ...) without CORS headers. Every upstream-error path
re-applies CORS since cheaper-inference#15159 (and cheaper-inference#17844 for translations), because the
canonical builders do not emit it and these routes set it only on the
OPTIONS preflight.

Extract the "errorResponse + CORS" step of upstreamErrorResponse into a
shared audioErrorResponse helper and use it in the three catch blocks.
The message is still sanitized by errorResponse.
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks, Andres. This is a tidy follow-up. Pulling the "canonical error + CORS" step out of upstreamErrorResponse into an exported audioErrorResponse() is the right shape: the existing path stays byte-for-byte the same and the three catch blocks now share it. Messages still go through errorResponse(), so sanitization is unchanged, and your test proves it by putting a credential and an absolute path into the thrown fetch error.

I checked it on a clean probe of the head (2a32e58). Your new test fails 3/3 when I revert only the three handler files, and passes 3/3 with the fix. The wider audio-* and api-error-response-sanitization-15159 suites pass 163/163 on my side. The changelog fragment is there and the commits are clean.

I also checked it against #17941 (the translation upstream-error change) and #17844. Both touch the !res.ok branch of audioTranslation.ts, while you touch the catch block, so they are complementary and merge cleanly with your PR.

Nothing blocking from my side. Two cosmetic notes, only if you feel like it: the changelog link points at the old repo path, and other fragments just use (#17938). We can also adjust either of these when we land it. Thanks again for the careful TDD evidence in the description.

@AndresDiagoM

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough check, and for confirming it lines up with #17941 and #17844! I took both notes: the fragment now uses the short (#17938) form like the other fragments, which also drops the old repo path (682d872).

@diegosouzapw
diegosouzapw merged commit 033f9ce into cheaper-inference:release/v3.8.53 Oct 26, 2026
42 of 57 checks passed
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.

fix(api): audio: fetch-failure 500 from speech/transcription/translation drops CORS headers

2 participants