add withCdpSession + getOrCreateCdpSession helpers

Two CDP-session lifecycle helpers in cdp-bridge.ts:

- withCdpSession(page, fn): ephemeral session with try/finally detach.
  For one-shot CDP work (archive snapshots, $B memory, single
  Page.captureScreenshot) where the caller doesn't need session reuse.
- getOrCreateCdpSession(page, cache): cached long-lived session that
  registers a page.once('close') hook to BOTH delete the cache entry
  AND call session.detach(). Pre-helper code only deleted the cache
  entry, leaving the Chromium-side CDP target attached until the
  underlying transport dropped.

Pure addition. Existing callers untouched in this commit; they migrate
in the next commit alongside the static-grep test that pins the
invariant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-05-27 07:22:16 -07:00
parent a6fb31726c
commit ec48c176f5
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
1 changed files with 73 additions and 1 deletions

View File

@ -25,8 +25,80 @@ import { logTelemetry } from './telemetry';
const CDP_TIMEOUT_MS = 5000;
const CDP_ACQUIRE_TIMEOUT_MS = 5000;
// ─── CDP session lifecycle helpers ─────────────────────────────
//
// Every direct `newCDPSession(page)` call needs a matching `session.detach()`
// to release the Chromium-side CDP target. Forgetting the detach leaves the
// target attached until the underlying transport drops (often process exit),
// which on a long-lived headed browser shows up as steadily-climbing
// browser-process RSS. To make the leak class unforgettable, callers should
// go through one of these two helpers and a static-grep test
// (browse/test/cdp-session-cleanup.test.ts) fails CI if any source file
// calls `newCDPSession(` outside this module.
/**
* Ephemeral CDP session with try/finally detach. Use for one-shot CDP work
* where the caller doesn't need session reuse e.g. archive snapshots,
* `$B memory`, a single `Page.captureScreenshot`. The session is detached
* in `finally` regardless of whether `fn` threw, so the Chromium target
* doesn't leak on the error path.
*
* For repeated use of the same page (e.g. the `$B cdp` bridge or the
* inspector), use `getOrCreateCdpSession` instead it caches and detaches
* on page close.
*/
export async function withCdpSession<T>(
page: Page,
fn: (session: any) => Promise<T>,
): Promise<T> {
const session = await page.context().newCDPSession(page);
try {
return await fn(session);
} finally {
try {
await session.detach();
} catch {
// Best-effort cleanup. Session may already be detached (target closed,
// context recreated, browser disconnect). Swallowing all errors is the
// correct cleanup posture per CLAUDE.md "best-effort cleanup paths".
}
}
}
/**
* Cached long-lived CDP session keyed by Page. First call creates the
* session and registers a `page.once('close', ...)` hook that removes the
* cache entry AND calls `session.detach()`. Pre-helper code only removed
* the cache entry, leaving the Chromium-side target attached.
*
* Pass a caller-owned WeakMap so this helper doesn't impose a single global
* cache the `$B cdp` bridge and the inspector each keep their own session
* pool with different invariants (e.g. the inspector also detaches on
* `framenavigated` because DOM/CSS domain state is tied to the document).
*/
export async function getOrCreateCdpSession(
page: Page,
cache: WeakMap<Page, any>,
): Promise<any> {
let session = cache.get(page);
if (session) return session;
session = await page.context().newCDPSession(page);
cache.set(page, session);
page.once('close', () => {
cache.delete(page);
session.detach().catch(() => {
// Best-effort cleanup — see withCdpSession finally block.
});
});
return session;
}
// ─── $B cdp bridge ─────────────────────────────────────────────
// Per-page CDPSession cache. Created lazily on first allow-listed call,
// cleaned up when the page closes.
// cleaned up when the page closes. TODO(C2): migrate to getOrCreateCdpSession
// so the session also detaches on close (currently only the cache entry is
// removed; the Chromium-side target lingers).
const sessionCache: WeakMap<Page, any> = new WeakMap();
async function getCdpSession(page: Page): Promise<any> {