Skip to content

media: trigger noop sendpacket on missed ping #925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
richtrwhereby wants to merge 3 commits into
base: main
Choose a base branch
from
Open

media: trigger noop sendpacket on missed ping #925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
721c855
media: Instead of immediately disconnecting on a missed ping, trigger…
richtrwhereby Feb 20, 2026
8f4a1b3
media: Do not test underlying socket connection on missed ping if it …
richtrwhereby Feb 20, 2026
9174fe8
media: Add changeset
richtrwhereby Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lemon-peas-stick.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@whereby.com/media": patch
---

Do not actively disconnect the socket if a single ping timeout is missed
8 changes: 5 additions & 3 deletions packages/media/src/utils/KeepAliveManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ export class KeepAliveManager {
clearTimeout(this.pingTimer);

this.pingTimer = setTimeout(() => {
// Terminate the underlying websocket to trigger
// websocket reconnection flow
this.serverSocket._socket.io.engine.close();
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");
}, SIGNAL_PING_INTERVAL + SIGNAL_PING_MAX_LATENCY);

this.lastPingTimestamp = Date.now();
Expand Down
40 changes: 28 additions & 12 deletions packages/media/tests/utils/KeepAliveManager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -65,54 +69,66 @@ 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);
});

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", () => {
Expand Down
Loading