fix(cartesia): surface rejected /tts/bytes responses as APIStatusError - #2443
Open
brodie-avoca wants to merge 4 commits into
Open
fix(cartesia): surface rejected /tts/bytes responses as APIStatusError#2443brodie-avoca wants to merge 4 commits into
brodie-avoca wants to merge 4 commits into
Conversation
8 tasks
🦋 Changeset detectedLatest commit: b293f4e The changes in this PR will be included in the next version bump. This PR includes changesets to release 38 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
ChunkedStream.run()never looks atres.statusCode. On a rejected response the error body is written into theAudioByteStream, is too short to fill a frame, and the stream completes successfully with zero audio. The caller only sees "no audio was received", never the reason, and a 401 or 400 is indistinguishable from an empty result.Supersedes #1660, which stalled on two review points: the test needed a self-signed certificate, and (per the Devin review) every status was wrapped in
APIConnectionError, so a 401 would be retried. This PR rejects withAPIStatusErrorso retryability follows the status code, and tests against a plain HTTP server (protocol selection landed in #2442).Parity: the Python plugin calls
resp.raise_for_status()on/tts/bytesand maps the failure toAPIStatusError(tts.py#L380, tts.py#L395-L398).Changes Made
APIStatusError. The message carries only the status; Cartesia's response text goes inerror.bodyper REVIEW.md, so it can be redacted from logs. 5xx and 429 stay retryable; other 4xx do not.tts.ChunkedStream(base class) now awaits its background task the same waySynthesizeStreamalready does, so a terminal failure no longer surfaces as an unhandled promise rejection. The failure is already reported through the TTSerrorevent. This removes the need for theunhandledRejectionsuppression in the Cartesia test.closewhen no response was received, so a late request close cannot beat the response handler's rejection.APIErrorinstances as-is in the catch instead of re-wrapping them as retryable connection errors, matching the websocket path.APIStatusErrorwith status 400, makes exactly one request under default connect options, and yields no audio; a 503 followed by 200 retries and yields audio.Pre-Review Checklist
Testing
restaurant_agent.ts/realtime_agent.ts: n/a, not a major changeAdditional Notes
Neuphonic and Resemble build their non-streaming request the same way and also never check the status. Left out of this PR to keep it to Cartesia; happy to follow up.
Co-authored-by: Jason Lernerman jason.lernerman@livekit.io