From c9d1605b1e3b8dbe88cd3674a9256aa0465f3e6d Mon Sep 17 00:00:00 2001 From: amankumarsingh77 Date: Sat, 2 May 2026 16:53:34 +0530 Subject: chore: address final-review minors — surface event-consumer crashes, document IIFE-wrap heuristic, log shutdown failures, fix CHANGELOG file-size claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five tweaks from the whole-implementation review: - session.ts: log unexpected event-consumer rejections instead of silent swallow - client.ts: comment the return-substring IIFE-wrap false-positive case - transport.ts: comment that ws.onclose post-open settle is no-op-by-design - index.ts: log client.stop() failures during shutdown - CHANGELOG.md: correct largest-file claim (transport.ts and client.ts are ~220 each, not "client.ts at ~180") Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cdp/session.ts | 8 +++++++- src/cdp/transport.ts | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) (limited to 'src/cdp') diff --git a/src/cdp/session.ts b/src/cdp/session.ts index fffa241..b1ae71c 100644 --- a/src/cdp/session.ts +++ b/src/cdp/session.ts @@ -45,7 +45,13 @@ export const createCdpSession = (transport: CdpTransport): CdpSession => { }; const restartConsumer = (): void => { - activeConsumer = activeConsumer.then(() => consumeEvents()).catch(() => undefined); + activeConsumer = activeConsumer.then(() => consumeEvents()).catch((e: unknown) => { + // The .then() chain calls consumeEvents() which iterates the transport's + // events() AsyncIterable. The iterable resolves cleanly on close (returns + // {done:true}); any rejection here is an unexpected bug in the event + // handler, not a normal termination. Surface it on stderr so it's not lost. + console.warn("[pi-browser-harness] CDP event consumer crashed:", e); + }); }; restartConsumer(); diff --git a/src/cdp/transport.ts b/src/cdp/transport.ts index 6da25fa..c5a5a41 100644 --- a/src/cdp/transport.ts +++ b/src/cdp/transport.ts @@ -156,6 +156,9 @@ export const createCdpTransport = (): CdpTransport => { clearTimeout(timer); ws = null; cleanup("WebSocket closed"); + // If onopen already settled the connect promise, this settle is a no-op + // (guarded by `settled`). cleanup() above still runs unconditionally so + // pending requests are rejected and the event queue is rotated. settle(err(cdpError("transport_closed", "CDP WebSocket closed"))); }; }); -- cgit v1.2.3