-
Notifications
You must be signed in to change notification settings - Fork 35
fix(browser): recover stale CDP sessions #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| import { bindDomains, type Domains, type Transport } from './generated.ts'; | ||
|
|
||
| type Pending = { | ||
| ws: WebSocket; | ||
| resolve: (v: unknown) => void; | ||
| reject: (e: unknown) => void; | ||
| }; | ||
|
|
@@ -46,6 +47,7 @@ export class Session implements Transport { | |
| private nextId = 1; | ||
| private pending = new Map<number, Pending>(); | ||
| private activeSessionId: string | undefined; | ||
| private reattachPromise?: Promise<void>; | ||
| private eventListeners: Array<(method: string, params: unknown, sessionId?: string) => void> = []; | ||
| private callResultListeners: Array<(method: string, params: unknown, result: unknown) => void> = []; | ||
|
|
||
|
|
@@ -124,15 +126,29 @@ export class Session implements Transport { | |
| else res(); | ||
| }; | ||
| const timer = setTimeout(() => finish(new Error(`timed out after ${timeoutMs}ms`)), timeoutMs); | ||
| ws.addEventListener('open', () => finish()); | ||
| ws.addEventListener('open', () => { | ||
| if (done) { | ||
| try { ws.close(); } catch { /* ignore */ } | ||
| return; | ||
| } | ||
| const previous = this.ws; | ||
| this.ws = ws; | ||
| this.activeSessionId = undefined; | ||
| finish(); | ||
| if (previous && previous !== ws) { | ||
| try { previous.close(); } catch { /* ignore */ } | ||
| } | ||
| }); | ||
| ws.addEventListener('error', (e) => finish(new Error(`WS error: ${(e as any)?.message ?? 'connect failed (likely 403, permission not granted, or port closed)'}`))); | ||
| ws.addEventListener('message', (e) => this.onMessage(String(e.data))); | ||
| ws.addEventListener('message', (e) => this.onMessage(String(e.data), ws)); | ||
| ws.addEventListener('close', () => { | ||
| for (const [, p] of this.pending) p.reject(new Error('CDP socket closed')); | ||
| this.pending.clear(); | ||
| this.rejectPending(ws, new Error('CDP socket closed')); | ||
| if (this.ws === ws) { | ||
| this.ws = undefined; | ||
| this.activeSessionId = undefined; | ||
| } | ||
| finish(new Error('WS closed before open (likely 403 or port closed)')); | ||
| }); | ||
| this.ws = ws; | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -206,17 +222,36 @@ export class Session implements Transport { | |
| } | ||
|
|
||
| // Transport implementation. Called by the generated domain bindings. | ||
| _call(method: string, params: unknown = {}): Promise<unknown> { | ||
| if (!this.ws || this.ws.readyState !== WebSocket.OPEN) { | ||
| async _call(method: string, params: unknown = {}): Promise<unknown> { | ||
| const browserLevel = isBrowserLevel(method); | ||
| const sentSessionId = browserLevel ? undefined : this.activeSessionId; | ||
| try { | ||
| return await this.send(method, params, sentSessionId); | ||
| } catch (error) { | ||
| if (!sentSessionId || !isMissingSessionError(error)) throw error; | ||
|
|
||
| // Chrome explicitly rejected the command before executing it, so this is | ||
| // safe to retry once. Socket drops are deliberately not retried: Chrome | ||
| // may have applied a click or submission before the response was lost. | ||
| if (this.activeSessionId === sentSessionId) this.activeSessionId = undefined; | ||
| if (!this.activeSessionId) await this.reattachFirstPage(); | ||
| if (!this.activeSessionId) throw error; | ||
| return this.send(method, params, this.activeSessionId); | ||
| } | ||
| } | ||
|
|
||
| private send(method: string, params: unknown, sessionId?: string): Promise<unknown> { | ||
| const ws = this.ws; | ||
| if (!ws || ws.readyState !== WebSocket.OPEN) { | ||
| return Promise.reject(new Error('Not connected. Call session.connect(...) first.')); | ||
| } | ||
|
|
||
| const id = this.nextId++; | ||
| const msg: Record<string, unknown> = { id, method, params: params ?? {} }; | ||
| if (this.activeSessionId && !isBrowserLevel(method)) { | ||
| msg.sessionId = this.activeSessionId; | ||
| } | ||
| if (sessionId) msg.sessionId = sessionId; | ||
| return new Promise((resolve, reject) => { | ||
| this.pending.set(id, { | ||
| ws, | ||
| resolve: (v) => { | ||
| for (const fn of this.callResultListeners) { | ||
| try { fn(method, params, v); } catch { /* ignore */ } | ||
|
|
@@ -225,16 +260,55 @@ export class Session implements Transport { | |
| }, | ||
| reject, | ||
| }); | ||
| this.ws!.send(JSON.stringify(msg)); | ||
| try { | ||
| ws.send(JSON.stringify(msg)); | ||
| } catch (error) { | ||
| this.pending.delete(id); | ||
| reject(error); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| private onMessage(raw: string): void { | ||
| private async reattachFirstPage(): Promise<void> { | ||
| if (this.reattachPromise) return this.reattachPromise; | ||
|
|
||
| const attempt = this.attachFirstPage(); | ||
| this.reattachPromise = attempt; | ||
| try { | ||
| await attempt; | ||
| } finally { | ||
| if (this.reattachPromise === attempt) this.reattachPromise = undefined; | ||
| } | ||
| } | ||
|
|
||
| private async attachFirstPage(): Promise<void> { | ||
| const { targetInfos } = await this.domains.Target.getTargets({}); | ||
| const pages = targetInfos as PageTarget[]; | ||
| const targetId = pages.find(isUsablePageTarget)?.targetId | ||
| ?? (await this.domains.Target.createTarget({ url: 'about:blank' })).targetId; | ||
| const sessionId = await this.use(targetId); | ||
| await Promise.allSettled( | ||
| ['Page', 'DOM', 'Runtime', 'Network'].map( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Recovery restores only four hard-coded domains, so callers that enabled Prompt for AI agents |
||
| domain => this.send(`${domain}.enable`, {}, sessionId), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| private rejectPending(ws: WebSocket, error: Error): void { | ||
| for (const [id, pending] of this.pending) { | ||
| if (pending.ws !== ws) continue; | ||
| this.pending.delete(id); | ||
| pending.reject(error); | ||
| } | ||
| } | ||
|
|
||
| private onMessage(raw: string, ws: WebSocket): void { | ||
| if (ws !== this.ws) return; | ||
| let m: any; | ||
| try { m = JSON.parse(raw); } catch { return; } | ||
| if (typeof m.id === 'number') { | ||
| const p = this.pending.get(m.id); | ||
| if (!p) return; | ||
| if (!p || p.ws !== ws) return; | ||
| this.pending.delete(m.id); | ||
| if (m.error) p.reject(new CdpError(m.error.code, m.error.message, m.error.data)); | ||
| else p.resolve(m.result); | ||
|
|
@@ -258,6 +332,12 @@ function isBrowserLevel(method: string): boolean { | |
| return method.startsWith('Browser.') || method.startsWith('Target.'); | ||
| } | ||
|
|
||
| function isMissingSessionError(error: unknown): boolean { | ||
| return error instanceof CdpError | ||
| && error.code === -32001 | ||
| && error.message.includes('Session with given id not found'); | ||
| } | ||
|
|
||
| /** | ||
| * Resolve a WebSocket URL for one of the explicit connect forms: | ||
| * { wsUrl } — passthrough. | ||
|
|
@@ -329,6 +409,15 @@ export async function listPageTargets(session: Session): Promise<PageTarget[]> { | |
| ); | ||
| } | ||
|
|
||
| function isUsablePageTarget(target: PageTarget): boolean { | ||
| return target.type === 'page' | ||
| && !target.url.startsWith('chrome://') | ||
| && !target.url.startsWith('chrome-untrusted://') | ||
| && !target.url.startsWith('devtools://') | ||
| && !target.url.startsWith('chrome-extension://') | ||
| && !target.url.startsWith('about:'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Recovery opens an extra blank tab whenever the only usable page is already Prompt for AI agents |
||
| } | ||
|
|
||
| /** | ||
| * Scan OS-specific user-data directories for Chromium-based browsers that | ||
| * currently have remote debugging enabled (a `DevToolsActivePort` file exists | ||
|
|
@@ -423,4 +512,3 @@ async function tryReadDevToolsActivePort( | |
| return undefined; | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Page-domain calls issued during reattachment are sent without
sessionIdand fail rather than recovering. Keep the stale ID until the serialized reattach replaces it, so overlapping calls receive Chrome’s explicit stale-session rejection and join the same recovery.Prompt for AI agents