Skip to content

Commit f5ea2eb

Browse files
committed
fix(browser): retire stale CDP attachments
1 parent 3933862 commit f5ea2eb

3 files changed

Lines changed: 77 additions & 5 deletions

File tree

packages/bcode-browser/skills/browser-execute/SKILL.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ const page = targets.find(t => t.type === "page" && !t.url.startsWith("chrome://
106106
await session.use(page.targetId)
107107
```
108108
109-
If a target-scoped command throws `CdpError` code `-32001` (`Session with given id not found`), the browser connection is still usable but the target session is stale. List targets again, `session.use(...)` the intended page, and retry the rejected command once. Repeated `session.connect()` calls do not replace a stale target session.
109+
If a target-scoped command throws `CdpError` code `-32001` (`Session with given id not found`), the browser connection is still usable but the target session is stale. List targets again, `session.use(...)` the intended page, and retry the rejected command once. Calling `session.connect()` without arguments is a no-op while connected; it does not replace a stale target session.
110+
111+
Every explicit reconnect or browser switch retires the previous socket and clears its active target attachment. Re-list targets, call `session.use(...)`, and rediscover DOM nodes and Runtime objects before continuing.
110112
111113
## Driving a page
112114
Domain methods follow `session.<Domain>.<method>(params)` and return Promises.

packages/bcode-browser/src/cdp/session.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,14 @@ export class Session implements Transport {
123123
if (this.invalidatedError) return Promise.reject(this.invalidatedError);
124124
return new Promise<void>((res, rej) => {
125125
const ws = new WebSocket(wsUrl);
126+
const previousWs = this.ws;
127+
this.ws = ws;
128+
this.activeSessionId = undefined;
129+
if (previousWs) {
130+
for (const [, p] of this.pending) p.reject(new Error('CDP connection replaced'));
131+
this.pending.clear();
132+
try { previousWs.close(); } catch { /* ignore */ }
133+
}
126134
let done = false;
127135
const finish = (err?: Error) => {
128136
if (done) return;
@@ -132,15 +140,28 @@ export class Session implements Transport {
132140
else res();
133141
};
134142
const timer = setTimeout(() => finish(new Error(`timed out after ${timeoutMs}ms`)), timeoutMs);
135-
ws.addEventListener('open', () => finish(this.invalidatedError));
143+
ws.addEventListener('open', () => {
144+
if (this.ws !== ws) {
145+
finish(new Error('CDP connection superseded'));
146+
return;
147+
}
148+
finish(this.invalidatedError);
149+
});
136150
ws.addEventListener('error', (e) => finish(new Error(`WS error: ${(e as any)?.message ?? 'connect failed (likely 403, permission not granted, or port closed)'}`)));
137-
ws.addEventListener('message', (e) => this.onMessage(String(e.data)));
151+
ws.addEventListener('message', (e) => {
152+
if (this.ws === ws) this.onMessage(String(e.data));
153+
});
138154
ws.addEventListener('close', () => {
155+
if (this.ws !== ws) {
156+
finish(new Error('CDP connection superseded'));
157+
return;
158+
}
159+
this.ws = undefined;
160+
this.activeSessionId = undefined;
139161
for (const [, p] of this.pending) p.reject(this.invalidatedError ?? new Error('CDP socket closed'));
140162
this.pending.clear();
141163
finish(this.invalidatedError ?? new Error('WS closed before open (likely 403 or port closed)'));
142164
});
143-
this.ws = ws;
144165
});
145166
}
146167

packages/bcode-browser/test/browser-auto-connect.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { BrowserExecute } from "../src/browser-execute";
77
import { SessionStore } from "../src/session-store";
88

99
let connections = 0;
10+
let closedConnections = 0;
1011
let attachedCalls = 0;
1112
let pageCallsWithSession = 0;
1213
let latestSocket: { close(): void } | undefined;
@@ -57,7 +58,9 @@ const server = Bun.serve({
5758
})();
5859
ws.send(JSON.stringify({ id: request.id, result }));
5960
},
60-
close() {},
61+
close() {
62+
closedConnections++;
63+
},
6164
},
6265
});
6366

@@ -198,6 +201,7 @@ test("parallel first calls share one connection attempt", async () => {
198201

199202
test("a dropped socket is surfaced instead of reconnecting the run-start browser", async () => {
200203
connections = 0;
204+
closedConnections = 0;
201205
attachedCalls = 0;
202206
await withEnv(
203207
{ V4_RUN_ID: "run-dropped", BU_CDP_WS: wsUrl, BU_CDP_URL: undefined },
@@ -222,11 +226,56 @@ test("a dropped socket is surfaced instead of reconnecting the run-start browser
222226
);
223227

224228
expect(connections).toBe(1);
229+
expect(closedConnections).toBe(1);
225230
expect(attachedCalls).toBe(1);
226231
}),
227232
);
228233
});
229234

235+
test("an explicit browser switch retires the old socket and target attachment", async () => {
236+
connections = 0;
237+
closedConnections = 0;
238+
attachedCalls = 0;
239+
pageCallsWithSession = 0;
240+
await withEnv(
241+
{ V4_RUN_ID: "run-switch", BU_CDP_WS: wsUrl, BU_CDP_URL: undefined },
242+
() =>
243+
withBrowserExecute("switch", async (impl, sessionID, workspaceDir) => {
244+
const run = (code: string) =>
245+
Effect.runPromise(
246+
impl.execute(
247+
{ description: "Switch provisioned browsers", code },
248+
{ sessionID, workspaceDir },
249+
),
250+
);
251+
252+
await run(
253+
"return await session.Page.navigate({ url: 'https://sap.com' })",
254+
);
255+
const switched = await run(`
256+
await session.connect({ wsUrl: ${JSON.stringify(wsUrl)} })
257+
await session.Page.navigate({ url: "https://example.com" })
258+
return { activeSession: session.getActiveSession() ?? null }
259+
`);
260+
expect(JSON.parse(switched.result)).toEqual({ activeSession: null });
261+
262+
await new Promise((resolve) => setTimeout(resolve, 10));
263+
expect(connections).toBe(2);
264+
expect(closedConnections).toBe(1);
265+
expect(attachedCalls).toBe(1);
266+
expect(pageCallsWithSession).toBe(1);
267+
268+
await run(`
269+
const page = (await session.Target.getTargets({})).targetInfos[0]
270+
await session.use(page.targetId)
271+
return await session.Page.navigate({ url: "https://example.com" })
272+
`);
273+
expect(attachedCalls).toBe(2);
274+
expect(pageCallsWithSession).toBe(2);
275+
}),
276+
);
277+
});
278+
230279
test("a timeout replacement does not auto-attach the run-start browser again", async () => {
231280
connections = 0;
232281
await withEnv(

0 commit comments

Comments
 (0)