chore: dispose-based callback termination (#27911)

docs/src/api/class-apirequestcontext.md

@@ -180,12 +180,7 @@ context cookies from the response. The method will automatically follow redirect
 ## async method: APIRequestContext.dispose
 * since: v1.16
 
-All responses returned by [`method: APIRequestContext.get`] and similar methods are stored in the memory, so that you can later call [`method: APIResponse.body`]. This method
-discards all stored responses, and makes [`method: APIResponse.body`] throw "Response disposed" error.
-
-If this [APIRequestContext] is obtained via [`property: BrowserContext.request`] or [`property: Page.request`], it will keep working until its owning [BrowserContext] closes.
-
-If this [APIRequestContext] was created by [`method: APIRequest.newContext`], this method discards all its resources, calling any method on disposed [APIRequestContext] will throw an exception.
+All responses returned by [`method: APIRequestContext.get`] and similar methods are stored in the memory, so that you can later call [`method: APIResponse.body`].This method discards all its resources, calling any method on disposed [APIRequestContext] will throw an exception.
 
 ## async method: APIRequestContext.fetch
 * since: v1.16

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

@@ -21,6 +21,7 @@ import { StreamDispatcher } from './streamDispatcher';
 import fs from 'fs';
 import { mkdirIfNeeded } from '../../utils/fileUtils';
 import type { Artifact } from '../artifact';
+import type { CallMetadata } from '../instrumentation';
 
 export class ArtifactDispatcher extends Dispatcher<Artifact, channels.ArtifactChannel, DispatcherScope> implements channels.ArtifactChannel {
   _type_Artifact = true;
@@ -105,7 +106,8 @@ export class ArtifactDispatcher extends Dispatcher<Artifact, channels.ArtifactCh
     await this._object.cancel();
   }
 
-  async delete(): Promise<void> {
+  async delete(_: any, metadata: CallMetadata): Promise<void> {
+    metadata.closesScope = true;
     await this._object.delete();
     this._dispose();
   }

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

@@ -273,6 +273,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
   }
 
   async close(params: channels.BrowserContextCloseParams, metadata: CallMetadata): Promise<void> {
+    metadata.closesScope = true;
     await this._context.close(params);
   }
 

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

@@ -55,11 +55,13 @@ export class BrowserDispatcher extends Dispatcher<Browser, channels.BrowserChann
     await this._object.stopPendingOperations(params.reason);
   }
 
