[fix]: bound navigations by their timeout before the navigate command returns (mirror of #3399)

FI
4d ago
+144-25

Mirror of #3399 by @ForrestMoret-at, opened under the external-contributor process so CI can run. The contributor's commit is cherry-picked onto main unchanged, with authorship preserved. A second maintainer commit addresses review notes (see below).

why

goto, reload, goBack and goForward create a LifecycleWatcher with the call's timeout, but the watcher only started counting in wait(), which runs after the CDP command that starts the navigation returns. Chrome answers Page.navigate only once the navigation commits or fails, so the time spent waiting for the server sat outside the timeout:

  • A server that accepts the connection but never responds leaves goto pending with no bound inside the extension. The caller gets the client's RPC response timed out after 25000ms: page.goto (the 15s default plus the 10s RPC grace) instead of a navigation timeout.
  • A server that takes T to respond gets T + timeout in total, so any commit slower than the 10s RPC grace runs into the RPC deadline rather than the navigation's own timeout.

Checked against headless Chrome over CDP: Page.navigate to a local server that never responds had not answered after 30s; to one that responds after 12s, it answered at 12.0s.

what changed

  • LifecycleWatcher fixes its deadline when it is created (startTime + timeout). wait() gets what is left of it, so timeout bounds the whole navigation, as it does in Playwright and Puppeteer.
  • New awaitNavigationCommand() races the command that starts the navigation against that deadline and the watcher's abort. It rejects with the existing TimeoutError (Navigation start timed out after <timeout>ms) and shares its timer with awaitNavigationStart.
  • goto, reload, goBack and goForward await their CDP command through it. Only Page.navigate is known to wait for commit. The other three are wrapped so timeout means the same thing for all four.

Behavior change: a navigation with a slow commit now gets timeout in total rather than timeout after the commit. As before, a timed-out navigation keeps running in the browser.

test plan

  • New case in packages/extension/tests/page-navigation-response.test.ts: Page.navigate never answers, and goto({ timeout: 50 }) rejects with Navigation start timed out after 50ms. On main the same case hangs until vitest's test timeout.
  • Extension unit tests: 1,189 passed. The 3 extension-build tests need pnpm run build first.
  • turbo run fmt:check lint typecheck --filter=@browserbasehq/stagehand-extension passes.
  • Python checks weren't run locally.

maintainer verification (mirror)

Real headless Chrome over CDP, understudy Page driven against local HTTP servers. Baseline is main's understudy/ and treatment is this branch. Each case uses a unique URL.

casemainthis PR
server never responds, timeout: 2000still pending at 15s caprejects at 2004ms, Navigation start timed out after 2000ms
server commits after 4s, timeout: 2000resolves at 4023ms (timeout ignored)rejects at 2004ms
server commits after 4s, timeout: 8000 (control)resolves at 4017msresolves at 4019ms
  • turbo run fmt:check lint typecheck --filter=@browserbasehq/stagehand-extension passes. The extension build passes.
  • Extension unit tests: 1184 passed. 5 failures in tests/browserbase-transport.test.ts (backoff timeouts) fail the same way on main locally, so they're unrelated.

(mirror-only) follow-up commit

Commit 2cb08745a addresses the review notes:

  • awaitNavigationCommand takes the CDP method name, so a command that never answers reports Page.navigate timed out after <timeout>ms (or Page.reload / Page.navigateToHistoryEntry) instead of Navigation start timed out, which pointed at lifecycle events.
  • goBack/goForward bound Page.getNavigationHistory by the same timeout. Before this, a hung history lookup left them pending forever, even though the changeset says all four are bounded. LifecycleWatcher takes an optional startTime so the lookup and the navigation share one deadline.
  • New tests: hung Page.reload, hung Page.navigateToHistoryEntry (goBack and goForward), and hung Page.getNavigationHistory (goBack and goForward). All 6 fail against the first commit alone: the error text differs, or the call hangs.
  • After the follow-up, the real-Chrome results above are unchanged except the message, which is now Page.navigate timed out after 2000ms (2001ms). Extension unit tests: 1189 passed. The same 5 local-only transport failures appear on main.

Not changed: huge timeout values above 2^31-1 ms overflow setTimeout, and they already do on main in waitForMainLoadState. A timed-out navigation still keeps running in the browser (no Page.stopLoading), as before.

🤖 Generated with Claude Code