diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 788179e985121..b99265052cb71 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -234,7 +234,7 @@ export abstract class BrowserContext 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; } diff --git a/packages/playwright-core/src/server/chromium/crConnection.ts b/packages/playwright-core/src/server/chromium/crConnection.ts index 54a776c5639cf..645e19eee3b2f 100644 --- a/packages/playwright-core/src/server/chromium/crConnection.ts +++ b/packages/playwright-core/src/server/chromium/crConnection.ts @@ -126,6 +126,7 @@ export class CRSession extends SdkObject _markAsCrashed() { this._crashed = true; + this._rejectPendingCallbacks(`Page crashed.`); } createChildSession(sessionId: string, eventListener?: SessionEventListener): CRSession { @@ -187,8 +188,12 @@ export class CRSession extends SdkObject 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); diff --git a/packages/playwright-core/src/server/dialog.ts b/packages/playwright-core/src/server/dialog.ts index dba5b7b51ef41..f6c2cb5270328 100644 --- a/packages/playwright-core/src/server/dialog.ts +++ b/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); diff --git a/packages/playwright-core/src/server/firefox/ffConnection.ts b/packages/playwright-core/src/server/firefox/ffConnection.ts index 9815e42a7659c..b846df7adaba7 100644 --- a/packages/playwright-core/src/server/firefox/ffConnection.ts +++ b/packages/playwright-core/src/server/firefox/ffConnection.ts @@ -120,14 +120,24 @@ export class FFSession extends EventEmitter { markAsCrashed() { this._crashed = true; + this._rejectPendingCallbacks(); } async send( method: T, params?: Protocol.CommandParameters[T] ): Promise { - 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( + method: T, + params?: Protocol.CommandParameters[T] + ): Promise { + 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 { 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; diff --git a/packages/playwright-core/src/server/firefox/ffPage.ts b/packages/playwright-core/src/server/firefox/ffPage.ts index f291c5f6fbd7f..aa3da24b31fa8 100644 --- a/packages/playwright-core/src/server/firefox/ffPage.ts +++ b/packages/playwright-core/src/server/firefox/ffPage.ts @@ -458,7 +458,7 @@ export class FFPage implements PageDelegate { } async closePage(runBeforeUnload: boolean): Promise { - 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 { diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index 31fdaca340b9e..00f6c617ac812 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/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 { _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 { 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); diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index c26d5d39ddb1a..1038e1301383e 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -166,13 +166,12 @@ export type PageError = { export class Page extends SdkObject { static Events = PageEvent; - private _closedState: 'open' | 'closing' | 'closed' = 'open'; + private _lifecycle: 'open' | 'crashed' | 'closing' | 'closed' = 'open'; readonly closedPromise = new ManualPromise(); private _initialized: Page | Error | undefined; private _initializedPromise = new ManualPromise(); 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 { } 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 { 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 { } async onBindingCalled(payload: string, context: dom.FrameExecutionContext) { - if (this._closedState === 'closed') + if (this.isClosedOrClosingOrCrashed()) return; await PageBinding.dispatch(this, payload, context); } @@ -816,7 +820,7 @@ export class Page extends SdkObject { } private async _close(options: { reason?: string } = {}) { - if (this._closedState === 'closed') + if (this._lifecycle === 'closed') return; if (options.reason) @@ -824,8 +828,8 @@ export class Page extends SdkObject { 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 { } 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) { diff --git a/packages/playwright-core/src/server/webkit/wkPage.ts b/packages/playwright-core/src/server/webkit/wkPage.ts index 37813bcf70269..0a5e547aed4a5 100644 --- a/packages/playwright-core/src/server/webkit/wkPage.ts +++ b/packages/playwright-core/src/server/webkit/wkPage.ts @@ -835,6 +835,14 @@ export class WKPage implements PageDelegate { } async closePage(runBeforeUnload: boolean): Promise { + 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 diff --git a/tests/library/page-event-crash.spec.ts b/tests/library/page-event-crash.spec.ts index b789136a98af6..5560049b60d8f 100644 --- a/tests/library/page-event-crash.spec.ts +++ b/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(`
This page should crash
`); 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(`
This page should crash
`); + 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'); +}); diff --git a/tests/page/expect-misc.spec.ts b/tests/page/expect-misc.spec.ts index 9ae86bbee5032..a118d402717b2 100644 --- a/tests/page/expect-misc.spec.ts +++ b/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(`
hello
`); + 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"'); +});