From 721c85533be8e40f16f9913378d10bdd9a324cc7 Mon Sep 17 00:00:00 2001 From: Richard Tibbett Date: Fri, 20 Feb 2026 11:03:24 +0100 Subject: [PATCH 1/3] media: Instead of immediately disconnecting on a missed ping, trigger the noop sendPacket flow instead to force disconnect if needed --- packages/media/src/utils/KeepAliveManager.ts | 6 ++-- .../tests/utils/KeepAliveManager.spec.ts | 30 +++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/media/src/utils/KeepAliveManager.ts b/packages/media/src/utils/KeepAliveManager.ts index 36a527db2..ce716cbe4 100644 --- a/packages/media/src/utils/KeepAliveManager.ts +++ b/packages/media/src/utils/KeepAliveManager.ts @@ -30,9 +30,9 @@ export class KeepAliveManager { clearTimeout(this.pingTimer); this.pingTimer = setTimeout(() => { - // Terminate the underlying websocket to trigger - // websocket reconnection flow - this.serverSocket._socket.io.engine.close(); + // try sending a noop message if socket still thinks it is connected (might not be) + // If this fails it will trigger the websocket reconnection flow. + this.serverSocket._socket.io.engine.sendPacket("noop"); }, SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY); this.lastPingTimestamp = Date.now(); diff --git a/packages/media/tests/utils/KeepAliveManager.spec.ts b/packages/media/tests/utils/KeepAliveManager.spec.ts index 3e3823749..e811756f4 100644 --- a/packages/media/tests/utils/KeepAliveManager.spec.ts +++ b/packages/media/tests/utils/KeepAliveManager.spec.ts @@ -9,7 +9,11 @@ import { ServerSocket } from "../../src/utils/ServerSocket"; class MockSocket extends EventEmitter {} -const createSocketIoEngine = () => ({ disconnect: jest.fn(), engine: { close: jest.fn() }, opts: { transports: [] } }); +const createSocketIoEngine = () => ({ + disconnect: jest.fn(), + engine: { readyState: "open", sendPacket: jest.fn() }, + opts: { transports: [] }, +}); const createMockSocketIoSocket = () => { const mockSocket = new MockSocket(); @@ -65,54 +69,56 @@ describe("KeepAliveManager", () => { expect(pingTimestamp - initialTimestamp).toEqual(1000); }); - it("should force close underlying socket if zero pings received after connect", () => { + it("should test underlying socket if zero pings received after connect", () => { serverSocket._socket.emit("connect"); - expect(serverSocket._socket.io.engine.close).toHaveBeenCalledTimes(0); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(0); jest.advanceTimersByTime(SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY + 1); - expect(serverSocket._socket.io.engine.close).toHaveBeenCalledTimes(1); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(1); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledWith("noop"); }); - it("should force close underlying socket on any missed ping", () => { + it("should test underlying socket on any missed ping", () => { serverSocket._socket.emit("connect"); jest.advanceTimersByTime(SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY - 1); - expect(serverSocket._socket.io.engine.close).toHaveBeenCalledTimes(0); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(0); serverSocket._socket.io.emit("ping"); jest.advanceTimersByTime(SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY - 1); - expect(serverSocket._socket.io.engine.close).toHaveBeenCalledTimes(0); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(0); serverSocket._socket.io.emit("ping"); jest.advanceTimersByTime(SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY + 1); - expect(serverSocket._socket.io.engine.close).toHaveBeenCalledTimes(1); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(1); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledWith("noop"); }); - it("should not force close underlying socket after socket is disconnected", () => { + it("should not test underlying socket after socket is disconnected", () => { serverSocket._socket.emit("connect"); jest.advanceTimersByTime(SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY - 1); - expect(serverSocket._socket.io.engine.close).toHaveBeenCalledTimes(0); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(0); serverSocket._socket.io.emit("ping"); jest.advanceTimersByTime(SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY - 1); - expect(serverSocket._socket.io.engine.close).toHaveBeenCalledTimes(0); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(0); serverSocket._socket.emit("disconnect"); jest.advanceTimersByTime(SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY + 1); - expect(serverSocket._socket.io.engine.close).toHaveBeenCalledTimes(0); + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(0); }); describe("disconnectDurationLimitExceeded", () => { From 8f4a1b36bc33e89e9ec13c5f32f136eb7b08ff3b Mon Sep 17 00:00:00 2001 From: Richard Tibbett Date: Fri, 20 Feb 2026 11:27:16 +0100 Subject: [PATCH 2/3] media: Do not test underlying socket connection on missed ping if it is already is a closed state --- packages/media/src/utils/KeepAliveManager.ts | 2 ++ packages/media/tests/utils/KeepAliveManager.spec.ts | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/packages/media/src/utils/KeepAliveManager.ts b/packages/media/src/utils/KeepAliveManager.ts index ce716cbe4..dea22db34 100644 --- a/packages/media/src/utils/KeepAliveManager.ts +++ b/packages/media/src/utils/KeepAliveManager.ts @@ -30,6 +30,8 @@ export class KeepAliveManager { clearTimeout(this.pingTimer); this.pingTimer = setTimeout(() => { + if (this.serverSocket._socket.io.engine.readyState === "closed") return; + // try sending a noop message if socket still thinks it is connected (might not be) // If this fails it will trigger the websocket reconnection flow. this.serverSocket._socket.io.engine.sendPacket("noop"); diff --git a/packages/media/tests/utils/KeepAliveManager.spec.ts b/packages/media/tests/utils/KeepAliveManager.spec.ts index e811756f4..703b209f7 100644 --- a/packages/media/tests/utils/KeepAliveManager.spec.ts +++ b/packages/media/tests/utils/KeepAliveManager.spec.ts @@ -121,6 +121,16 @@ describe("KeepAliveManager", () => { expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(0); }); + it("should not test underlying socket if socket is already in a closed state", () => { + serverSocket._socket.io.engine.readyState = "closed"; + + serverSocket._socket.io.emit("ping"); + + jest.advanceTimersByTime(SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY + 1); + + expect(serverSocket._socket.io.engine.sendPacket).toHaveBeenCalledTimes(0); + }); + describe("disconnectDurationLimitExceeded", () => { describe("when enabled", () => { beforeEach(() => { From 9174fe8ff0c60ccc7429a303d5d2c31751830907 Mon Sep 17 00:00:00 2001 From: Richard Tibbett Date: Tue, 24 Mar 2026 09:58:57 +0100 Subject: [PATCH 3/3] media: Add changeset --- .changeset/lemon-peas-stick.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/lemon-peas-stick.md diff --git a/.changeset/lemon-peas-stick.md b/.changeset/lemon-peas-stick.md new file mode 100644 index 000000000..24eb499e6 --- /dev/null +++ b/.changeset/lemon-peas-stick.md @@ -0,0 +1,5 @@ +--- +"@whereby.com/media": patch +--- + +Do not actively disconnect the socket if a single ping timeout is missed