Skip to content

Commit 9d674e0

Browse files
committed
fix(browser): handle navigation waiter edge cases
1 parent 50d806f commit 9d674e0

3 files changed

Lines changed: 37 additions & 5 deletions

File tree

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,12 @@ Common moves:
116116
// Navigate.
117117
await session.Page.enable()
118118
const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 15_000 })
119+
// Keep the rejection observed if navigation itself fails before `await loaded`.
120+
void loaded.catch(() => {})
119121
const navigation = await session.Page.navigate({ url: "https://example.com" })
120122
if (navigation.errorText) throw new Error(`Navigation failed: ${navigation.errorText}`)
121-
await loaded
123+
// Same-document navigations have no loaderId and do not fire a new load event.
124+
if (navigation.loaderId) await loaded
122125
123126
// Evaluate JS in the page.
124127
const r = await session.Runtime.evaluate({
@@ -156,9 +159,10 @@ export async function scrapeTitles(session: any, urls: string[]) {
156159
await session.Page.enable()
157160
for (const url of urls) {
158161
const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 15_000 })
162+
void loaded.catch(() => {})
159163
const navigation = await session.Page.navigate({ url })
160164
if (navigation.errorText) throw new Error(`Navigation failed: ${navigation.errorText}`)
161-
await loaded
165+
if (navigation.loaderId) await loaded
162166
const r = await session.Runtime.evaluate({ expression: "document.title", returnByValue: true })
163167
titles.push(r.result.value)
164168
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ export class Session implements Transport {
216216
}
217217
const predicate = typeof predicateOrOptions === 'function' ? predicateOrOptions : options?.predicate;
218218
const timeoutMs = options?.timeoutMs ?? positionalTimeoutMs ?? 30_000;
219+
const sessionId = isBrowserLevel(method) ? undefined : this.activeSessionId;
219220
if (!Number.isFinite(timeoutMs) || timeoutMs < 0) {
220221
throw new TypeError('waitFor timeoutMs must be a non-negative finite number');
221222
}
@@ -225,8 +226,9 @@ export class Session implements Transport {
225226
unsub();
226227
reject(new Error(`Timeout waiting for ${method}`));
227228
}, timeoutMs);
228-
const unsub = this.onEvent((m, params) => {
229+
const unsub = this.onEvent((m, params, eventSessionId) => {
229230
if (m !== method) return;
231+
if (sessionId !== undefined && eventSessionId !== sessionId) return;
230232
try {
231233
if (predicate && !predicate(params as T)) return;
232234
} catch (error) {

‎packages/bcode-browser/test/cdp-session.test.ts‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ const server = Bun.serve({
1717
const method = Reflect.get(message, "method")
1818
const id = Reflect.get(message, "id")
1919
if (method !== "Page.navigate" || typeof id !== "number") return
20+
const params = Reflect.get(message, "params")
21+
const url = typeof params === "object" && params !== null ? Reflect.get(params, "url") : undefined
22+
if (url === "https://navigation-fails.example") {
23+
ws.send(JSON.stringify({ id, result: { frameId: "frame", errorText: "net::ERR_FAILED" } }))
24+
return
25+
}
2026
ws.send(JSON.stringify({ method: "Page.loadEventFired", params: { timestamp: 1 } }))
2127
ws.send(JSON.stringify({ id, result: { frameId: "frame" } }))
2228
},
@@ -34,8 +40,8 @@ afterAll(() => {
3440
server.stop(true)
3541
})
3642

37-
const emit = (method: string, params: unknown) => {
38-
server.publish(channel, JSON.stringify({ method, params }))
43+
const emit = (method: string, params: unknown, sessionId?: string) => {
44+
server.publish(channel, JSON.stringify({ method, params, sessionId }))
3945
}
4046

4147
test("waitFor accepts predicate and timeout options", async () => {
@@ -82,6 +88,26 @@ test("a waiter registered before navigation catches an event emitted before the
8288
expect(await loaded).toEqual({ timestamp: 1 })
8389
})
8490

91+
test("waitFor ignores matching events from another attached session", async () => {
92+
session.setActiveSession("session-active")
93+
try {
94+
const waiting = session.waitFor<{ source: string }>("Page.loadEventFired", { timeoutMs: 1_000 })
95+
emit("Page.loadEventFired", { source: "background" }, "session-background")
96+
emit("Page.loadEventFired", { source: "active" }, "session-active")
97+
expect(await waiting).toEqual({ source: "active" })
98+
} finally {
99+
session.setActiveSession(undefined)
100+
}
101+
})
102+
103+
test("navigation failure does not leave an unhandled waiter rejection", async () => {
104+
const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 20 })
105+
void loaded.catch(() => {})
106+
const navigation = await session.domains.Page.navigate({ url: "https://navigation-fails.example" })
107+
expect(navigation.errorText).toBe("net::ERR_FAILED")
108+
await Bun.sleep(30)
109+
})
110+
85111
test("waitFor rejects invalid runtime arguments immediately", () => {
86112
expect(() =>
87113
// @ts-expect-error Runtime callers can still pass invalid JavaScript.

0 commit comments

Comments
 (0)