diff --git a/src/remote/api-client.test.ts b/src/remote/api-client.test.ts new file mode 100644 index 0000000..b9c51a2 --- /dev/null +++ b/src/remote/api-client.test.ts @@ -0,0 +1,84 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { newWebSocketRpcSession, RpcTarget } from "capnweb"; +import { WebSocketServer, type WebSocket as ServerSocket } from "ws"; +import type { Remote } from "./remote.ts"; +import { ApiClient } from "./api-client.ts"; + +// Relay stand-in: answers `authenticate` with a ServerApi-shaped stub so +// ApiClient.connect runs its real handshake and dispose path over a genuine +// WebSocket. That transport is the only place capnweb fires onRpcBroken with +// "RPC session was shut down by disposing the main stub" — an in-memory session +// doesn't reproduce it, so the bug would slip through a lighter fake. +class FakeServerApi extends RpcTarget { + async ping() { + return true; + } +} + +class FakeRelay extends RpcTarget { + async authenticate() { + return new FakeServerApi(); + } +} + +// In CI mode connect() never calls back into `remote` (that wiring is the +// persistent-server path), so an empty stand-in is enough. +const CI_MODE = { kind: "ci", branch: "main", sha: "" } as const; +const NO_REMOTE = {} as unknown as Remote; + +async function startRelay() { + const wss = new WebSocketServer({ port: 0 }); + await new Promise((resolve) => wss.once("listening", () => resolve())); + const sockets: ServerSocket[] = []; + const closes: Promise[] = []; + wss.on("connection", (socket) => { + sockets.push(socket); + closes.push(new Promise((resolve) => socket.once("close", () => resolve()))); + newWebSocketRpcSession(socket as unknown as WebSocket, new FakeRelay()); + }); + const { port } = wss.address() as { port: number }; + return { + endpoint: `http://127.0.0.1:${port}`, + dropConnection: () => sockets.forEach((s) => s.terminate()), + // The client's onRpcBroken can only fire once its transport is gone. + // Awaiting the server-observed close and then flushing the timer queue + // guarantees that window has fully passed, so "callback not called" means it + // genuinely never fired rather than that we asserted too early. + afterTransportClosed: async () => { + await Promise.all(closes); + await new Promise((resolve) => setTimeout(resolve, 50)); + }, + close: () => new Promise((resolve) => wss.close(() => resolve())), + }; +} + +describe("ApiClient.connect (integration)", () => { + let relay: Awaited> | undefined; + + afterEach(async () => { + await relay?.close(); + relay = undefined; + }); + + it("stays silent when the caller disposes the connection", async () => { + relay = await startRelay(); + const onBroken = vi.fn(); + const { dispose } = await ApiClient.connect(relay.endpoint, "token", CI_MODE, NO_REMOTE, onBroken); + + dispose(); + await relay.afterTransportClosed(); + + expect(onBroken).not.toHaveBeenCalled(); + }); + + it("reports a broken connection when the transport drops mid-run", async () => { + relay = await startRelay(); + const onBroken = vi.fn(); + await ApiClient.connect(relay.endpoint, "token", CI_MODE, NO_REMOTE, onBroken); + + relay.dropConnection(); + await relay.afterTransportClosed(); + + expect(onBroken).toHaveBeenCalledWith(expect.any(Error)); + }); +}); diff --git a/src/remote/api-client.ts b/src/remote/api-client.ts index a580acb..31112e5 100644 --- a/src/remote/api-client.ts +++ b/src/remote/api-client.ts @@ -108,6 +108,11 @@ export class ApiClient extends RpcTarget implements ClientApi { const dispose = () => { if (disposed) return; disposed = true; + // Mark the connection broken before disposing the stub. Disposing makes + // capnweb fire onRpcBroken with "RPC session was shut down by disposing + // the main stub"; without this guard triggerBroken would report our own + // intentional teardown as a broken connection. + broken = true; stopPing(); try { api[Symbol.dispose](); } catch { /* already gone */ } try { (unauthenticated as unknown as Disposable)[Symbol.dispose]?.(); } catch { /* already gone */ }