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/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/fetch.ts b/packages/playwright-core/src/server/fetch.ts index 75e24cc3f8a75..b392e18d49f5e 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'; @@ -109,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) { @@ -147,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(); @@ -328,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) => { @@ -530,7 +536,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/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/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; 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(); }); 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 });