fix(browser): make waitFor options-only and register load waiters before navigate

Author: Alezander9

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

@@ -113,10 +113,12 @@ For unknown param shapes, call with `{}` and inspect the thrown `CdpError` — `
 Common moves:
 
 ```js
-// Navigate.
+// Navigate. Register the load waiter BEFORE navigate so a fast load isn't missed.
 await session.Page.enable()
+const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 15_000 })
 await session.Page.navigate({ url: "https://example.com" })
-await session.waitFor("Page.loadEventFired")
+await loaded
+// Page.navigate resolves even on network errors — its result carries `errorText` when the load failed.
 
 // Evaluate JS in the page.
 const r = await session.Runtime.evaluate({
@@ -153,8 +155,9 @@ export async function scrapeTitles(session: any, urls: string[]) {
   const titles: string[] = []
   await session.Page.enable()
   for (const url of urls) {
+    const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 15_000 })
     await session.Page.navigate({ url })
-    await session.waitFor("Page.loadEventFired")
+    await loaded
     const r = await session.Runtime.evaluate({ expression: "document.title", returnByValue: true })
     titles.push(r.result.value)
   }

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

@@ -188,21 +188,42 @@ export class Session implements Transport {
     };
   }
 
-  /** Wait for the next event matching `method` (and optional predicate). */
-  waitFor<T = unknown>(method: string, predicate?: (params: T) => boolean, timeoutMs = 30_000): Promise<T> {
-    return new Promise((resolve, reject) => {
+  /**
+   * Wait for the next event matching `method` (and optional predicate).
+   * Register the waiter before the call that triggers the event:
+   *   const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 15_000 })
+   *   await session.Page.navigate({ url })
+   *   await loaded
+   */
+  waitFor<T = unknown>(method: string, opts: { predicate?: (params: T) => boolean; timeoutMs?: number } = {}): Promise<T> {
+    if (typeof opts === 'function') {
+      throw new TypeError('waitFor(method, { predicate, timeoutMs }) — pass the predicate in the options object');
+    }
+    const p = new Promise<T>((resolve, reject) => {
       const timer = setTimeout(() => {
         unsub();
         reject(new Error(`Timeout waiting for ${method}`));
-      }, timeoutMs);
+      }, opts.timeoutMs ?? 30_000);
       const unsub = this.onEvent((m, params) => {
         if (m !== method) return;
-        if (predicate && !predicate(params as T)) return;
+        try {
+          if (opts.predicate && !opts.predicate(params as T)) return;
+        } catch (e) {
+          clearTimeout(timer);
+          unsub();
+          reject(e);
+          return;
+        }
         clearTimeout(timer);
         unsub();
         resolve(params as T);
       });
     });
+    // Pre-observe so an abandoned waiter (snippet returned or threw before
+    // awaiting it) times out without an unhandled rejection. Awaiting
+    // callers still see the rejection.
+    p.catch(() => {});
+    return p;
   }
 
   // Transport implementation. Called by the generated domain bindings.

packages/bcode-browser/test/browser-execute.test.ts

@@ -87,8 +87,9 @@ test.skipIf(!enabled)("workspace import inside a snippet", async () => {
          await session.use(page.targetId)
        }
        await session.Page.enable()
+       const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 5000 })
        await session.Page.navigate({ url: "data:text/html,<title>bcode-be</title>" })
-       await session.waitFor("Page.loadEventFired", undefined, 5000)
+       await loaded
        const r = await session.Runtime.evaluate({ expression: "document.title", returnByValue: true })
        return r.result.value
      }`,
@@ -125,8 +126,9 @@ test.skipIf(!enabled)("Page.captureScreenshot is collected into result.screensho
           {
             description: "Capture two screenshots",
             code: `await session.Page.enable();
+                   const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 5000 });
                    await session.Page.navigate({ url: "data:text/html,<title>shot</title><body>hi" });
-                   await session.waitFor("Page.loadEventFired", undefined, 5000);
+                   await loaded;
                    const a = await session.Page.captureScreenshot({ format: "png" });
                    const b = await session.Page.captureScreenshot({ format: "jpeg", quality: 50 });
                    return { aLen: a.data.length, bLen: b.data.length };`,

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

@@ -0,0 +1,66 @@
+// waitFor semantics against a bare WebSocket server (no Chrome needed).
+// Test structure adapted from PR #111 by @MagMueller.
+import { afterAll, beforeAll, expect, test } from "bun:test"
+import { Session } from "../src/cdp/session"
+
+const channel = "cdp-events"
+const server = Bun.serve({
+  port: 0,
+  fetch(req, srv) {
+    return srv.upgrade(req) ? undefined : new Response("nope", { status: 400 })
+  },
+  websocket: {
+    open(ws) {
+      ws.subscribe(channel)
+    },
+    message() {},
+  },
+})
+const session = new Session()
+const emit = (method: string, params: unknown) => {
+  server.publish(channel, JSON.stringify({ method, params }))
+}
+
+beforeAll(async () => {
+  await session.connect({ wsUrl: `ws://127.0.0.1:${server.port}/` })
+})
+
+afterAll(() => {
+  session.close()
+  server.stop(true)
+})
+
+test("waitFor resolves on a matching event, respecting the predicate", async () => {
+  const waiting = session.waitFor<{ ready: boolean }>("Test.event", {
+    predicate: (params) => params.ready,
+    timeoutMs: 1_000,
+  })
+  emit("Test.event", { ready: false })
+  emit("Test.event", { ready: true })
+  expect(await waiting).toEqual({ ready: true })
+})
+
+test("waitFor honors timeoutMs", async () => {
+  await expect(session.waitFor("Test.timeout", { timeoutMs: 20 })).rejects.toThrow("Timeout waiting for Test.timeout")
+})
+
+test("waitFor rejects and unsubscribes when a predicate throws", async () => {
+  let calls = 0
+  const waiting = session.waitFor("Test.bad", {
+    predicate: () => {
+      calls++
+      throw new Error("predicate failed")
+    },
+    timeoutMs: 1_000,
+  })
+  emit("Test.bad", {})
+  await expect(waiting).rejects.toThrow("predicate failed")
+  emit("Test.bad", {})
+  await Bun.sleep(10)
+  expect(calls).toBe(1)
+})
+
+test("waitFor throws synchronously on the removed positional-predicate form", () => {
+  // @ts-expect-error old signature: waitFor(method, predicate, timeoutMs)
+  expect(() => session.waitFor("Test.positional", () => true, 1_000)).toThrow(TypeError)
+})

packages/bcode-browser/test/cdp-smoke.test.ts

@@ -32,8 +32,9 @@ test.skipIf(!enabled)("Session connects, navigates, reads title", async () => {
     }
 
     await session.domains.Page.enable()
+    const loaded = session.waitFor("Page.loadEventFired", { timeoutMs: 5000 })
     await session.domains.Page.navigate({ url: "data:text/html,<title>bcode-smoke</title>" })
-    await session.waitFor("Page.loadEventFired", undefined, 5000)
+    await loaded
 
     const r = (await session.domains.Runtime.evaluate({
       expression: "document.title",