diff options
| author | amankumarsingh77 <amankumarsingh7702@gmail.com> | 2026-07-28 22:08:16 +0530 |
|---|---|---|
| committer | Aman Kumar <145339995+amankumarsingh77@users.noreply.github.com> | 2026-08-02 19:12:46 +0530 |
| commit | b913f2d4af218ae61de7073778feb56da89a15de (patch) | |
| tree | d1c0f6796b3756563284c8edfd8be46a54e4f221 /test | |
| parent | c88082515dcd9c0df1328b85420e97d7286ad428 (diff) | |
refactor: daemon internals, and fix a leak the merge exposed
IdMultiplexer and EventRouter were the only classes in src/; they are now
createIdMultiplexer/createEventRouter closures like every other factory
here. The multiplexer kept {clientId, localId} per daemon id while the
bridge kept {send, clientId, localId, timer, isAttach} under the same key
— two maps of overlapping data, and mux.resolve()'s return value was
discarded at three of four call sites. One map now holds the union.
That merge fixes a real leak: removeClient cleared the multiplexer but
never the callbacks map, so a disconnected client's timers stayed armed
and later fired a response at a socket that was gone. clearClient now
cancels them, and a test pins it.
handleRequest polled every 250ms for up to 15s per request when Chrome
was down; N queued requests now await one connect signal.
server.ts uses its own protocol's serialize/deserialize instead of
hand-rolled JSON, and broadcast no longer routes through `this` — the
only `this` in the codebase, and the one thing that breaks when the
server object is destructured.
cdp/event-queue.ts held two unrelated concerns; the request/timeout half
moves to cdp/pending-requests.ts. cdp/daemon-transport.ts is the client
half of the daemon protocol, so it moves to daemon/transport.ts.
test/daemon/ needs adding to the test glob to run at all — there was no
daemon test directory before because none of this was reachable without
live Chrome.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Diffstat (limited to 'test')
| -rw-r--r-- | test/cdp/event-queue-test.ts | 3 | ||||
| -rw-r--r-- | test/daemon/bridge-routing-test.ts | 131 | ||||
| -rw-r--r-- | test/manual/daemon-transport-test.ts | 2 | ||||
| -rw-r--r-- | test/manual/e2e-test.ts | 2 | ||||
| -rw-r--r-- | test/manual/forms-test.ts | 2 | ||||
| -rw-r--r-- | test/manual/live-daemon-test.ts | 2 | ||||
| -rw-r--r-- | test/manual/profile-concurrency-test.ts | 2 |
7 files changed, 138 insertions, 6 deletions
diff --git a/test/cdp/event-queue-test.ts b/test/cdp/event-queue-test.ts index cae8143..771ce8e 100644 --- a/test/cdp/event-queue-test.ts +++ b/test/cdp/event-queue-test.ts @@ -1,6 +1,7 @@ import { test, describe } from "node:test"; import assert from "node:assert/strict"; -import { makeEventQueue, rejectAllPending, sendWithTimeout, type Pending } from "../../src/cdp/event-queue"; +import { makeEventQueue } from "../../src/cdp/event-queue"; +import { rejectAllPending, sendWithTimeout, type Pending } from "../../src/cdp/pending-requests"; describe("event queue iteration", () => { test("yields pushed events in order", async () => { diff --git a/test/daemon/bridge-routing-test.ts b/test/daemon/bridge-routing-test.ts new file mode 100644 index 0000000..7c96bad --- /dev/null +++ b/test/daemon/bridge-routing-test.ts @@ -0,0 +1,131 @@ +import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import { createEventRouter, createIdMultiplexer, type InFlight } from "../../src/daemon/bridge"; +import type { SendToClient } from "../../src/daemon/bridge"; +import type { WireEvent } from "../../src/daemon/protocol"; + +const noSend: SendToClient = () => {}; +const entry = (clientId: string, localId: number): Omit<InFlight, "timer"> => ({ + clientId, + localId, + send: noSend, + isAttach: false, +}); + +const inertTimer = (): ReturnType<typeof setTimeout> => setTimeout(() => {}, 60_000); + +describe("daemon id multiplexer", () => { + test("two clients using the same local id get distinct daemon ids", () => { + const mux = createIdMultiplexer(); + const a = mux.allocate(entry("client-a", 1), inertTimer); + const b = mux.allocate(entry("client-b", 1), inertTimer); + assert.notEqual(a, b); + }); + + test("a response is routed back to the client that asked, with its own id", () => { + const mux = createIdMultiplexer(); + const a = mux.allocate(entry("client-a", 7), inertTimer); + const b = mux.allocate(entry("client-b", 7), inertTimer); + assert.equal(mux.take(b)?.clientId, "client-b"); + const takenA = mux.take(a); + assert.equal(takenA?.clientId, "client-a"); + assert.equal(takenA?.localId, 7); + }); + + test("taking the same daemon id twice yields nothing the second time", () => { + const mux = createIdMultiplexer(); + const id = mux.allocate(entry("client-a", 1), inertTimer); + assert.ok(mux.take(id)); + assert.equal(mux.take(id), undefined); + }); + + test("an unknown daemon id is not mistaken for an in-flight request", () => { + const mux = createIdMultiplexer(); + assert.equal(mux.take(999), undefined); + }); + + test("clearClient drops only that client's in-flight requests", () => { + const mux = createIdMultiplexer(); + const a = mux.allocate(entry("client-a", 1), inertTimer); + const b = mux.allocate(entry("client-b", 1), inertTimer); + mux.clearClient("client-a"); + assert.equal(mux.take(a), undefined); + assert.equal(mux.take(b)?.clientId, "client-b"); + }); + + test("clearClient cancels the pending timeout so a gone client is never answered", () => { + const mux = createIdMultiplexer(); + let fired = false; + mux.allocate(entry("client-a", 1), () => setTimeout(() => { fired = true; }, 1)); + mux.clearClient("client-a"); + return new Promise<void>((resolve) => setTimeout(() => { + assert.equal(fired, false); + resolve(); + }, 30)); + }); + + test("takeAll drains every in-flight request exactly once", () => { + const mux = createIdMultiplexer(); + mux.allocate(entry("client-a", 1), inertTimer); + mux.allocate(entry("client-b", 2), inertTimer); + assert.equal(mux.takeAll().length, 2); + assert.equal(mux.takeAll().length, 0); + }); +}); + +const ev = (sessionId?: string): WireEvent => ({ + type: "event", + method: "Page.loadEventFired", + ...(sessionId !== undefined ? { sessionId } : {}), +}); + +describe("daemon event router", () => { + test("a session-scoped event goes only to the client owning that session", () => { + const router = createEventRouter(); + router.record("client-a", "S1"); + router.record("client-b", "S2"); + assert.deepEqual(router.route(ev("S1")), ["client-a"]); + assert.deepEqual(router.route(ev("S2")), ["client-b"]); + }); + + test("an event for an unknown session goes nowhere rather than to everyone", () => { + const router = createEventRouter(); + router.record("client-a", "S1"); + assert.deepEqual(router.route(ev("S9")), []); + }); + + test("a session-less event goes to every client that holds a session", () => { + const router = createEventRouter(); + router.record("client-a", "S1"); + router.record("client-b", "S2"); + assert.deepEqual(router.route(ev()).sort(), ["client-a", "client-b"]); + }); + + test("re-recording a session moves ownership and does not leave it with the old client", () => { + const router = createEventRouter(); + router.record("client-a", "S1"); + router.record("client-b", "S1"); + assert.equal(router.getOwner("S1"), "client-b"); + assert.deepEqual(router.route(ev("S1")), ["client-b"]); + }); + + test("releasing a session leaves its events unrouted", () => { + const router = createEventRouter(); + router.record("client-a", "S1"); + router.release("S1"); + assert.equal(router.getOwner("S1"), undefined); + assert.deepEqual(router.route(ev("S1")), []); + }); + + test("removing a client releases every session it owned", () => { + const router = createEventRouter(); + router.record("client-a", "S1"); + router.record("client-a", "S2"); + router.record("client-b", "S3"); + router.removeClient("client-a"); + assert.equal(router.getOwner("S1"), undefined); + assert.equal(router.getOwner("S2"), undefined); + assert.equal(router.getOwner("S3"), "client-b"); + assert.deepEqual(router.route(ev()), ["client-b"]); + }); +}); diff --git a/test/manual/daemon-transport-test.ts b/test/manual/daemon-transport-test.ts index 5738e23..30f5a29 100644 --- a/test/manual/daemon-transport-test.ts +++ b/test/manual/daemon-transport-test.ts @@ -1,5 +1,5 @@ import { createIpcServer } from "../../src/daemon/server"; -import { createDaemonTransport } from "../../src/cdp/daemon-transport"; +import { createDaemonTransport } from "../../src/daemon/transport"; import { DAEMON_SOCKET_PATH } from "../../src/daemon/protocol"; import type { WireMessage } from "../../src/daemon/protocol"; import { unlinkSync } from "node:fs"; diff --git a/test/manual/e2e-test.ts b/test/manual/e2e-test.ts index 81cb4df..02e04a4 100644 --- a/test/manual/e2e-test.ts +++ b/test/manual/e2e-test.ts @@ -1,6 +1,6 @@ import { createIpcServer } from "../../src/daemon/server"; import { createCdpBridge, type SendToClient } from "../../src/daemon/bridge"; -import { createDaemonTransport } from "../../src/cdp/daemon-transport"; +import { createDaemonTransport } from "../../src/daemon/transport"; import { DAEMON_SOCKET_PATH } from "../../src/daemon/protocol"; import type { WireRequest } from "../../src/daemon/protocol"; import { unlinkSync } from "node:fs"; diff --git a/test/manual/forms-test.ts b/test/manual/forms-test.ts index 43b2863..274f039 100644 --- a/test/manual/forms-test.ts +++ b/test/manual/forms-test.ts @@ -1,4 +1,4 @@ -import { createDaemonTransport } from "../../src/cdp/daemon-transport"; +import { createDaemonTransport } from "../../src/daemon/transport"; import { createBrowserClient } from "../../src/client"; import { fillTool, selectOptionTool, setCheckedTool } from "../../src/domains/form"; import { fillFormTool } from "../../src/domains/form-batch"; diff --git a/test/manual/live-daemon-test.ts b/test/manual/live-daemon-test.ts index 28ec7cd..5334cbc 100644 --- a/test/manual/live-daemon-test.ts +++ b/test/manual/live-daemon-test.ts @@ -1,4 +1,4 @@ -import { createDaemonTransport } from "../../src/cdp/daemon-transport"; +import { createDaemonTransport } from "../../src/daemon/transport"; async function main() { const t = createDaemonTransport("pi-live-test"); diff --git a/test/manual/profile-concurrency-test.ts b/test/manual/profile-concurrency-test.ts index 43cb880..83b233d 100644 --- a/test/manual/profile-concurrency-test.ts +++ b/test/manual/profile-concurrency-test.ts @@ -3,7 +3,7 @@ import { existsSync, mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createBrowserClient } from "../../src/client"; -import { createDaemonTransport } from "../../src/cdp/daemon-transport"; +import { createDaemonTransport } from "../../src/daemon/transport"; import { isDaemonRunning, spawnDaemon } from "../../src/daemon/spawn"; import { closeIsolatedTab, openIsolatedTab } from "../../src/domains/isolated-tab"; |
