From 9686065c0ff379ab443528c034d9babc2a12c8a3 Mon Sep 17 00:00:00 2001 From: smoghe-bw Date: Thu, 20 Aug 2026 11:34:02 -0400 Subject: [PATCH 1/2] fix(signaling): stop auto-reconnecting when the gateway returns 409 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/v1/signaling.test.ts | 18 ++++++++++++++++-- src/v1/signaling.ts | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/v1/signaling.test.ts b/src/v1/signaling.test.ts index 88d03c0..0711941 100644 --- a/src/v1/signaling.test.ts +++ b/src/v1/signaling.test.ts @@ -145,14 +145,28 @@ describe("Signaling websocket event handlers", () => { expect(ws.setAutoReconnect).toHaveBeenCalledWith(false); }); - test("should handle non-403 error without throwing", async () => { + // A 409 means another device holds this endpoint. Retrying cannot succeed + // until that device leaves, and the client is configured with unlimited + // auto-reconnect, so it must be disabled or the SDK storms the gateway. + test("should reject with error and stop reconnecting on 409 error", async () => { + const errorCallback = getWsCallback("error"); + expect(errorCallback).toBeDefined(); + + const ws = (signaling as any).ws; + errorCallback({ message: "Unexpected server response: 409" }); + + expect(ws.close).toHaveBeenCalledWith(409); + expect(ws.setAutoReconnect).toHaveBeenCalledWith(false); + }); + + test("should handle non-fatal error without throwing", async () => { const errorCallback = getWsCallback("error"); expect(errorCallback).toBeDefined(); // Should not throw on a generic error expect(() => errorCallback({ message: "some other error" })).not.toThrow(); - // ws should not be closed on non-403 errors + // ws should not be closed on errors we can recover from by reconnecting const ws = (signaling as any).ws; expect(ws.setAutoReconnect).not.toHaveBeenCalled(); }); diff --git a/src/v1/signaling.ts b/src/v1/signaling.ts index 4c2f34c..2df5b7d 100644 --- a/src/v1/signaling.ts +++ b/src/v1/signaling.ts @@ -7,6 +7,31 @@ import { EndpointType, HangupResult, OutboundConnectionResult, RtcAuthParams, Rt import { PublishSdpAnswer, PublishMetadata, ReadyMetadata, SetMediaPreferencesWebRtcResponse, SdpAnswer } from "./types"; import { Diagnostics, DiagnosticsBatcher } from "./diagnostics"; +/** + * Handshake failures that will keep recurring for as long as the underlying + * condition holds, keyed by the `ws` error message for a non-101 response. + * + * The client is built with unlimited auto-reconnect, so without this the SDK + * retries these forever — several times a second against a gateway that is + * telling it, correctly, that the connection cannot be established. Reconnect + * is disabled and the error surfaced instead, leaving retry policy to the app. + * + * Note this only works under Node: browsers do not expose the HTTP status of a + * failed websocket upgrade to the error handler. + */ +const FATAL_HANDSHAKE_ERRORS: Record = { + "Unexpected server response: 403": { + status: 403, + logMessage: "Authentication error: Invalid token", + error: "Invalid token", + }, + "Unexpected server response: 409": { + status: 409, + logMessage: "Endpoint already has an active connection from a different device", + error: "Endpoint already has an active connection", + }, +}; + class Signaling extends EventEmitter { private defaultWebsocketUrl: string = "wss://gateway.pv.prod.global.aws.bandwidth.com/prod/gateway-service/api/v1/endpoints"; private ws: JsonRpcClient | null = null; @@ -86,13 +111,15 @@ class Signaling extends EventEmitter { }); ws.on("error", (error: ErrorEvent) => { - if (error.message === "Unexpected server response: 403") { - logger.error("Authentication error: Invalid token"); - ws.close(403); + const fatal = FATAL_HANDSHAKE_ERRORS[error.message]; + if (fatal) { + logger.error(fatal.logMessage); + ws.close(fatal.status); ws.setAutoReconnect(false); - reject(new Error("Invalid token")); + reject(new Error(fatal.error)); // Disconnect without calling leave since we are not connected this._disconnect(false); + return; } // TODO: make this a more informative error message logger.error(`Websocket error: ${error.message}`); From d62d87a75e8f31af51e0072d82a7b5b1eb99ef09 Mon Sep 17 00:00:00 2001 From: smoghe-bw Date: Thu, 20 Aug 2026 13:48:42 -0400 Subject: [PATCH 2/2] fix(signaling): tear down prior client before reconnecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/v1/signaling.test.ts | 13 +++++++++++++ src/v1/signaling.ts | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/v1/signaling.test.ts b/src/v1/signaling.test.ts index 0711941..a993a41 100644 --- a/src/v1/signaling.test.ts +++ b/src/v1/signaling.test.ts @@ -109,6 +109,19 @@ describe("Signaling connect method", () => { expect(emitSpy).toHaveBeenCalledWith("established", testEvent); } }); + + test("should tear down a prior client before connecting again", async () => { + await signaling.connect({ endpointToken: "test-token" }); + const firstWs = (signaling as any).ws; + + await signaling.connect({ endpointToken: "test-token" }); + const secondWs = (signaling as any).ws; + + expect(firstWs.setAutoReconnect).toHaveBeenCalledWith(false); + expect(firstWs.removeAllListeners).toHaveBeenCalled(); + expect(firstWs.close).toHaveBeenCalled(); + expect(secondWs).not.toBe(firstWs); + }); }); describe("Signaling websocket event handlers", () => { diff --git a/src/v1/signaling.ts b/src/v1/signaling.ts index 2df5b7d..cd369bf 100644 --- a/src/v1/signaling.ts +++ b/src/v1/signaling.ts @@ -54,6 +54,16 @@ class Signaling extends EventEmitter { let rpc_id = 1; return new Promise((resolve, reject) => { + if (this.ws) { + // A prior connect() left a client behind. Tear it down before replacing + // this.ws — otherwise the old client's unlimited auto-reconnect keeps + // running in the background forever. Its "open" handler calls + // setMediaPreferences() via this.ws, which by then points at the new + // client, so the orphaned socket never sends anything on its own + // connection and just sits idle until the gateway reaps it. + this._disconnect(false); + } + let rtcOptions: RtcOptions = { websocketUrl: this.defaultWebsocketUrl, };