mirror of
https://github.com/getpaseo/paseo.git
synced 2026-11-08 09:24:41 +00:00
fix(client): close abandoned browser connections immediately (#9067)
* fix(client): close abandoned browser connections immediately When the client gave up on a connection (liveness timeout, connect timeout, reconnect, subscription failure) it closed the WebSocket with 1001. Browsers, Electron and Node's global WebSocket accept only 1000 or 3000-4999 and throw InvalidAccessError otherwise; the throw was swallowed and the reference dropped, so the physical socket stayed open and the daemon kept streaming into it until its 45 s lease expired. The WebSocket adapter is the only code that calls WebSocket.close, so it now sends 1000 for codes the API rejects and trims reasons to the 123-byte limit. Callers keep their reasons, and Node's `ws` sends the same frame as a browser, so Node tests now catch this class of bug. * test(client): drop timing-dependent hello count from abandoned-close test Handshake retries after close are owned and tested by the relay's handshake-lifetime test; this test only checks that the abandoned socket closes.
This commit is contained in:
@@ -1,5 +1,9 @@
|
||||
import { EventEmitter } from "node:events";
|
||||
import { describe, expect, test, vi } from "vitest";
|
||||
import { WebSocketServer, type WebSocket as ServerSocket } from "ws";
|
||||
import { exportPublicKey, generateKeyPair } from "@getpaseo/relay/e2ee";
|
||||
import { DaemonClient } from "./daemon-client.js";
|
||||
import { defaultWebSocketFactory } from "./daemon-client-websocket-transport.js";
|
||||
import {
|
||||
createEncryptedTransport,
|
||||
createWebSocketTransportFactory,
|
||||
@@ -190,3 +194,103 @@ test("native drain capability survives WebSocket and encryption wrappers", async
|
||||
});
|
||||
expect(encrypted.supportsDrain).toBe(false);
|
||||
});
|
||||
|
||||
interface ObservedSocket {
|
||||
closeFrame: { code: number; reason: string } | null;
|
||||
}
|
||||
|
||||
/** A daemon endpoint that accepts sockets and never answers, so every attempt times out. */
|
||||
async function silentDaemon() {
|
||||
const server = new WebSocketServer({ host: "127.0.0.1", port: 0 });
|
||||
await new Promise<void>((resolve) => server.once("listening", () => resolve()));
|
||||
const sockets: ObservedSocket[] = [];
|
||||
server.on("connection", (socket: ServerSocket) => {
|
||||
const observed: ObservedSocket = { closeFrame: null };
|
||||
sockets.push(observed);
|
||||
socket.on("close", (code, reason) => {
|
||||
observed.closeFrame = { code, reason: reason.toString() };
|
||||
});
|
||||
});
|
||||
const address = server.address();
|
||||
if (!address || typeof address === "string") throw new Error("Missing port");
|
||||
return {
|
||||
url: `ws://127.0.0.1:${address.port}/ws`,
|
||||
sockets,
|
||||
openSockets: () => sockets.filter((entry) => entry.closeFrame === null),
|
||||
stop: () =>
|
||||
new Promise<void>((resolve) => {
|
||||
for (const client of server.clients) client.terminate();
|
||||
server.close(() => resolve());
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
// Node's global WebSocket applies the WHATWG close rules Chromium and Electron apply:
|
||||
// close() accepts only 1000 or 3000-4999 and a reason of at most 123 UTF-8 bytes.
|
||||
describe("abandoned connections close their WebSocket", () => {
|
||||
test.each([
|
||||
{ name: "direct", e2ee: false },
|
||||
{ name: "relay E2EE", e2ee: true },
|
||||
])(
|
||||
"$name: a timed-out attempt sends a close frame before the client reconnects",
|
||||
async ({ e2ee }) => {
|
||||
const daemon = await silentDaemon();
|
||||
const client = new DaemonClient({
|
||||
url: daemon.url,
|
||||
clientId: "cid_abandoned_close_test",
|
||||
webSocketFactory: defaultWebSocketFactory,
|
||||
connectTimeoutMs: 1_500,
|
||||
reconnect: { enabled: true, baseDelayMs: 50, maxDelayMs: 50 },
|
||||
logger: { debug() {}, info() {}, warn() {}, error() {} },
|
||||
...(e2ee
|
||||
? {
|
||||
e2ee: {
|
||||
enabled: true,
|
||||
daemonPublicKeyB64: exportPublicKey(generateKeyPair().publicKey),
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
try {
|
||||
void client.connect().catch(() => {});
|
||||
await vi.waitFor(() => expect(daemon.sockets.length).toBeGreaterThanOrEqual(2), {
|
||||
timeout: 4_000,
|
||||
});
|
||||
const [abandoned] = daemon.sockets;
|
||||
await vi.waitFor(() => expect(abandoned?.closeFrame).not.toBeNull(), { timeout: 500 });
|
||||
expect(abandoned?.closeFrame?.reason).toBe("Connection timed out");
|
||||
|
||||
// The newest attempt may still be connecting; aborting it closes without a frame.
|
||||
await client.close();
|
||||
await vi.waitFor(() => expect(daemon.openSockets()).toEqual([]), { timeout: 500 });
|
||||
} finally {
|
||||
await client.close();
|
||||
await daemon.stop();
|
||||
}
|
||||
},
|
||||
10_000,
|
||||
);
|
||||
|
||||
test("a code or reason the WebSocket API rejects still closes the socket", async () => {
|
||||
const daemon = await silentDaemon();
|
||||
try {
|
||||
const transport = createWebSocketTransportFactory(defaultWebSocketFactory)({
|
||||
url: daemon.url,
|
||||
});
|
||||
await new Promise<void>((resolve) => transport.onOpen(() => resolve()));
|
||||
const reason = `Decryption failed: ${"é".repeat(100)}`;
|
||||
|
||||
expect(() => transport.close(1011, reason)).not.toThrow();
|
||||
|
||||
await vi.waitFor(() => expect(daemon.sockets[0]?.closeFrame).not.toBeNull(), {
|
||||
timeout: 500,
|
||||
});
|
||||
const frame = daemon.sockets[0]?.closeFrame;
|
||||
expect(frame?.code).toBe(1000);
|
||||
expect(reason.startsWith(frame?.reason ?? "")).toBe(true);
|
||||
expect(Buffer.byteLength(frame?.reason ?? "")).toBe(123);
|
||||
} finally {
|
||||
await daemon.stop();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -82,7 +82,7 @@ export function createWebSocketTransportFactory(factory: WebSocketFactory): Daem
|
||||
// connect timeouts does not crash the CLI with an unhandled error event.
|
||||
const suppressEarlyCloseError = bindTemporaryEarlyCloseErrorHandler(ws);
|
||||
try {
|
||||
ws.close(code, reason);
|
||||
ws.close(toSendableCloseCode(code), toSendableCloseReason(reason));
|
||||
} finally {
|
||||
if (typeof ws.on !== "function" && typeof ws.addEventListener !== "function") {
|
||||
suppressEarlyCloseError();
|
||||
@@ -97,6 +97,32 @@ export function createWebSocketTransportFactory(factory: WebSocketFactory): Daem
|
||||
};
|
||||
}
|
||||
|
||||
// WHATWG WebSocket.close throws for a code other than 1000 or 3000-4999 and for a
|
||||
// reason over 123 UTF-8 bytes, and the throw leaves the socket open: the peer keeps
|
||||
// streaming into a connection nobody reads. Browsers, Electron and Node's global
|
||||
// WebSocket enforce this; Node's `ws` accepts more. Translate once here so callers
|
||||
// keep their reasons and every runtime sends the same close frame.
|
||||
const MAX_CLOSE_REASON_BYTES = 123;
|
||||
|
||||
function toSendableCloseCode(code: number | undefined): number {
|
||||
if (code === 1000 || (code !== undefined && code >= 3000 && code <= 4999)) return code;
|
||||
return 1000;
|
||||
}
|
||||
|
||||
function toSendableCloseReason(reason: string | undefined): string | undefined {
|
||||
if (reason === undefined) return undefined;
|
||||
const encoder = new TextEncoder();
|
||||
if (encoder.encode(reason).byteLength <= MAX_CLOSE_REASON_BYTES) return reason;
|
||||
let truncated = "";
|
||||
let bytes = 0;
|
||||
for (const character of reason) {
|
||||
bytes += encoder.encode(character).byteLength;
|
||||
if (bytes > MAX_CLOSE_REASON_BYTES) break;
|
||||
truncated += character;
|
||||
}
|
||||
return truncated;
|
||||
}
|
||||
|
||||
function bindWsMessageHandler(
|
||||
ws: WebSocketLike,
|
||||
handler: (data: unknown, isBinary: boolean) => void,
|
||||
|
||||
Reference in New Issue
Block a user