From 2611e0edcca914a527fa7793aad12e9c0d3c0ef5 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 18 May 2026 15:12:11 +0100 Subject: [PATCH 1/4] chore: do not abort operations upon dispatcher being disposed The server-side library should correctly resolve/reject any calls instead. --- .../playwright-core/src/server/browser.ts | 3 +++ .../src/server/browserContext.ts | 1 + .../src/server/dispatchers/dispatcher.ts | 5 ---- .../playwright-core/src/server/network.ts | 23 ++++++++++++------- tests/library/browsercontext-fetch.spec.ts | 2 +- tests/library/global-fetch.spec.ts | 3 +-- 6 files changed, 21 insertions(+), 16 deletions(-) diff --git a/packages/playwright-core/src/server/browser.ts b/packages/playwright-core/src/server/browser.ts index 0722c511ee41a..812d0f23df6ac 100644 --- a/packages/playwright-core/src/server/browser.ts +++ b/packages/playwright-core/src/server/browser.ts @@ -27,6 +27,7 @@ import { PlaywrightPipeServer } from '../remote/playwrightPipeServer'; import { PlaywrightWebSocketServer } from '../remote/playwrightWebSocketServer'; import { BrowserInfo, serverRegistry } from '../serverRegistry'; import { nullProgress } from './progress'; +import { TargetClosedError } from './errors'; import type * as types from './types'; import type { ProxySettings } from './types'; @@ -174,6 +175,8 @@ export abstract class Browser extends SdkObject { context.browserClosed(); if (this._defaultContext) this._defaultContext.browserClosed(); + for (const download of this._downloads.values()) + download.artifact.reportFinished(new TargetClosedError(undefined)); this.stopServer(nullProgress).catch(() => {}); this.emit(Browser.Events.Disconnected); this.instrumentation.onBrowserClose(this); diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 1124acb9f9faf..934202a047131 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -264,6 +264,7 @@ export abstract class BrowserContext extends Sdk } this._clientCertificatesProxy?.close().catch(() => {}); this.tracing.abort(); + this._debugger.resume(); if (this._isPersistentContext) this.onClosePersistent(); this._closePromiseFulfill!(new Error('Context closed')); diff --git a/packages/playwright-core/src/server/dispatchers/dispatcher.ts b/packages/playwright-core/src/server/dispatchers/dispatcher.ts index 7e91fc1e7f7e9..df40e458af6f5 100644 --- a/packages/playwright-core/src/server/dispatchers/dispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/dispatcher.ts @@ -144,11 +144,6 @@ export class Dispatcher {}); - } this._onDispose(); this._disposed = true; eventsHelper.removeEventListeners(this._eventListeners); diff --git a/packages/playwright-core/src/server/network.ts b/packages/playwright-core/src/server/network.ts index eafa7f097be68..ce78b508798aa 100644 --- a/packages/playwright-core/src/server/network.ts +++ b/packages/playwright-core/src/server/network.ts @@ -214,12 +214,19 @@ export class Request extends SdkObject { this._isFavicon = url.endsWith('/favicon.ico') || !!redirectedFrom?._isFavicon; } + async raceWithPageClosure(progress: Progress, promise: Promise): Promise { + const scope = this._serviceWorker?.openScope ?? this._frame?._page.openScope; + if (scope) + return await progress.race(scope.race(promise)); + return await progress.race(promise); + } + async rawRequestHeaders(progress: Progress): Promise { - return await progress.race(this._rawRequestHeaders()); + return await this.raceWithPageClosure(progress, this._rawRequestHeaders()); } async response(progress: Progress): Promise { - return await progress.race(this._waitForResponse()); + return await this.raceWithPageClosure(progress, this._waitForResponse()); } _setFailureText(failureText: string) { @@ -539,27 +546,27 @@ export class Response extends SdkObject { } async body(progress: Progress): Promise { - return await progress.race(this.internalBody()); + return await this._request.raceWithPageClosure(progress, this.internalBody()); } async securityDetails(progress: Progress): Promise { - return await progress.race(this.internalSecurityDetails()); + return await this._request.raceWithPageClosure(progress, this.internalSecurityDetails()); } async serverAddr(progress: Progress): Promise { - return (await progress.race(this._serverAddrPromise)) || null; + return (await this._request.raceWithPageClosure(progress, this._serverAddrPromise)) || null; } async rawResponseHeaders(progress: Progress): Promise { - return await progress.race(this._rawResponseHeadersPromise); + return await this._request.raceWithPageClosure(progress, this._rawResponseHeadersPromise); } async httpVersion(progress: Progress): Promise { - return await progress.race(this._httpVersion()); + return await this._request.raceWithPageClosure(progress, this._httpVersion()); } async sizes(progress: Progress): Promise { - return await progress.race(this._sizes()); + return await this._request.raceWithPageClosure(progress, this._sizes()); } _serverAddrFinished(addr?: RemoteAddr) { diff --git a/tests/library/browsercontext-fetch.spec.ts b/tests/library/browsercontext-fetch.spec.ts index 864f34fa51ac8..871e435b6df70 100644 --- a/tests/library/browsercontext-fetch.spec.ts +++ b/tests/library/browsercontext-fetch.spec.ts @@ -1256,7 +1256,7 @@ it('should abort requests when browser context closes', async ({ contextFactory, server.waitForRequest('/empty.html').then(() => context.close()) ]); expect(error instanceof Error).toBeTruthy(); - expect(error.message).toContain(kTargetClosedErrorMessage); + expect(error.message).toMatch(/Request context disposed|Target page, context or browser has been closed/); await connectionClosed; }); diff --git a/tests/library/global-fetch.spec.ts b/tests/library/global-fetch.spec.ts index c307d3d83cf1a..402f6d7368d92 100644 --- a/tests/library/global-fetch.spec.ts +++ b/tests/library/global-fetch.spec.ts @@ -18,7 +18,6 @@ import os from 'os'; import * as util from 'util'; import { getPlaywrightVersion } from '../../packages/playwright-core/lib/coreBundle'; import { expect, playwrightTest as base } from '../config/browserTest'; -import { kTargetClosedErrorMessage } from '../config/errors'; const it = base.extend({ context: async ({}, use) => { @@ -324,7 +323,7 @@ it('should abort redirected requests when context is disposed', async ({ playwri server.waitForRequest('/test').then(() => request.dispose()) ]); expect(result instanceof Error).toBeTruthy(); - expect(result.message).toContain(kTargetClosedErrorMessage); + expect(result.message).toMatch(/Request context disposed|Target page, context or browser has been closed/); await connectionClosed; await request.dispose(); }); From 33746f20514decdc43f708dd4f2878f2fc4daa0c Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 19 May 2026 12:37:35 +0100 Subject: [PATCH 2/4] chore: do not resume debugger upon context close The compilation cache fix and the server-side resolve/reject behavior already handle resuming a paused debugger when the context closes, so the explicit resume() call is unnecessary. --- packages/playwright-core/src/server/browserContext.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 934202a047131..1124acb9f9faf 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -264,7 +264,6 @@ export abstract class BrowserContext extends Sdk } this._clientCertificatesProxy?.close().catch(() => {}); this.tracing.abort(); - this._debugger.resume(); if (this._isPersistentContext) this.onClosePersistent(); this._closePromiseFulfill!(new Error('Context closed')); From faa85c80900f6ae499182b4520a8929b2b67444e Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 19 May 2026 14:53:04 +0100 Subject: [PATCH 3/4] chore: resolve pickLocator, waitForTimeout and route.fetch on close These operations relied on dispatcher disposal aborting in-flight progress. Make the server-side library terminate them itself: - pickLocator races against page.openScope - waitForTimeout races against page close / frame detach - request context dispose rejects with a TargetClosedError --- packages/playwright-core/src/server/fetch.ts | 3 ++- packages/playwright-core/src/server/frames.ts | 12 +++++++++++- packages/playwright-core/src/server/recorder.ts | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index 75e24cc3f8a75..a20b34fdb1608 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -31,6 +31,7 @@ import { getUserAgent } from './userAgent'; import { BrowserContext, verifyClientCertificates } from './browserContext'; import { Cookie, CookieStore, domainMatches, parseRawCookie } from './cookieStore'; import { MultipartFormData } from './formData'; +import { TargetClosedError } from './errors'; import { SdkObject } from './instrumentation'; import { isAbortError } from './progress'; import { getMatchingTLSOptionsForOrigin, rewriteOpenSSLErrorIfNeeded } from './socksClientCertificatesInterceptor'; @@ -530,7 +531,7 @@ export abstract class APIRequestContext extends SdkObject { listeners.push( eventsHelper.addEventListener(this, APIRequestContext.Events.Dispose, () => { - reject(new Error('Request context disposed.')); + reject(new TargetClosedError(this._closeReason || 'Request context disposed.')); request.destroy(); }) ); diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 4afb08a9d07d0..3ec233045373b 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -1447,7 +1447,17 @@ export class Frame extends SdkObject { } async waitForTimeout(progress: Progress, timeout: number) { - return progress.wait(timeout); + let timer: NodeJS.Timeout; + const promise = new Promise(f => timer = setTimeout(f, timeout)); + try { + // Make sure we react to page close or frame detach. + await progress.race(LongStandingScope.raceMultiple([ + this._page.openScope, + this._detachedScope, + ], promise)); + } finally { + clearTimeout(timer!); + } } async expect(progress: Progress, selector: string | undefined, options: FrameExpectParams): Promise { diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index e67f56ac505c3..43ad5d0df1458 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -301,7 +301,7 @@ export class Recorder extends EventEmitter implements Instrume await this.setMode('inspecting'); return await selectorPromise; }; - return await progress.race(doPickLocator()); + return await progress.race(page.openScope.race(doPickLocator())); } finally { eventsHelper.removeEventListeners(listeners); this._pickLocatorPage = undefined; From e7a9ea6542b9f92cbc72de1cd2261507ea594412 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 20 May 2026 13:42:19 +0100 Subject: [PATCH 4/4] chore: reject route.fetch after request context is disposed The previous commit relied on dispatcher disposal aborting in-flight progress to make late route.fetch() calls reject with TargetClosedError. With that aborting removed, a request sent before _sendRequest attached its Dispose listener could either complete successfully or hang. Track _disposed on APIRequestContext and reject in _sendRequest right after _updateRequestCookieHeader so that any request reaching the HTTP layer after disposal fails with TargetClosedError. Also make the playwright.fetch unrouteAll-hint test deterministic by waiting for the page to actually close before invoking the late fetch. --- packages/playwright-core/src/server/fetch.ts | 5 +++++ tests/playwright-test/playwright.fetch.spec.ts | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index a20b34fdb1608..b392e18d49f5e 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -110,6 +110,7 @@ export abstract class APIRequestContext extends SdkObject { readonly fetchLog: Map = new Map(); protected static allInstances: Set = new Set(); _closeReason: string | undefined; + private _disposed = false; static findResponseBody(guid: string): Buffer | undefined { for (const request of APIRequestContext.allInstances) { @@ -148,6 +149,7 @@ export abstract class APIRequestContext extends SdkObject { abstract cookies(progress: Progress, url: URL): Promise; protected _disposeImpl() { + this._disposed = true; APIRequestContext.allInstances.delete(this); this.fetchResponses.clear(); this.fetchLog.clear(); @@ -329,6 +331,9 @@ export abstract class APIRequestContext extends SdkObject { }; this.emit(APIRequestContext.Events.Request, requestEvent); + if (this._disposed) + throw new TargetClosedError(this._closeReason || 'Request context disposed.'); + let destroyRequest: (() => void) | undefined; progress.setAllowConcurrentOrNestedRaces(true); const resultPromise = new Promise((fulfill, reject) => { diff --git a/tests/playwright-test/playwright.fetch.spec.ts b/tests/playwright-test/playwright.fetch.spec.ts index 3b2e32af153db..8fe962218fc78 100644 --- a/tests/playwright-test/playwright.fetch.spec.ts +++ b/tests/playwright-test/playwright.fetch.spec.ts @@ -91,6 +91,8 @@ test('should hint unrouteAll if failed in the handler', async ({ runInlineTest, await page.route('**/empty.html', async route => { await route.continue(); await closedPromise; + // Wait for the page to actually close before calling route.fetch(). + await new Promise(f => setTimeout(f, 500)); await route.fetch(); }); await page.goto('${server.EMPTY_PAGE}'); @@ -99,7 +101,7 @@ test('should hint unrouteAll if failed in the handler', async ({ runInlineTest, test('second test', async ({ page }) => { // Wait enough for the worker to be killed. - await new Promise(f => setTimeout(f, 1000)); + await new Promise(f => setTimeout(f, 2000)); }); `, }, { workers: 1 });