VAPI-3808 VAPI-3804 fix(signaling): stop auto-reconnecting when the gateway returns 409 - #17
Conversation
The client is constructed with `reconnect: true, max_reconnects: 0`, and the error handler only ever treated 403 as terminal. A 409 — the gateway rejecting the connection because another device already holds the endpoint — was therefore treated as retryable, so the SDK reconnected several times a second against a gateway that was correctly telling it the connection could not be established. Gateway logs for one endpoint show 21 such rejections in 90 seconds across three hosts. Worse, each retry that did get a socket before being rejected left an inert connection behind, which held the endpoint and caused the next round of rejections — the same self-sustaining loop described in the _disconnect comment added by #13. Collect the terminal handshake failures into one table, disable reconnect and surface the error for both, and return so a fatal error no longer falls through into the generic error log. Retry policy is left to the app. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
connect() overwrote this.ws without closing the previous JsonRpcClient, leaving its unlimited auto-reconnect loop running in the background. That orphaned client's "open" handler calls setMediaPreferences() via this.ws, which by then points at the new client, so the orphan's own socket never sends anything and just idles until the gateway reaps it — producing repeated "new websocket connection" / "never called setMediaPreferences" storms against the same endpoint. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Investigated a production incident (gateway logs 08:06–08:08 today) showing repeated Root cause: The orphan's Fix: |
… from 403 (#19) The JS SDK (Bandwidth/javascript-brtc-sdk#17) fixed a reconnect storm caused by unlimited auto-reconnect treating a 409 (another device holds the endpoint) as retryable. The Swift SignalingClient has no auto-reconnect at all, so that storm can't happen here, but it also gave the app no way to tell a 403 (bad token) or 409 (endpoint occupied) apart from any other WebSocket drop — both non-retryable conditions worth surfacing distinctly. Capture the rejected handshake's HTTP status via WebSocketProtocol.response, add BandwidthRTCError.endpointOccupied, and thread the classification through the "close" event to a new BandwidthRTCClient.onDisconnected callback. Co-authored-by: smoghe-bw <smoghe-bw> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Problem
Signaling.connectbuilds its client with unlimited auto-reconnect:but the error handler only ever treated one status as terminal:
So a 409 — the gateway rejecting the connection because another device already holds that endpoint — was treated as retryable. The SDK reconnected several times a second against a gateway that was correctly telling it the connection could not be established.
Gateway logs for a single endpoint show 21 rejections in 90 seconds across three hosts. And it was self-sustaining: each retry that won a socket before being rejected left an inert connection behind — no
openhandler, so it never calledsetMediaPreferences, never created peers, never answered the heartbeat — which held the endpoint and caused the next round of rejections. That's the same failure mode already described in the_disconnectcomment added by #13; this is the other path into it.Change
FATAL_HANDSHAKE_ERRORStable keyed by thewserror message, covering 403 and now 409.leave.return, so a fatal error no longer falls through into the genericWebsocket error: ...log below it (403 was double-logging).Retry policy is deliberately left to the application. A 409 clears when the other device disconnects, and the app knows better than the SDK whether and when re-attempting is appropriate.
Note this only works under Node — browsers don't expose the HTTP status of a failed websocket upgrade to the error handler. Same pre-existing limitation as the 403 branch; called out in a comment.
Test plan
npm test— 72 passednpx tsc --noEmitclean,prettier --checkcleansetAutoReconnect(false), mirroring the existing 403 testsetAutoReconnectis untouched for recoverable errors🤖 Generated with Claude Code