fix: report actual HTTP status codes in connectWs instead of generic connection error#1075
Open
codicerecta wants to merge 2 commits intolivekit:mainfrom
Open
fix: report actual HTTP status codes in connectWs instead of generic connection error#1075codicerecta wants to merge 2 commits intolivekit:mainfrom
codicerecta 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 |
Contributor
|
@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". |
toubatbrian
approved these changes
Feb 26, 2026
Contributor
|
LG! Can you fix the linting error? |
Contributor
|
cc @codicerecta |
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
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.