[fix]: bound navigations by their timeout before the navigate command returns (mirror of #3399)
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
gotopending with no bound inside the extension. The caller gets the client'sRPC 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 +
timeoutin 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
LifecycleWatcherfixes its deadline when it is created (startTime + timeout).wait()gets what is left of it, sotimeoutbounds 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 existingTimeoutError(Navigation start timed out after <timeout>ms) and shares its timer withawaitNavigationStart. goto,reload,goBackandgoForwardawait their CDP command through it. OnlyPage.navigateis known to wait for commit. The other three are wrapped sotimeoutmeans 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.navigatenever answers, andgoto({ timeout: 50 })rejects withNavigation start timed out after 50ms. Onmainthe same case hangs until vitest's test timeout. - Extension unit tests: 1,189 passed. The 3
extension-buildtests needpnpm run buildfirst. turbo run fmt:check lint typecheck --filter=@browserbasehq/stagehand-extensionpasses.- 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.
| case | main | this PR |
|---|---|---|
server never responds, timeout: 2000 | still pending at 15s cap | rejects at 2004ms, Navigation start timed out after 2000ms |
server commits after 4s, timeout: 2000 | resolves at 4023ms (timeout ignored) | rejects at 2004ms |
server commits after 4s, timeout: 8000 (control) | resolves at 4017ms | resolves at 4019ms |
turbo run fmt:check lint typecheck --filter=@browserbasehq/stagehand-extensionpasses. The extension build passes.- Extension unit tests: 1184 passed. 5 failures in
tests/browserbase-transport.test.ts(backoff timeouts) fail the same way onmainlocally, so they're unrelated.
(mirror-only) follow-up commit
Commit 2cb08745a addresses the review notes:
awaitNavigationCommandtakes the CDP method name, so a command that never answers reportsPage.navigate timed out after <timeout>ms(orPage.reload/Page.navigateToHistoryEntry) instead ofNavigation start timed out, which pointed at lifecycle events.goBack/goForwardboundPage.getNavigationHistoryby the sametimeout. Before this, a hung history lookup left them pending forever, even though the changeset says all four are bounded.LifecycleWatchertakes an optionalstartTimeso the lookup and the navigation share one deadline.- New tests: hung
Page.reload, hungPage.navigateToHistoryEntry(goBack and goForward), and hungPage.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 onmain.
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