Skip to content

fix(browser): dispose CDP listeners when tester disconnects (fix #10810) - #10884

Open
alohasick wants to merge 1 commit into
vitest-dev:v4from
alohasick:fix/browser-cdp-stale-listeners
Open

fix(browser): dispose CDP listeners when tester disconnects (fix #10810)#10884
alohasick wants to merge 1 commit into
vitest-dev:v4from
alohasick:fix/browser-cdp-stale-listeners

Conversation

@alohasick

Copy link
Copy Markdown

Description

Fixes a crash in browser mode where stale CDP event listeners cause the entire test run to abort mid-way.

Root cause: removeCDPHandler() in packages/browser/src/node/projectParent.ts only deleted the BrowserServerCDPHandler from its internal map when a tester's WebSocket disconnected — it never unregistered the listeners that handler had registered on the underlying Playwright CDPSession. Since a browser page/CDP session can outlive a single test file's connection, a listener registered by one file could still be attached when a later file triggered the same CDP event. The listener would then call tester.cdpEvent(...) on a birpc channel that was already closed, and birpc throws synchronously in that case ([birpc] rpc is closed, cannot call "cdpEvent"). That throw happens inside Playwright's internal event dispatch with nothing to catch it, so it crashes the whole run instead of just failing one test.

Fix: added BrowserServerCDPHandler.dispose(), which calls session.off() for every event the handler ever registered and clears its internal state. removeCDPHandler() now calls dispose() before removing the handler from the map, so no listener is left attached to the CDP session once a tester disconnects.

Resolves #10810

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time.
  • Includes a test that fails without this PR but passes with it (verified locally by reverting the fix and confirming the new test reproduces the exact crash).
  • No changes to pnpm-lock.yaml.
  • Please check Allow edits by maintainers.

Tests

  • Ran the tests with pnpm test:ci (targeted: test/core/test/browser-cdp-handler.test.ts and test/browser/specs/cdp.test.ts).

Documentation

  • N/A — internal bugfix, no public API change.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

Base branch note

This targets v4 directly (not main) so the fix ships in the next Vitest 4.x patch release.

AI disclosure

Claude Code assisted in diagnosing the root cause, implementing the fix, and writing the regression tests for this PR.


const { stderr, testTree } = await runInlineBrowserTests(
{
'a.test.ts': `

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would either of the tests here fail without the fix?

@@ -0,0 +1,68 @@
import type { CDPSession } from 'vitest/node'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests don't test anything, everything is mocked

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.

2 participants