feat(test runner): cancel operations on test end via default abort signal

Inject a default AbortSignal that fires on test end into every client
operation, instead of relying on closing the context to unblock
in-flight operations.

Author: Skn0tt

packages/isomorphic/abortSignal.ts

@@ -18,3 +18,28 @@ export function assertionAbortedMessage(reason: unknown): string {
   const detail = reason instanceof Error ? reason.message : reason === undefined || reason === null ? '' : String(reason);
   return 'The assertion was aborted' + (detail ? `: ${detail}` : '');
 }
+
+export function combineSignals(a?: AbortSignal, b?: AbortSignal): AbortSignal | undefined {
+  if (!a)
+    return b;
+  if (!b)
+    return a;
+  if (a.aborted)
+    return a;
+  if (b.aborted)
+    return b;
+
+  const controller = new AbortController();
+  const onA = () => onAbort(a);
+  const onB = () => onAbort(b);
+  const onAbort = (source: AbortSignal) => {
+    controller.abort(source.reason);
+    a.removeEventListener('abort', onA);
+    b.removeEventListener('abort', onB);
+  };
+  a.addEventListener('abort', onA, { once: true });
+  b.addEventListener('abort', onB, { once: true });
+  return controller.signal;
+}
+
+export class TestEndedError extends Error {}

packages/isomorphic/index.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+export * from './abortSignal';
 export * from './ariaSnapshot';
 export * from './assert';
 export * from './base64';

packages/playwright-core/src/client/channelOwner.ts

@@ -15,6 +15,7 @@
  */
 
 import { getMetainfo } from '@isomorphic/protocolMetainfo';
+import { combineSignals } from '@isomorphic/abortSignal';
 import { showInternalStackFrames, stringifyStackFrames } from '@utils/stackTrace';
 import { isUnderTest } from '@utils/debug';
 import { debugLogger } from '@utils/debugLogger';
@@ -154,16 +155,18 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
             return async (params: any, signal: AbortSignal | undefined) => {
               return await this._wrapApiCall(async apiZone => {
                 const validatedParams = validator(params, '', this._validatorToWireContext());
+                const def = this._connection._defaultOperationSignal;
+                const effectiveSignal = combineSignals(signal, def?.aborted ? undefined : def);
                 if (!apiZone.internal && !apiZone.reported) {
                   // Reporting/tracing/logging this api call for the first time.
                   apiZone.reported = true;
                   this._instrumentation.onApiCallBegin(apiZone, { type: this._type, method: prop, params });
                   logApiCall(this._logger, `=> ${apiZone.apiName} started`);
-                  return await this._connection.sendMessageToServer(this, prop, validatedParams, { ...apiZone, signal });
+                  return await this._connection.sendMessageToServer(this, prop, validatedParams, { ...apiZone, signal: effectiveSignal });
                 }
                 // Since this api call is either internal, or has already been reported/traced once,
                 // passing as internal.
-                return await this._connection.sendMessageToServer(this, prop, validatedParams, { internal: true, signal });
+                return await this._connection.sendMessageToServer(this, prop, validatedParams, { internal: true, signal: effectiveSignal });
               }, { internal });
             };
           }

packages/playwright-core/src/client/connection.ts

@@ -15,6 +15,7 @@
  */
 
 import colors from 'colors/safe';
+import { TestEndedError } from '@isomorphic/abortSignal';
 import { rewriteErrorMessage } from '@utils/stackTrace';
 import { isUnderTest } from '@utils/debug';
 import { debugLogger } from '@utils/debugLogger';
