chore: do not abort operations upon dispatcher being disposed

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);

packages/playwright-core/src/server/dispatchers/dispatcher.ts

@@ -144,11 +144,6 @@ export class Dispatcher<Type extends SdkObject, ChannelType, ParentScopeType ext
 
   private _disposeRecursively(error: Error) {
     assert(!this._disposed, `${this._guid} is disposed more than once`);
-    for (const controller of this._activeProgressControllers) {
-      const metainfo = getMetainfo({ type: controller.metadata.type, method: controller.metadata.method });
-      if (!metainfo?.potentiallyClosesScope)
-        controller.abort(error).catch(() => {});
-    }
     this._onDispose();
     this._disposed = true;
     eventsHelper.removeEventListeners(this._eventListeners);

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<string, string[]> = new Map();
   protected static allInstances: Set<APIRequestContext> = 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<channels.NetworkCookie[]>;
 
   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<SendRequestResult>((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();
           })
       );

packages/playwright-core/src/server/frames.ts

@@ -1447,7 +1447,17 @@ export class Frame extends SdkObject<FrameEventMap> {
   }
 
   async waitForTimeout(progress: Progress, timeout: number) {
-    return progress.wait(timeout);
+    let timer: NodeJS.Timeout;
+    const promise = new Promise<void>(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<ExpectResult> {

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<T>(progress: Progress, promise: Promise<T>): Promise<T> {
+    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<HeadersArray> {
-    return await progress.race(this._rawRequestHeaders());
+    return await this.raceWithPageClosure(progress, this._rawRequestHeaders());
   }
 
   async response(progress: Progress): Promise<Response | null> {
-    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<Buffer> {
-    return await progress.race(this.internalBody());
+    return await this._request.raceWithPageClosure(progress, this.internalBody());
   }
 
   async securityDetails(progress: Progress): Promise<SecurityDetails | null> {
-    return await progress.race(this.internalSecurityDetails());
+    return await this._request.raceWithPageClosure(progress, this.internalSecurityDetails());
   }
 
   async serverAddr(progress: Progress): Promise<RemoteAddr | null> {
-    return (await progress.race(this._serverAddrPromise)) || null;
+    return (await this._request.raceWithPageClosure(progress, this._serverAddrPromise)) || null;
   }
 
   async rawResponseHeaders(progress: Progress): Promise<NameValue[]> {
-    return await progress.race(this._rawResponseHeadersPromise);
+    return await this._request.raceWithPageClosure(progress, this._rawResponseHeadersPromise);
   }
 
   async httpVersion(progress: Progress): Promise<string> {
-    return await progress.race(this._httpVersion());
+    return await this._request.raceWithPageClosure(progress, this._httpVersion());
   }
 
   async sizes(progress: Progress): Promise<ResourceSizes> {
-    return await progress.race(this._sizes());
+    return await this._request.raceWithPageClosure(progress, this._sizes());
   }
 
   _serverAddrFinished(addr?: RemoteAddr) {

packages/playwright-core/src/server/recorder.ts

@@ -301,7 +301,7 @@ export class Recorder extends EventEmitter<RecorderEventMap> 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;

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;
 });
 

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();
 });

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 });