fix: report actual HTTP status codes in connectWs instead of generic connection error#1075
fix: report actual HTTP status codes in connectWs instead of generic connection error#1075codicerecta wants to merge 2 commits intolivekit:mainfrom
Conversation
…connection error The ws library doesn't set err.code to the HTTP status code on failed WebSocket upgrades — it only includes the status in the error message string. This meant the existing err.code === 429 check never matched, and all HTTP errors (rate limits, auth failures, server errors) were misreported as a generic APIConnectionError with "Error connecting to LiveKit WebSocket". Fix: register an 'unexpected-response' handler on the WebSocket, which receives the actual http.IncomingMessage with res.statusCode before the generic error event fires. HTTP errors now reject with APIStatusError carrying the real status code. The onError handler is simplified to only handle network errors (DNS, TLS, ECONNREFUSED) and preserves the original error message instead of discarding it. Also narrows the import from '../index.js' to '../_exceptions.js' and adds tests covering 429, 401, 500, network errors, and timeouts.
🦋 Changeset detectedLatest commit: 14bdd85 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 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 |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
LG! Can you fix the linting error? |
|
cc @codicerecta |
|
@toubatbrian Checking on the protocol here - are you looking for me to fix the linting error? |
|
@codicerecta Yes, please run the |
Description
The
connectWsfunction misreports all HTTP errors (rate limits, auth failures, server errors) as a genericAPIConnectionErrorwith the message "Error connecting to LiveKit WebSocket". This makes it impossible to diagnose issues like 429 rate limiting, 401 authfailures, or 500 server errors.
Root cause: The
wslibrary doesn't seterr.codeto the HTTP status code on failed WebSocket upgrades — it only includes the status in the error message string (e.g., "Unexpected server response: 429"). The existingerr.code === 429check never matched.Fix: Register a
wsunexpected-responseevent handler that receives the actualhttp.IncomingMessagewithres.statusCodebefore the generic error event fires. This is the documentedwsmechanism for handling non-101 HTTP responses.Changes Made
unexpected-responsehandler that rejects withAPIStatusErrorcarrying the real HTTP status coderetryable: true(overriding theAPIStatusErrordefault ofretryable: falsefor 4xx)APIStatusErrorand the actual status codeonErrorto only handle real network errors (DNS, TLS, ECONNREFUSED) and preserve the original error message instead of discarding it../index.jsto../_exceptions.jsPre-Review Checklist
Testing
restaurant_agent.tsandrealtime_agent.tswork properly (for major changes)New test file
agents/src/inference/utils.test.tscovers:APIStatusErrorwithstatusCode: 429,retryable: trueAPIStatusErrorwithstatusCode: 401,retryable: falseAPIStatusErrorwithstatusCode: 500,retryable: trueAPIConnectionErrorwith original message preservedAPIConnectionErrorwith timeout messageTests use real HTTP servers bound to
127.0.0.1:0rather than mocking, exercising actualwslibrary behavior.Additional Notes
Before this fix, a 429 rate limit from the LiveKit inference gateway produced this unhelpful error:
APIConnectionError: Error connecting to LiveKit WebSocket
After this fix:
APIStatusError: LiveKit gateway quota exceeded (statusCode=429, retryable=true)
Note to reviewers: Please ensure the pre-review checklist is completed before starting your review.