Skip to content

fix(browser): swallow waitForEvent('download') timeout so failed clicks don't crash the host#40

Draft
caffeinum wants to merge 1 commit into
webllm:mainfrom
caffeinum:fix/click-download-unhandled-rejection
Draft

fix(browser): swallow waitForEvent('download') timeout so failed clicks don't crash the host#40
caffeinum wants to merge 1 commit into
webllm:mainfrom
caffeinum:fix/click-download-unhandled-rejection

Conversation

@caffeinum
Copy link
Copy Markdown
Contributor

Summary

In BrowserSession._click_element_node, when a downloads dir is configured we arm page.waitForEvent('download', { timeout: 5000 }) BEFORE calling performClick(), then await it after. If performClick() throws (e.g. element-not-clickable, click intercepted by overlay, nested iframe issues), the await never executes and the waitForEvent promise rejects 5s later as an unhandledRejection — which in a long-running Node host (the queue watcher, in our case) escapes upward and crashes the process.

Repro

Any SaaS dashboard with nested Stripe iframes — e.g. browser-use.com/settings. A click on a side-nav element is heuristically flagged as download-capable but is actually a route nav. The click throws (Playwright can't reach the inner element), the dangling 5s timer fires, and the host dies with unhandledRejection: page.waitForEvent: Timeout 5000ms exceeded.

Fix

One line: downloadPromise.catch(() => null); immediately after creation. This marks the rejection as observed without changing semantics — the real await this._withAbort(downloadPromise, signal) below still consumes the outcome and the existing try/catch still handles the timeout-as-no-download case. Matches the parity behavior at the second download-wait site in perform_click.

Reference: Python upstream

Python browser-use doesn't have this bug — it uses CDP-driven download detection in browser_use/browser/watchdogs/default_action_watchdog.py::_execute_click_with_download_detection. The pattern is:

download_started = asyncio.Event()
downloads_watchdog.register_download_callbacks(on_start=..., on_progress=..., on_complete=...)
try:
    click_metadata = await click_coro                          # if this throws,
    await asyncio.wait_for(download_started.wait(), timeout=0.5)  # this is never created
finally:
    downloads_watchdog.unregister_download_callbacks(...)

asyncio.Event.wait() is created after the click resolves, so a click failure can't leave a dangling awaitable. The TS port translated this into a Playwright waitForEvent pattern that arms the future before the click — introducing the regression. This PR is the minimum patch to make the TS pattern safe; a deeper refactor to mirror python's CDP-driven approach is out of scope.

Test plan

  • Added test/browser-session.test.ts case that mocks a failing click + verifies no unhandledRejection escapes
  • Manual repro on a page with nested iframe download-capable side-nav

🤖 Generated with Claude Code

Most clicks don't trigger a file download. The unawaited
page.waitForEvent('download', { timeout: 5000 }) rejects on timeout,
escaping as an unhandledRejection that kills the whole watcher
process. Attach a no-op .catch() so the timeout is treated as
"no download, proceed" — matching the existing perform_click
behavior at the second download-wait call site (parity, not a new
heuristic).

Repro: any SaaS dashboard with nested Stripe iframes (e.g.
browser-use.com/settings) where a side-nav click is heuristically
flagged as download-capable but is actually a route nav. Crash
visible in queue logs as
"unhandledRejection: page.waitForEvent: Timeout 5000ms exceeded".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant