fix: do not hang API calls after a page crash

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

@@ -234,7 +234,7 @@ export abstract class BrowserContext<EM extends EventMap = EventMap> extends Sdk
     const otherPages = this.possiblyUninitializedPages().filter(p => p !== page);
     for (const p of otherPages)
       await p.close(progress);
-    if (page && page.hasCrashed()) {
+    if (page && page.isClosedOrClosingOrCrashed()) {
       await page.close(progress);
       page = undefined;
     }

packages/playwright-core/src/server/chromium/crConnection.ts

@@ -126,6 +126,7 @@ export class CRSession extends SdkObject<Protocol.EventMap & ConnectionEventMap>
 
   _markAsCrashed() {
     this._crashed = true;
+    this._rejectPendingCallbacks(`Page crashed.`);
   }
 
   createChildSession(sessionId: string, eventListener?: SessionEventListener): CRSession {
@@ -187,8 +188,12 @@ export class CRSession extends SdkObject<Protocol.EventMap & ConnectionEventMap>
   dispose() {
     this._closed = true;
     this._connection._sessions.delete(this._sessionId);
+    this._rejectPendingCallbacks(`Internal server error, session closed.`);
+  }
+
+  private _rejectPendingCallbacks(message: string) {
     for (const callback of this._callbacks.values()) {
-      callback.error.setMessage(`Internal server error, session closed.`);
+      callback.error.setMessage(message);
       callback.error.type = this._crashed ? 'crashed' : 'closed';
       callback.error.logs = this._connection._browserDisconnectedLogs;
       callback.reject(callback.error);

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

@@ -101,7 +101,7 @@ export class DialogManager {
   dialogDidOpen(dialog: Dialog) {
     // Any ongoing evaluations will be stalled until the dialog is closed.
     for (const frame of dialog.page().frameManager.frames())
-      frame.invalidateNonStallingEvaluations('JavaScript dialog interrupted evaluation');
+      frame.invalidateNonStallingEvaluations(new Error('JavaScript dialog interrupted evaluation'));
     this._openedDialogs.add(dialog);
     this._instrumentation.onDialog(dialog);
 

packages/playwright-core/src/server/firefox/ffConnection.ts

@@ -120,14 +120,24 @@ export class FFSession extends EventEmitter<Protocol.EventMap> {
 
   markAsCrashed() {
     this._crashed = true;
+    this._rejectPendingCallbacks();
   }
 
   async send<T extends keyof Protocol.CommandParameters>(
     method: T,
     params?: Protocol.CommandParameters[T]
   ): Promise<Protocol.CommandReturnValues[T]> {
-    if (this._crashed || this._disposed || this._connection._closed || this._connection._browserDisconnectedLogs)
-      throw new ProtocolError(this._crashed ? 'crashed' : 'closed', undefined, this._connection._browserDisconnectedLogs);
+    if (this._crashed)
+      throw new ProtocolError('crashed', undefined, this._connection._browserDisconnectedLogs);
+    return this.sendEvenAfterCrash(method, params);
+  }
+
+  async sendEvenAfterCrash<T extends keyof Protocol.CommandParameters>(
+    method: T,
+    params?: Protocol.CommandParameters[T]
+  ): Promise<Protocol.CommandReturnValues[T]> {
+    if (this._disposed || this._connection._closed || this._connection._browserDisconnectedLogs)
+      throw new ProtocolError('closed', undefined, this._connection._browserDisconnectedLogs);
     const id = this._connection.nextMessageId();
     this._rawSend({ method, params, id });
     return new Promise((resolve, reject) => {
@@ -160,6 +170,10 @@ export class FFSession extends EventEmitter<Protocol.EventMap> {
   dispose() {
     this._disposed = true;
     this._connection._sessions.delete(this._sessionId);
+    this._rejectPendingCallbacks();
+  }
+
+  private _rejectPendingCallbacks() {
     for (const callback of this._callbacks.values()) {
       callback.error.type = this._crashed ? 'crashed' : 'closed';
       callback.error.logs = this._connection._browserDisconnectedLogs;

packages/playwright-core/src/server/firefox/ffPage.ts

@@ -458,7 +458,7 @@ export class FFPage implements PageDelegate {
   }
 
   async closePage(runBeforeUnload: boolean): Promise<void> {
-    await this._session.send('Page.close', { runBeforeUnload });
+    await this._session.sendEvenAfterCrash('Page.close', { runBeforeUnload });
   }
 
   async setBackgroundColor(color?: { r: number; g: number; b: number; a: number; }): Promise<void> {

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

@@ -28,7 +28,7 @@ import { makeWaitForNextTask } from '@utils/task';
 import { createGuid } from '@utils/crypto';
 import { BrowserContext } from './browserContext';
 import * as dom from './dom';
-import { TimeoutError } from './errors';
+import { TimeoutError, isTargetClosedError } from './errors';
 import { prepareFilesForUpload } from './fileUploadUtils';
 import { FrameSelectors } from './frameSelectors';
 import { helper } from './helper';
@@ -139,10 +139,10 @@ export class FrameManager {
       this.frameAttached(kDummyFrameId, null);
   }
 
-  dispose() {
+  dispose(error: Error) {
     for (const frame of this._frames.values()) {
       frame._stopNetworkIdleTimer();
-      frame.invalidateNonStallingEvaluations('Target crashed');
+      frame.invalidateNonStallingEvaluations(error);
     }
   }
 
@@ -572,17 +572,16 @@ export class Frame extends SdkObject<FrameEventMap> {
   _setPendingDocument(documentInfo: DocumentInfo | undefined) {
     this._pendingDocument = documentInfo;
     if (documentInfo)
-      this.invalidateNonStallingEvaluations('Navigation interrupted the evaluation');
+      this.invalidateNonStallingEvaluations(new Error('Navigation interrupted the evaluation'));
   }
 
   pendingDocument(): DocumentInfo | undefined {
     return this._pendingDocument;
   }
 
-  invalidateNonStallingEvaluations(message: string) {
+  invalidateNonStallingEvaluations(error: Error) {
     if (!this._raceAgainstEvaluationStallingEventsPromises.size)
       return;
-    const error = new Error(message);
     for (const promise of this._raceAgainstEvaluationStallingEventsPromises)
       promise.reject(error);
   }
@@ -1551,6 +1550,8 @@ export class Frame extends SdkObject<FrameEventMap> {
         details.received = lastIntermediateResult.received;
         details.customErrorMessage = lastIntermediateResult.errorMessage;
       }
+      if (isTargetClosedError(e) || isSessionClosedError(e))
+        progress.log(e.message);
       if (e instanceof TimeoutError)
         details.timedOut = true;
       throw new ExpectError(details);

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

@@ -166,13 +166,12 @@ export type PageError = {
 export class Page extends SdkObject<PageEventMap> {
   static Events = PageEvent;
 
-  private _closedState: 'open' | 'closing' | 'closed' = 'open';
+  private _lifecycle: 'open' | 'crashed' | 'closing' | 'closed' = 'open';
   readonly closedPromise = new ManualPromise<void>();
   private _initialized: Page | Error | undefined;
   private _initializedPromise = new ManualPromise<Page | Error>();
   private _consoleMessages: ConsoleMessage[] = [];
   private _pageErrors: PageError[] = [];
-  private _crashed = false;
   readonly openScope = new LongStandingScope();
   readonly browserContext: BrowserContext;
   readonly keyboard: input.Keyboard;
@@ -228,7 +227,7 @@ export class Page extends SdkObject<PageEventMap> {
     }
     if (opener) {
       const openerPageOrError = await opener.waitForInitializedOrError();
-      if (openerPageOrError instanceof Page && !openerPageOrError.isClosed())
+      if (openerPageOrError instanceof Page && !openerPageOrError.isClosedOrClosingOrCrashed())
         this._opener = openerPageOrError;
     }
     this._markInitialized(error);
@@ -293,27 +292,32 @@ export class Page extends SdkObject<PageEventMap> {
     await this.delegate.resetForReuse(progress);
   }
 
-  _didClose() {
-    this.frameManager.dispose();
+  private _didDisconnect(error: TargetClosedError) {
+    if (this.openScope.isClosed())
+      return;
+    this.frameManager.dispose(error);
     this.screencast.dispose();
     this.overlay.dispose();
-    assert(this._closedState !== 'closed', 'Page closed twice');
-    this._closedState = 'closed';
+    this.openScope.close(error);
+  }
+
+  _didClose() {
+    if (this._lifecycle === 'closed')
+      return;
+    this._lifecycle = 'closed';
+    this._didDisconnect(new TargetClosedError(this.closeReason()));
     this.emit(Page.Events.Close);
     this.browserContext.emit(BrowserContext.Events.PageClosed, this);
     this.closedPromise.resolve();
     this.instrumentation.onPageClose(this);
-    this.openScope.close(new TargetClosedError(this.closeReason()));
   }
 
   _didCrash() {
-    this.frameManager.dispose();
-    this.screencast.dispose();
-    this.overlay.dispose();
+    if (this._lifecycle !== 'open')
+      return;
+    this._lifecycle = 'crashed';
+    this._didDisconnect(new TargetClosedError('Page crashed'));
     this.emit(Page.Events.Crash);
-    this._crashed = true;
-    this.instrumentation.onPageClose(this);
-    this.openScope.close(new Error('Page crashed'));
   }
 
   async _onFileChooserOpened(handle: dom.ElementHandle) {
@@ -398,7 +402,7 @@ export class Page extends SdkObject<PageEventMap> {
   }
 
   async onBindingCalled(payload: string, context: dom.FrameExecutionContext) {
-    if (this._closedState === 'closed')
+    if (this.isClosedOrClosingOrCrashed())
       return;
     await PageBinding.dispatch(this, payload, context);
   }
@@ -816,16 +820,16 @@ export class Page extends SdkObject<PageEventMap> {
   }
 
   private async _close(options: { reason?: string } = {}) {
-    if (this._closedState === 'closed')
+    if (this._lifecycle === 'closed')
       return;
 
     if (options.reason)
       this._closeReason = options.reason;
 
     await this.screencast.handlePageOrContextClose();
 
-    if (this._closedState !== 'closing') {
-      this._closedState = 'closing';
+    if (this._lifecycle !== 'closing') {
+      this._lifecycle = 'closing';
       // This might throw if the browser context containing the page closes
       // while we are trying to close the page.
       await this.delegate.closePage(false).catch(e => debugLogger.log('error', e));
@@ -844,15 +848,11 @@ export class Page extends SdkObject<PageEventMap> {
   }
 
   isClosed(): boolean {
-    return this._closedState === 'closed';
-  }
-
-  hasCrashed() {
-    return this._crashed;
+    return this._lifecycle === 'closed';
   }
 
   isClosedOrClosingOrCrashed() {
-    return this._closedState !== 'open' || this._crashed;
+    return this._lifecycle !== 'open';
   }
 
   addWorker(workerId: string, worker: Worker) {

packages/playwright-core/src/server/webkit/wkPage.ts

@@ -835,6 +835,14 @@ export class WKPage implements PageDelegate {
   }
 
   async closePage(runBeforeUnload: boolean): Promise<void> {
+    if (this._session.isDisposed()) {
+      // The page content target is gone (e.g. after a crash) and the orphaned page
+      // proxy cannot be destroyed via the protocol. Finalize the close locally so
+      // page.close() resolves and the page is reported closed.
+      // TODO: the proper fix is in the browser - destroy the orphaned page proxy.
+      this.didClose();
+      return;
+    }
     await this._pageProxySession.sendMayFail('Target.close', {
       targetId: this._session.sessionId,
       runBeforeUnload

tests/library/page-event-crash.spec.ts

@@ -46,17 +46,29 @@ test('should emit crash event when page crashes', async ({ page, crash }) => {
   expect(crashedPage).toBe(page);
 });
 
-test('should throw on any action after page crashes', async ({ page, crash, browserName }) => {
+test('should throw on any action after page crashes', async ({ page, crash, server, browserName }) => {
   await page.setContent(`<div>This page should crash</div>`);
   crash();
   await page.waitForEvent('crash');
-  const err = await page.evaluate(() => {}).then(() => null, e => e);
-  expect(err).toBeTruthy();
-  // In Firefox, crashed page is sometimes "closed".
-  if (browserName === 'firefox')
-    expect(err.message.includes('Target page, context or browser has been closed') || err.message.includes('Target crashed'), err.message).toBe(true);
-  else
-    expect(err.message).toContain('Target crashed');
+  const expectCrashError = (error: Error | null) => {
+    expect(error, 'action should reject after crash').toBeTruthy();
+    // In Firefox, crashed page is sometimes "closed".
+    if (browserName === 'firefox')
+      expect(error!.message.includes('has been closed') || error!.message.includes('crashed'), error!.message).toBe(true);
+    else
+      expect(error!.message).toContain('crashed');
+  };
+  expectCrashError(await page.evaluate(() => {}).then(() => null, e => e));
+  expectCrashError(await page.goto(server.EMPTY_PAGE).then(() => null, e => e));
+  expectCrashError(await page.reload().then(() => null, e => e));
+});
+
+test('expect should not hang when page crashed', {
+  annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/31907' },
+}, async ({ page, crash }) => {
+  const expectPromise = expect(page.getByText('child')).toBeVisible();
+  crash();
+  await expect(expectPromise).rejects.toThrowError();
 });
 
 test('should cancel waitForEvent when page crashes', async ({ page, crash }) => {
@@ -85,3 +97,24 @@ test('should be able to close context when page crashes', async ({ isAndroid, pa
   await page.waitForEvent('crash');
   await page.context().close();
 });
+
+test('should be able to close page after crash', async ({ page, crash }) => {
+  await page.setContent(`<div>This page should crash</div>`);
+  crash();
+  await page.waitForEvent('crash');
+  await page.close();
+  expect(page.isClosed()).toBe(true);
+});
+
+test.fixme('should reject in-flight worker.evaluate when page crashes', async ({ page, crash, server }) => {
+  await page.goto(server.EMPTY_PAGE);
+  const [worker] = await Promise.all([
+    page.waitForEvent('worker'),
+    page.evaluate(() => new Worker(URL.createObjectURL(
+        new Blob(['self.onmessage = () => {}'], { type: 'application/javascript' })))),
+  ]);
+  const evalPromise = worker.evaluate(() => new Promise(() => {})).catch((e: Error) => e); // never resolves in-worker
+  crash();
+  const error = await evalPromise as Error;
+  expect(error.message).toContain('crash');
+});

tests/page/expect-misc.spec.ts

@@ -664,3 +664,14 @@ Error: Unexpected token "#" while parsing css selector "##". Did you mean to CSS
 Call log:
 `);
 });
+
+test('should report expect error details when page closes during expect', async ({ page }) => {
+  await page.setContent(`<div>hello</div>`);
+  const promise = expect(page.locator('div')).toHaveText('world', { timeout: 10000 }).catch((e: Error) => e);
+  await page.waitForTimeout(1000);
+  await page.close();
+  const error = await promise as Error;
+  expect(stripAnsi(error.message)).toContain('expect(locator).toHaveText(expected) failed');
+  expect(stripAnsi(error.message)).toContain('Expected: "world"');
+  expect(stripAnsi(error.message)).toContain('Received: "hello"');
+});