fix: handle Playwright CDP disconnect race condition
- Add safeSend() in cdp-relay.ts to catch errors when sending to closing WebSockets - Add safeCloseCDPBrowser() helper in test-utils.ts to drain message queue before close - Update vitest.setup.ts with documented suppression for Playwright's messageWrap race - Fix debugger test to await evaluate after resume Root cause: Playwright's messageWrap() schedules CDP message processing for next task, but browser.close() triggers _onClose() immediately, clearing callbacks before scheduled messages are processed. This is a Playwright internal issue we cannot fix.
This commit is contained in:
@@ -210,14 +210,31 @@ export async function startPlayWriterCDPRelayServer({
|
||||
|
||||
const messageStr = JSON.stringify(messageToSend)
|
||||
|
||||
// Helper to safely send to a WebSocket, catching errors from closing connections.
|
||||
// When a Playwright client closes its WebSocket, there's a race window where:
|
||||
// 1. Playwright's _onClose runs (clears callbacks map)
|
||||
// 2. We might still have messages in flight or try to send
|
||||
// This can cause "Assertion error" in Playwright's crConnection.js if a response
|
||||
// arrives after callbacks were cleared. We wrap in try-catch to handle this gracefully.
|
||||
const safeSend = (client: PlaywrightClient) => {
|
||||
try {
|
||||
client.ws.send(messageStr)
|
||||
} catch (e) {
|
||||
// WebSocket might be closing/closed - this is expected during disconnect
|
||||
logger?.log(pc.gray(`[Relay] Skipped sending to closing client ${client.id}: ${(e as Error).message}`))
|
||||
}
|
||||
}
|
||||
|
||||
if (clientId) {
|
||||
const client = playwrightClients.get(clientId)
|
||||
if (client) {
|
||||
client.ws.send(messageStr)
|
||||
safeSend(client)
|
||||
}
|
||||
} else {
|
||||
for (const client of playwrightClients.values()) {
|
||||
client.ws.send(messageStr)
|
||||
// Copy the clients array to avoid issues if a client disconnects during iteration
|
||||
const clients = Array.from(playwrightClients.values())
|
||||
for (const client of clients) {
|
||||
safeSend(client)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,7 +5,7 @@ import { getCdpUrl } from './utils.js'
|
||||
import { getCDPSessionForPage } from './cdp-session.js'
|
||||
import { Debugger } from './debugger.js'
|
||||
import { Editor } from './editor.js'
|
||||
import { setupTestContext, cleanupTestContext, getExtensionServiceWorker, createSseServer, type TestContext, withTimeout, js } from './test-utils.js'
|
||||
import { setupTestContext, cleanupTestContext, getExtensionServiceWorker, createSseServer, safeCloseCDPBrowser, type TestContext, withTimeout, js } from './test-utils.js'
|
||||
import './test-declarations.js'
|
||||
|
||||
const TEST_PORT = 19993
|
||||
@@ -992,9 +992,7 @@ describe('Service Worker Target Tests', () => {
|
||||
const title = await targetPage!.title()
|
||||
expect(title).toBeTruthy()
|
||||
|
||||
// Small delay to let pending CDP operations flush before closing
|
||||
await new Promise(r => setTimeout(r, 100))
|
||||
await browser.close()
|
||||
await safeCloseCDPBrowser(browser)
|
||||
await page.close()
|
||||
}, 60000)
|
||||
|
||||
|
||||
@@ -303,6 +303,29 @@ export function tryJsonParse(str: string) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Safely close a browser connected via connectOverCDP.
|
||||
*
|
||||
* Playwright's CRConnection uses async message handling (messageWrap) that can cause
|
||||
* a race condition where _onClose() runs before all pending _onMessage() handlers complete.
|
||||
* This results in "Assertion error" from crConnection.js when a CDP response arrives
|
||||
* after callbacks were cleared by dispose().
|
||||
*
|
||||
* This helper waits for the message queue to drain before closing, avoiding the race.
|
||||
*
|
||||
* @param browser - Browser instance from chromium.connectOverCDP()
|
||||
* @param drainDelayMs - Time to wait for pending messages to be processed (default: 50ms)
|
||||
*/
|
||||
export async function safeCloseCDPBrowser(
|
||||
browser: Awaited<ReturnType<typeof import('playwright-core').chromium.connectOverCDP>>,
|
||||
drainDelayMs = 50
|
||||
): Promise<void> {
|
||||
// Wait for any queued message handlers to run
|
||||
// This gives Playwright's messageWrap time to process pending CDP responses
|
||||
await new Promise(r => setTimeout(r, drainDelayMs))
|
||||
await browser.close()
|
||||
}
|
||||
|
||||
export type SimpleServer = {
|
||||
baseUrl: string
|
||||
close: () => Promise<void>
|
||||
|
||||
+31
-13
@@ -1,25 +1,43 @@
|
||||
/**
|
||||
* Vitest setup file - handles unhandled rejections from Playwright CDP cleanup.
|
||||
* Vitest setup - handles Playwright CDP disconnect race condition.
|
||||
*
|
||||
* When tests use connectOverCDP() to connect to Chrome, Playwright may have
|
||||
* pending CDP messages when browser.close() is called. These messages get
|
||||
* rejected when the connection closes, causing "Assertion error" from
|
||||
* Playwright's internal crConnection.js. This is expected cleanup behavior,
|
||||
* not a real error.
|
||||
* ROOT CAUSE (from Playwright source crConnection.ts:164):
|
||||
*
|
||||
* _onMessage(object: ProtocolResponse) {
|
||||
* if (object.id && this._callbacks.has(object.id)) {
|
||||
* // Handle response with matching callback
|
||||
* } else if (object.id && object.error?.code === -32001) {
|
||||
* // Closed session error - ignore
|
||||
* } else {
|
||||
* assert(!object.id); // ← FAILS: expects event, got orphaned response
|
||||
* }
|
||||
* }
|
||||
*
|
||||
* WHY IT HAPPENS:
|
||||
* 1. Relay sends CDP response to Playwright
|
||||
* 2. Playwright's messageWrap() schedules _onMessage for next task
|
||||
* 3. browser.close() is called
|
||||
* 4. _onClose() fires IMMEDIATELY and clears callbacks via dispose()
|
||||
* 5. Scheduled _onMessage finally runs
|
||||
* 6. Looks for callback → NOT FOUND → assertion fails
|
||||
*
|
||||
* This is a race condition in Playwright's async message handling that we cannot
|
||||
* fix without patching Playwright. The assertion error during disconnect is benign
|
||||
* and expected - it just means a CDP response arrived after we stopped caring.
|
||||
*
|
||||
* See: https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/server/chromium/crConnection.ts
|
||||
*/
|
||||
|
||||
process.on('unhandledRejection', (reason: any) => {
|
||||
// Suppress Playwright's internal assertion errors during CDP cleanup
|
||||
// These happen when browser.close() is called with pending CDP messages
|
||||
if (reason?.message === 'Assertion error' || reason?.name === 'AssertionError') {
|
||||
// Check if it's from Playwright's internal code
|
||||
// Check if this is Playwright's CDP disconnect assertion error
|
||||
if (reason?.message === 'Assertion error') {
|
||||
const stack = reason?.stack || ''
|
||||
if (stack.includes('crConnection.js') || stack.includes('crSession')) {
|
||||
// Silently ignore - this is expected cleanup behavior
|
||||
// Benign race condition during disconnect - suppress
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// Re-throw other unhandled rejections
|
||||
console.error('Unhandled rejection:', reason)
|
||||
// Re-throw other unhandled rejections to fail the test
|
||||
throw reason
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user