-  async close(params: channels.BrowserCloseParams): Promise<void> {
+  async close(params: channels.BrowserCloseParams, metadata: CallMetadata): Promise<void> {
+    metadata.closesScope = true;
     await this._object.close(params);
   }
 
-  async killForTests(): Promise<void> {
+  async killForTests(_: any, metadata: CallMetadata): Promise<void> {
+    metadata.closesScope = true;
     await this._object.killForTests();
   }
 

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

@@ -19,6 +19,7 @@ import type * as channels from '@protocol/channels';
 import { Dispatcher } from './dispatcher';
 import type { BrowserDispatcher } from './browserDispatcher';
 import type { BrowserContextDispatcher } from './browserContextDispatcher';
+import type { CallMetadata } from '../instrumentation';
 
 export class CDPSessionDispatcher extends Dispatcher<CDPSession, channels.CDPSessionChannel, BrowserDispatcher | BrowserContextDispatcher> implements channels.CDPSessionChannel {
   _type_CDPSession = true;
@@ -33,7 +34,8 @@ export class CDPSessionDispatcher extends Dispatcher<CDPSession, channels.CDPSes
     return { result: await this._object.send(params.method as any, params.params) };
   }
 
-  async detach(): Promise<void> {
-    return this._object.detach();
+  async detach(_: any, metadata: CallMetadata): Promise<void> {
+    metadata.closesScope = true;
+    await this._object.detach();
   }
 }

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

@@ -17,7 +17,7 @@
 import { EventEmitter } from 'events';
 import type * as channels from '@protocol/channels';
 import { findValidator, ValidationError, createMetadataValidator, type ValidatorContext } from '../../protocol/validator';
-import { assert, isUnderTest, monotonicTime, rewriteErrorMessage } from '../../utils';
+import { LongStandingScope, assert, isUnderTest, monotonicTime, rewriteErrorMessage } from '../../utils';
 import { TargetClosedError, isTargetClosedError, serializeError } from '../errors';
 import type { CallMetadata } from '../instrumentation';
 import { SdkObject } from '../instrumentation';
@@ -51,6 +51,7 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
   readonly _guid: string;
   readonly _type: string;
   _object: Type;
+  private _openScope = new LongStandingScope();
 
   constructor(parent: ParentScopeType | DispatcherConnection, object: Type, type: string, initializer: channels.InitializerTraits<Type>) {
     super();
@@ -93,6 +94,17 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
     this._connection.sendAdopt(this, child);
   }
 
+  async _handleCommand(callMetadata: CallMetadata, method: string, validParams: any) {
+    const commandPromise = (this as any)[method](validParams, callMetadata);
+    try {
+      return await this._openScope.race(commandPromise);
+    } catch (e) {
+      if (callMetadata.closesScope && isTargetClosedError(e))
+        return await commandPromise;
+      throw e;
+    }
+  }
+
   _dispatchEvent<T extends keyof channels.EventsTraits<ChannelType>>(method: T, params?: channels.EventsTraits<ChannelType>[T]) {
     if (this._disposed) {
       if (isUnderTest())
@@ -105,14 +117,14 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
   }
 
   _dispose(reason?: 'gc') {
-    this._disposeRecursively();
+    this._disposeRecursively(new TargetClosedError());
     this._connection.sendDispose(this, reason);
   }
 
   protected _onDispose() {
   }
 
-  private _disposeRecursively() {
+  private _disposeRecursively(error: Error) {
     assert(!this._disposed, `${this._guid} is disposed more than once`);
     this._onDispose();
     this._disposed = true;
@@ -126,9 +138,10 @@ export class Dispatcher<Type extends { guid: string }, ChannelType, ParentScopeT
 
     // Dispose all children.
     for (const dispatcher of [...this._dispatchers.values()])
-      dispatcher._disposeRecursively();
+      dispatcher._disposeRecursively(error);
     this._dispatchers.clear();
     delete (this._object as any)[dispatcherSymbol];
+    this._openScope.close(error);
   }
 
   _debugScopeState(): any {
@@ -325,7 +338,7 @@ export class DispatcherConnection {
 
     await sdkObject?.instrumentation.onBeforeCall(sdkObject, callMetadata);
     try {
-      const result = await (dispatcher as any)[method](validParams, callMetadata);
+      const result = await dispatcher._handleCommand(callMetadata, method, validParams);
       const validator = findValidator(dispatcher._type, method, 'Result');
       callMetadata.result = validator(result, '', { tChannelImpl: this._tChannelImplToWire.bind(this), binary: this._isLocal ? 'buffer' : 'toBase64' });
     } catch (e) {

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

@@ -265,6 +265,7 @@ export class FrameDispatcher extends Dispatcher<Frame, channels.FrameChannel, Br
   }
 
   async expect(params: channels.FrameExpectParams, metadata: CallMetadata): Promise<channels.FrameExpectResult> {
+    metadata.closesScope = true;
     const expectedValue = params.expectedValue ? parseArgument(params.expectedValue) : undefined;
     const result = await this._frame.expect(metadata, params.selector, { ...params, expectedValue });
     if (result.received !== undefined)

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

@@ -22,6 +22,7 @@ import { parseSerializedValue, serializeValue } from '../../protocol/serializers
 import type { PageDispatcher, WorkerDispatcher } from './pageDispatcher';
 import type { ElectronApplicationDispatcher } from './electronDispatcher';
 import type { FrameDispatcher } from './frameDispatcher';
+import type { CallMetadata } from '../instrumentation';
 
 export type JSHandleDispatcherParentScope = PageDispatcher | FrameDispatcher | WorkerDispatcher | ElectronApplicationDispatcher;
 
@@ -66,8 +67,10 @@ export class JSHandleDispatcher extends Dispatcher<js.JSHandle, channels.JSHandl
     return { count: await this._object.objectCount() };
   }
 
-  async dispose() {
-    await this._object.dispose();
+  async dispose(_: any, metadata: CallMetadata) {
+    metadata.closesScope = true;
+    this._object.dispose();
+    this._dispose();
   }
 }
 

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

@@ -15,7 +15,7 @@
  */
 
 import type * as channels from '@protocol/channels';
-import { APIRequestContext } from '../fetch';
+import type { APIRequestContext } from '../fetch';
 import type { CallMetadata } from '../instrumentation';
 import type { Request, Response, Route } from '../network';
 import { WebSocket } from '../network';
@@ -191,17 +191,17 @@ export class APIRequestContextDispatcher extends Dispatcher<APIRequestContext, c
       tracing,
     });
 
-    this.addObjectListener(APIRequestContext.Events.Dispose, () => this._dispose());
-
     this.adopt(tracing);
   }
 
-  async storageState(params?: channels.APIRequestContextStorageStateParams): Promise<channels.APIRequestContextStorageStateResult> {
+  async storageState(): Promise<channels.APIRequestContextStorageStateResult> {
     return this._object.storageState();
   }
 
-  async dispose(params?: channels.APIRequestContextDisposeParams): Promise<void> {
+  async dispose(_: channels.APIRequestContextDisposeParams, metadata: CallMetadata): Promise<void> {
+    metadata.closesScope = true;
     await this._object.dispose();
+    this._dispose();
   }
 
   async fetch(params: channels.APIRequestContextFetchParams, metadata: CallMetadata): Promise<channels.APIRequestContextFetchResult> {

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

@@ -200,6 +200,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
   }
 
   async close(params: channels.PageCloseParams, metadata: CallMetadata): Promise<void> {
+    if (!params.runBeforeUnload)
+      metadata.closesScope = true;
     await this._page.close(metadata, params);
   }
 

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

@@ -20,7 +20,7 @@ import type * as frames from './frames';
 import type * as types from './types';
 import type * as channels from '@protocol/channels';
 import { assert } from '../utils';
-import { LongStandingScope, ManualPromise } from '../utils/manualPromise';
+import { ManualPromise } from '../utils/manualPromise';
 import { SdkObject } from './instrumentation';
 import type { HeadersArray, NameValue } from '../common/types';
 import { APIRequestContext } from './fetch';
@@ -129,10 +129,6 @@ export class Request extends SdkObject {
     this._isFavicon = url.endsWith('/favicon.ico') || !!redirectedFrom?._isFavicon;
   }
 
-  private _targetClosedScope(): LongStandingScope {
-    return this._serviceWorker?.openScope || this._frame?._page.openScope || new LongStandingScope();
-  }
-
   _setFailureText(failureText: string) {
     this._failureText = failureText;
     this._waitForResponsePromise.resolve(null);
@@ -183,11 +179,11 @@ export class Request extends SdkObject {
   }
 
   async rawRequestHeaders(): Promise<HeadersArray> {
-    return this._overrides?.headers || this._targetClosedScope().race(this._rawRequestHeadersPromise);
+    return this._overrides?.headers || this._rawRequestHeadersPromise;
   }
 
   response(): PromiseLike<Response | null> {
-    return this._targetClosedScope().race(this._waitForResponsePromise);
+    return this._waitForResponsePromise;
   }
 
   _existingResponse(): Response | null {

packages/playwright-core/types/types.d.ts

@@ -15258,20 +15258,8 @@ export interface APIRequestContext {
    * All responses returned by
    * [apiRequestContext.get(url[, options])](https://playwright.dev/docs/api/class-apirequestcontext#api-request-context-get)
    * and similar methods are stored in the memory, so that you can later call
-   * [apiResponse.body()](https://playwright.dev/docs/api/class-apiresponse#api-response-body). This method discards all
-   * stored responses, and makes
-   * [apiResponse.body()](https://playwright.dev/docs/api/class-apiresponse#api-response-body) throw "Response disposed"
-   * error.
-   *
-   * If this {@link APIRequestContext} is obtained via
-   * [browserContext.request](https://playwright.dev/docs/api/class-browsercontext#browser-context-request) or
-   * [page.request](https://playwright.dev/docs/api/class-page#page-request), it will keep working until its owning
-   * {@link BrowserContext} closes.
-   *
-   * If this {@link APIRequestContext} was created by
-   * [apiRequest.newContext([options])](https://playwright.dev/docs/api/class-apirequest#api-request-new-context), this
-   * method discards all its resources, calling any method on disposed {@link APIRequestContext} will throw an
-   * exception.
+   * [apiResponse.body()](https://playwright.dev/docs/api/class-apiresponse#api-response-body).This method discards all
+   * its resources, calling any method on disposed {@link APIRequestContext} will throw an exception.
    */
   dispose(): Promise<void>;
 

packages/protocol/src/callMetadata.ts

@@ -42,4 +42,5 @@ export type CallMetadata = {
   objectId?: string;
   pageId?: string;
   frameId?: string;
+  closesScope?: boolean;
 };

tests/library/browsercontext-fetch.spec.ts

@@ -22,6 +22,7 @@ import { pipeline } from 'stream';
 import zlib from 'zlib';
 import { contextTest as it, expect } from '../config/browserTest';
 import { suppressCertificateWarning } from '../config/utils';
+import { kTargetClosedErrorMessage } from 'tests/config/errors';
 
 it.skip(({ mode }) => mode !== 'default');
 
@@ -1078,7 +1079,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('Request context disposed');
+  expect(error.message).toContain(kTargetClosedErrorMessage);
   await connectionClosed;
 });
 
@@ -1193,8 +1194,8 @@ it('should update host header on redirect', async ({ context, server }) => {
   expect((await reqPromise).headers.host).toBe(new URL(server.CROSS_PROCESS_PREFIX).host);
 });
 
-it('should keep working after dispose', async ({ context, server }) => {
+it('should not work after dispose', async ({ context, server }) => {
   it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/27822' });
   await context.request.dispose();
-  await expect(await context.request.get(server.EMPTY_PAGE)).toBeOK();
+  expect(await context.request.get(server.EMPTY_PAGE).catch(e => e.message)).toContain(kTargetClosedErrorMessage);
 });

tests/library/global-fetch.spec.ts

@@ -18,6 +18,7 @@ import os from 'os';
 import * as util from 'util';
 import { getPlaywrightVersion } from '../../packages/playwright-core/lib/utils/userAgent';
 import { expect, playwrightTest as it } from '../config/browserTest';
+import { kTargetClosedErrorMessage } from 'tests/config/errors';
 
 it.skip(({ mode }) => mode !== 'default');
 
@@ -226,7 +227,7 @@ it('should abort requests when context is disposed', async ({ playwright, server
   ]);
   for (const result of results.slice(0, -1)) {
     expect(result instanceof Error).toBeTruthy();
-    expect(result.message).toContain('Request context disposed');
+    expect(result.message).toContain(kTargetClosedErrorMessage);
   }
   await connectionClosed;
 });
@@ -242,7 +243,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('Request context disposed');
+  expect(result.message).toContain(kTargetClosedErrorMessage);
   await connectionClosed;
 });
 

tests/page/page-close.spec.ts

@@ -21,7 +21,7 @@ it.skip(({ isWebView2 }) => isWebView2, 'Page.close() is not supported in WebVie
 
 it('should close page with active dialog', async ({ page }) => {
   await page.setContent(`<button onclick="setTimeout(() => alert(1))">alert</button>`);
-  void page.click('button');
+  void page.click('button').catch(() => {});
   await page.waitForEvent('dialog');
   await page.close();
 });

tests/page/page-evaluate.spec.ts

@@ -452,7 +452,7 @@ it('should throw if underlying element was disposed', async ({ page }) => {
   await element.dispose();
   let error = null;
   await page.evaluate(e => e.textContent, element).catch(e => error = e);
-  expect(error.message).toContain('JSHandle is disposed');
+  expect(error.message).toContain('no object with guid');
 });
 
 it('should simulate a user gesture', async ({ page }) => {