@@ -87,6 +88,8 @@ export class Connection extends EventEmitter {
   readonly headers: HeadersArray;
   private _objectFactories = new Map<string, ChannelOwnerFactory>();
 
+  _defaultOperationSignal: AbortSignal | undefined;
+
   constructor(localUtils?: LocalUtils, instrumentation?: ClientInstrumentation, headers: HeadersArray = []) {
     super();
     this._instrumentation = instrumentation || createInstrumentation();
@@ -246,6 +249,9 @@ export class Connection extends EventEmitter {
         const parsedError = parseError(error);
         if (callback.signal?.aborted && parsedError instanceof AbortError)
           parsedError.cause = callback.signal.reason;
+        // hack to preserve error message strings from @playwright/test
+        if (parsedError.cause instanceof TestEndedError)
+          rewriteErrorMessage(parsedError, parsedError.cause.message);
         parsedError.log = log || [];
         rewriteErrorMessage(parsedError, parsedError.message + formatCallLog(log));
         const detailsValidator = maybeFindValidator(callback.type, callback.method, 'ErrorDetails');

packages/playwright-core/src/client/playwright.ts

@@ -42,6 +42,14 @@ export class Playwright extends ChannelOwner<channels.PlaywrightChannel> {
   _defaultContextTimeout?: number;
   _defaultContextNavigationTimeout?: number;
 
+  get _defaultOperationSignal(): AbortSignal | undefined {
+    return this._connection._defaultOperationSignal;
+  }
+
+  set _defaultOperationSignal(signal: AbortSignal | undefined) {
+    this._connection._defaultOperationSignal = signal;
+  }
+
   constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.PlaywrightInitializer) {
     super(parent, type, guid, initializer);
     this.request = new APIRequest(this);

packages/playwright/src/index.ts

@@ -384,9 +384,11 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures, UtilityTestFixt
 
     playwright._defaultContextTimeout = actionTimeout || 0;
     playwright._defaultContextNavigationTimeout = navigationTimeout || 0;
+    playwright._defaultOperationSignal = testInfo._operationAbortController.signal;
     await use();
     playwright._defaultContextTimeout = undefined;
     playwright._defaultContextNavigationTimeout = undefined;
+    playwright._defaultOperationSignal = undefined;
   }, { auto: 'all-hooks-included',  title: 'context configuration', box: true } as any],
 
   _contextFactory: [async ({ browser, video, _reuseContext, _combinedContextOptions /** mitigate dep-via-auto lack of traceability */ }, use, testInfo) => {
@@ -420,8 +422,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures, UtilityTestFixt
         if (closed)
           return;
         closed = true;
-        const closeReason = testInfo.status === 'timedOut' ? 'Test timeout of ' + testInfo.timeout + 'ms exceeded.' : 'Test ended.';
-        await context.close({ reason: closeReason });
+        await context.close();
         const preserveVideo = captureVideo && shouldPreserveVideo(videoMode, testInfo);
         if (preserveVideo) {
           const { pagesWithVideo: pagesForVideo } = contexts.get(context)!;

packages/playwright/src/worker/testInfo.ts

@@ -18,6 +18,7 @@ import fs from 'fs';
 import path from 'path';
 
 import { ManualPromise } from '@isomorphic/manualPromise';
+import { TestEndedError } from '@isomorphic/abortSignal';
 import { captureRawStack, stringifyStackFrames, filteredStackTrace } from '@utils/stackTrace';
 import { escapeWithQuotes } from '@isomorphic/stringUtils';
 import { monotonicTime } from '@isomorphic/time';
@@ -94,6 +95,7 @@ export class TestInfoImpl implements TestInfo {
   readonly _uniqueSymbol;
 
   private _interruptedPromise = new ManualPromise<void>();
+  readonly _operationAbortController = new AbortController();
   _lastStepId = 0;
   private readonly _requireFile: string;
   readonly _projectInternal: commonConfig.FullProjectInternal;
@@ -404,6 +406,11 @@ export class TestInfoImpl implements TestInfo {
     throw new TestAbortError('Test aborted' + (message ? ': ' + message : ''));
   }
 
+  _abortOperations(reason: string) {
+    if (!this._operationAbortController.signal.aborted)
+      this._operationAbortController.abort(new TestEndedError(reason));
+  }
+
   _interrupt() {
     // Mark as interrupted so we can ignore TimeoutError thrown by interrupt() call.
     this._interruptedPromise.resolve();

packages/playwright/src/worker/workerMain.ts

@@ -452,6 +452,9 @@ export class WorkerMain extends ProcessRunner {
 
       testInfo._tracing.didFinishTestFunctionAndAfterEachHooks();
 
+      const abortReason = testInfo.status === 'timedOut' ? 'Test timeout of ' + testInfo.timeout + 'ms exceeded.' : 'Test ended.';
+      testInfo._abortOperations(abortReason);
+
       try {
         // Teardown test-scoped fixtures. Attribute to 'test' so that users understand
         // they should probably increase the test timeout to fix this issue.

tests/playwright-test/test-step.spec.ts

@@ -630,14 +630,13 @@ pw:api    |      Launch browser
 pw:api    |    Create page @ a.test.ts:6
 pw:api    |Set content @ a.test.ts:15
 pw:api    |Click locator('div') @ a.test.ts:16
-pw:api    |↪ error: Error: page.click: Target page, context or browser has been closed
 hook      |After Hooks
 hook      |  afterAll hook @ a.test.ts:9
 pw:api    |    Close context @ a.test.ts:10
 hook      |Worker Cleanup
 fixture   |  Fixture "browser"
           |Test timeout of 2000ms exceeded.
-          |Error: page.click: Target page, context or browser has been closed
+          |AbortError: page.click: Test timeout of 2000ms exceeded.
 `);
 });