From a54dbfdadf82d5bc201f816a419604557a207611 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 16 Oct 2023 20:32:13 -0700 Subject: [PATCH] chore: plumb the target close reason when test fails (#27640) --- docs/src/api/class-browser.md | 6 ++ docs/src/api/class-browsercontext.md | 6 ++ docs/src/api/class-page.md | 6 ++ .../playwright-core/src/client/browser.ts | 4 +- .../src/client/browserContext.ts | 4 +- .../playwright-core/src/protocol/validator.ts | 9 +- .../src/remote/playwrightConnection.ts | 10 +- .../src/remote/playwrightServer.ts | 2 +- .../playwright-core/src/server/browser.ts | 7 +- .../src/server/browserContext.ts | 9 +- .../src/server/chromium/crBrowser.ts | 4 +- .../src/server/debugController.ts | 6 +- .../dispatchers/browserContextDispatcher.ts | 2 +- .../server/dispatchers/browserDispatcher.ts | 7 +- .../src/server/dispatchers/dispatcher.ts | 16 +++- .../src/server/electron/electron.ts | 2 +- packages/playwright-core/src/server/fetch.ts | 1 + .../src/server/firefox/ffBrowser.ts | 4 +- packages/playwright-core/src/server/page.ts | 9 +- .../src/server/recorder/recorderApp.ts | 4 +- .../src/server/trace/viewer/traceViewer.ts | 2 +- .../src/server/webkit/wkBrowser.ts | 4 +- packages/playwright-core/types/types.d.ts | 95 +++++++++++-------- packages/playwright/src/index.ts | 10 +- packages/protocol/src/channels.ts | 22 +++-- packages/protocol/src/protocol.yml | 5 + tests/config/traceViewerFixtures.ts | 2 +- tests/playwright-test/playwright.spec.ts | 9 +- 28 files changed, 171 insertions(+), 96 deletions(-) diff --git a/docs/src/api/class-browser.md b/docs/src/api/class-browser.md index c52804dbe2..360b844d24 100644 --- a/docs/src/api/class-browser.md +++ b/docs/src/api/class-browser.md @@ -102,6 +102,12 @@ This is similar to force quitting the browser. Therefore, you should call [`meth The [Browser] object itself is considered to be disposed and cannot be used anymore. +### option: Browser.close.reason +* since: v1.40 +- `reason` <[string]> + +The reason to be reported to the operations interrupted by the browser closure. + ## method: Browser.contexts * since: v1.8 - returns: <[Array]<[BrowserContext]>> diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md index c135382d7c..ca58882471 100644 --- a/docs/src/api/class-browsercontext.md +++ b/docs/src/api/class-browsercontext.md @@ -94,6 +94,12 @@ Emitted when Browser context gets closed. This might happen because of one of th * Browser application is closed or crashed. * The [`method: Browser.close`] method was called. +### option: BrowserContext.close.reason +* since: v1.40 +- `reason` <[string]> + +The reason to be reported to the operations interrupted by the context closure. + ## event: BrowserContext.console * since: v1.34 * langs: diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index 2515820ab1..f4612acdbe 100644 --- a/docs/src/api/class-page.md +++ b/docs/src/api/class-page.md @@ -826,6 +826,12 @@ if [`option: runBeforeUnload`] is passed as true, a `beforeunload` dialog might manually via [`event: Page.dialog`] event. ::: +### option: Page.close.reason +* since: v1.40 +- `reason` <[string]> + +The reason to be reported to the operations interrupted by the page closure. + ### option: Page.close.runBeforeUnload * since: v1.8 - `runBeforeUnload` <[boolean]> diff --git a/packages/playwright-core/src/client/browser.ts b/packages/playwright-core/src/client/browser.ts index 3025dc2621..650e980952 100644 --- a/packages/playwright-core/src/client/browser.ts +++ b/packages/playwright-core/src/client/browser.ts @@ -130,12 +130,12 @@ export class Browser extends ChannelOwner implements ap return buffer; } - async close(): Promise { + async close(options: { reason?: string } = {}): Promise { try { if (this._shouldCloseConnectionOnClose) this._connection.close(); else - await this._channel.close(); + await this._channel.close(options); await this._closedPromise; } catch (e) { if (isTargetClosedError(e)) diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 2350f14040..c4664b1a36 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -383,7 +383,7 @@ export class BrowserContext extends ChannelOwner this.emit(Events.BrowserContext.Close, this); } - async close(): Promise { + async close(options: { reason?: string } = {}): Promise { if (this._closeWasCalled) return; this._closeWasCalled = true; @@ -404,7 +404,7 @@ export class BrowserContext extends ChannelOwner await artifact.delete(); } }, true); - await this._channel.close(); + await this._channel.close(options); await this._closedPromise; } diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index f3e26cbba2..706512bb9f 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -588,7 +588,9 @@ scheme.BrowserInitializer = tObject({ name: tString, }); scheme.BrowserCloseEvent = tOptional(tObject({})); -scheme.BrowserCloseParams = tOptional(tObject({})); +scheme.BrowserCloseParams = tObject({ + reason: tOptional(tString), +}); scheme.BrowserCloseResult = tOptional(tObject({})); scheme.BrowserKillForTestsParams = tOptional(tObject({})); scheme.BrowserKillForTestsResult = tOptional(tObject({})); @@ -831,7 +833,9 @@ scheme.BrowserContextClearCookiesParams = tOptional(tObject({})); scheme.BrowserContextClearCookiesResult = tOptional(tObject({})); scheme.BrowserContextClearPermissionsParams = tOptional(tObject({})); scheme.BrowserContextClearPermissionsResult = tOptional(tObject({})); -scheme.BrowserContextCloseParams = tOptional(tObject({})); +scheme.BrowserContextCloseParams = tObject({ + reason: tOptional(tString), +}); scheme.BrowserContextCloseResult = tOptional(tObject({})); scheme.BrowserContextCookiesParams = tObject({ urls: tArray(tString), @@ -1000,6 +1004,7 @@ scheme.PageAddInitScriptParams = tObject({ scheme.PageAddInitScriptResult = tOptional(tObject({})); scheme.PageCloseParams = tObject({ runBeforeUnload: tOptional(tBoolean), + reason: tOptional(tString), }); scheme.PageCloseResult = tOptional(tObject({})); scheme.PageEmulateMediaParams = tObject({ diff --git a/packages/playwright-core/src/remote/playwrightConnection.ts b/packages/playwright-core/src/remote/playwrightConnection.ts index 7e30e7a79a..7da1eefee2 100644 --- a/packages/playwright-core/src/remote/playwrightConnection.ts +++ b/packages/playwright-core/src/remote/playwrightConnection.ts @@ -116,7 +116,7 @@ export class PlaywrightConnection { this._cleanups.push(async () => { for (const browser of playwright.allBrowsers()) - await browser.close(); + await browser.close({ reason: 'Connection terminated' }); }); browser.on(Browser.Events.Disconnected, () => { // Underlying browser did close for some reason - force disconnect the client. @@ -143,7 +143,7 @@ export class PlaywrightConnection { // In pre-launched mode, keep only the pre-launched browser. for (const b of playwright.allBrowsers()) { if (b !== browser) - await b.close(); + await b.close({ reason: 'Connection terminated' }); } this._cleanups.push(() => playwrightDispatcher.cleanup()); return playwrightDispatcher; @@ -189,7 +189,7 @@ export class PlaywrightConnection { if (b === browser) continue; if (b.options.name === this._options.browserName && b.options.channel === this._options.launchOptions.channel) - await b.close(); + await b.close({ reason: 'Connection terminated' }); } if (!browser) { @@ -209,12 +209,12 @@ export class PlaywrightConnection { for (const browser of playwright.allBrowsers()) { for (const context of browser.contexts()) { if (!context.pages().length) - await context.close(serverSideCallMetadata()); + await context.close({ reason: 'Connection terminated' }); else await context.stopPendingOperations('Connection closed'); } if (!browser.contexts()) - await browser.close(); + await browser.close({ reason: 'Connection terminated' }); } }); diff --git a/packages/playwright-core/src/remote/playwrightServer.ts b/packages/playwright-core/src/remote/playwrightServer.ts index f907018bf0..cfccdb3fcf 100644 --- a/packages/playwright-core/src/remote/playwrightServer.ts +++ b/packages/playwright-core/src/remote/playwrightServer.ts @@ -197,7 +197,7 @@ export class PlaywrightServer { debugLogger.log('server', 'closing browsers'); if (this._preLaunchedPlaywright) - await Promise.all(this._preLaunchedPlaywright.allBrowsers().map(browser => browser.close())); + await Promise.all(this._preLaunchedPlaywright.allBrowsers().map(browser => browser.close({ reason: 'Playwright Server stopped' }))); debugLogger.log('server', 'closed browsers'); } } diff --git a/packages/playwright-core/src/server/browser.ts b/packages/playwright-core/src/server/browser.ts index 26c3e2f32e..711d68b06d 100644 --- a/packages/playwright-core/src/server/browser.ts +++ b/packages/playwright-core/src/server/browser.ts @@ -64,6 +64,7 @@ export abstract class Browser extends SdkObject { private _startedClosing = false; readonly _idToVideo = new Map(); private _contextForReuse: { context: BrowserContext, hash: string } | undefined; + _closeReason: string | undefined; constructor(parent: SdkObject, options: BrowserOptions) { super(parent, 'browser'); @@ -90,7 +91,7 @@ export abstract class Browser extends SdkObject { const hash = BrowserContext.reusableContextHash(params); if (!this._contextForReuse || hash !== this._contextForReuse.hash || !this._contextForReuse.context.canResetForReuse()) { if (this._contextForReuse) - await this._contextForReuse.context.close(metadata); + await this._contextForReuse.context.close({ reason: 'Context reused' }); this._contextForReuse = { context: await this.newContext(metadata, params), hash }; return { context: this._contextForReuse.context, needsReset: false }; } @@ -149,8 +150,10 @@ export abstract class Browser extends SdkObject { this.instrumentation.onBrowserClose(this); } - async close() { + async close(options: { reason?: string }) { if (!this._startedClosing) { + if (options.reason) + this._closeReason = options.reason; this._startedClosing = true; await this.options.browserProcess.close(); } diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index c1afc39683..3b7b41497b 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -86,6 +86,7 @@ export abstract class BrowserContext extends SdkObject { readonly initScripts: string[] = []; private _routesInFlight = new Set(); private _debugger!: Debugger; + _closeReason: string | undefined; constructor(browser: Browser, options: channels.BrowserNewContextParams, browserContextId: string | undefined) { super(browser, 'browser-context'); @@ -272,7 +273,7 @@ export abstract class BrowserContext extends SdkObject { protected abstract doExposeBinding(binding: PageBinding): Promise; protected abstract doRemoveExposedBindings(): Promise; protected abstract doUpdateRequestInterception(): Promise; - protected abstract doClose(): Promise; + protected abstract doClose(reason: string | undefined): Promise; protected abstract onClosePersistent(): void; async cookies(urls: string | string[] | undefined = []): Promise { @@ -412,8 +413,10 @@ export abstract class BrowserContext extends SdkObject { this._customCloseHandler = handler; } - async close(metadata: CallMetadata) { + async close(options: { reason?: string }) { if (this._closedStatus === 'open') { + if (options.reason) + this._closeReason = options.reason; this.emit(BrowserContext.Events.BeforeClose); this._closedStatus = 'closing'; @@ -433,7 +436,7 @@ export abstract class BrowserContext extends SdkObject { await this._customCloseHandler(); } else { // Close the context. - await this.doClose(); + await this.doClose(options.reason); } // We delete downloads after context closure diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index 38616682fa..e7e66ebe02 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -510,7 +510,7 @@ export class CRBrowserContext extends BrowserContext { await (sw as CRServiceWorker).updateRequestInterception(); } - async doClose() { + async doClose(reason: string | undefined) { // Headful chrome cannot dispose browser context with opened 'beforeunload' // dialogs, so we should close all that are currently opened. // We also won't get new ones since `Target.disposeBrowserContext` does not trigger @@ -525,7 +525,7 @@ export class CRBrowserContext extends BrowserContext { if (!this._browserContextId) { await Promise.all(this._crPages().map(crPage => crPage._mainFrameSession._stopVideoRecording())); // Closing persistent context should close the browser. - await this._browser.close(); + await this._browser.close({ reason }); return; } diff --git a/packages/playwright-core/src/server/debugController.ts b/packages/playwright-core/src/server/debugController.ts index 99f167e6ea..533d6eaf94 100644 --- a/packages/playwright-core/src/server/debugController.ts +++ b/packages/playwright-core/src/server/debugController.ts @@ -172,7 +172,7 @@ export class DebugController extends SdkObject { } async closeAllBrowsers() { - await Promise.all(this.allBrowsers().map(browser => browser.close())); + await Promise.all(this.allBrowsers().map(browser => browser.close({ reason: 'Close all browsers requested' }))); } private _emitSnapshot() { @@ -210,10 +210,10 @@ export class DebugController extends SdkObject { for (const browser of this._playwright.allBrowsers()) { for (const context of browser.contexts()) { if (!context.pages().length) - await context.close(serverSideCallMetadata()); + await context.close({ reason: 'Browser collected' }); } if (!browser.contexts()) - await browser.close(); + await browser.close({ reason: 'Browser collected' }); } } } diff --git a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts index 704a979653..bcfa3e7822 100644 --- a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts @@ -270,7 +270,7 @@ export class BrowserContextDispatcher extends Dispatcher { - await this._context.close(metadata); + await this._context.close(params); } async recorderSupplementEnable(params: channels.BrowserContextRecorderSupplementEnableParams): Promise { diff --git a/packages/playwright-core/src/server/dispatchers/browserDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserDispatcher.ts index c5622a3e9c..e41c29a97c 100644 --- a/packages/playwright-core/src/server/dispatchers/browserDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/browserDispatcher.ts @@ -24,7 +24,6 @@ import { Dispatcher } from './dispatcher'; import type { CRBrowser } from '../chromium/crBrowser'; import type { PageDispatcher } from './pageDispatcher'; import type { CallMetadata } from '../instrumentation'; -import { serverSideCallMetadata } from '../instrumentation'; import { BrowserContext } from '../browserContext'; import { Selectors } from '../selectors'; import type { BrowserTypeDispatcher } from './browserTypeDispatcher'; @@ -56,8 +55,8 @@ export class BrowserDispatcher extends Dispatcher { - await this._object.close(); + async close(params: channels.BrowserCloseParams): Promise { + await this._object.close(params); } async killForTests(): Promise { @@ -155,7 +154,7 @@ export class ConnectedBrowserDispatcher extends Dispatcher context.close(serverSideCallMetadata()))); + await Promise.all(Array.from(this._contexts).map(context => context.close({ reason: 'Global context cleanup (connection terminated)' }))); } } diff --git a/packages/playwright-core/src/server/dispatchers/dispatcher.ts b/packages/playwright-core/src/server/dispatchers/dispatcher.ts index ab70a6a145..6cbb4294ac 100644 --- a/packages/playwright-core/src/server/dispatchers/dispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/dispatcher.ts @@ -19,7 +19,7 @@ import type * as channels from '@protocol/channels'; import { serializeError } from '../../protocol/serializers'; import { findValidator, ValidationError, createMetadataValidator, type ValidatorContext } from '../../protocol/validator'; import { assert, isUnderTest, monotonicTime, rewriteErrorMessage } from '../../utils'; -import { TargetClosedError, kTargetClosedErrorMessage, kTargetCrashedErrorMessage } from '../../common/errors'; +import { TargetClosedError, isTargetClosedError, kTargetClosedErrorMessage, kTargetCrashedErrorMessage } from '../../common/errors'; import type { CallMetadata } from '../instrumentation'; import { SdkObject } from '../instrumentation'; import type { PlaywrightDispatcher } from './playwrightDispatcher'; @@ -330,9 +330,13 @@ export class DispatcherConnection { const validator = findValidator(dispatcher._type, method, 'Result'); callMetadata.result = validator(result, '', { tChannelImpl: this._tChannelImplToWire.bind(this), binary: this._isLocal ? 'buffer' : 'toBase64' }); } catch (e) { + if (isTargetClosedError(e) && sdkObject) + rewriteErrorMessage(e, closeReason(sdkObject)); if (isProtocolError(e)) { - if (e.type === 'closed') - rewriteErrorMessage(e, kTargetClosedErrorMessage + e.browserLogMessage()); + if (e.type === 'closed') { + const closedReason = sdkObject ? closeReason(sdkObject) : kTargetClosedErrorMessage; + rewriteErrorMessage(e, closedReason + e.browserLogMessage()); + } if (e.type === 'crashed') rewriteErrorMessage(e, kTargetCrashedErrorMessage + e.browserLogMessage()); } @@ -352,3 +356,9 @@ export class DispatcherConnection { this.onmessage(response); } } + +function closeReason(sdkObject: SdkObject) { + return sdkObject.attribution.page?._closeReason || + sdkObject.attribution.context?._closeReason || + sdkObject.attribution.browser?._closeReason || kTargetClosedErrorMessage; +} diff --git a/packages/playwright-core/src/server/electron/electron.ts b/packages/playwright-core/src/server/electron/electron.ts index 90f90aad07..677446695b 100644 --- a/packages/playwright-core/src/server/electron/electron.ts +++ b/packages/playwright-core/src/server/electron/electron.ts @@ -100,7 +100,7 @@ export class ElectronApplication extends SdkObject { async close() { const progressController = new ProgressController(serverSideCallMetadata(), this); const closed = progressController.run(progress => helper.waitForEvent(progress, this, ElectronApplication.Events.Close).promise); - await this._browserContext.close(serverSideCallMetadata()); + await this._browserContext.close({ reason: 'Application exited' }); this._nodeConnection.close(); await closed; } diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index 3a96a00aa0..7ffb330b83 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -91,6 +91,7 @@ export abstract class APIRequestContext extends SdkObject { readonly fetchLog: Map = new Map(); protected static allInstances: Set = new Set(); readonly _activeProgressControllers = new Set(); + _closeReason: string | undefined; static findResponseBody(guid: string): Buffer | undefined { for (const request of APIRequestContext.allInstances) { diff --git a/packages/playwright-core/src/server/firefox/ffBrowser.ts b/packages/playwright-core/src/server/firefox/ffBrowser.ts index 6df7b4807e..d299e4c734 100644 --- a/packages/playwright-core/src/server/firefox/ffBrowser.ts +++ b/packages/playwright-core/src/server/firefox/ffBrowser.ts @@ -376,7 +376,7 @@ export class FFBrowserContext extends BrowserContext { await this._browser.session.send('Browser.clearCache'); } - async doClose() { + async doClose(reason: string | undefined) { if (!this._browserContextId) { if (this._options.recordVideo) { await this._browser.session.send('Browser.setVideoRecordingOptions', { @@ -385,7 +385,7 @@ export class FFBrowserContext extends BrowserContext { }); } // Closing persistent context should close the browser. - await this._browser.close(); + await this._browser.close({ reason }); } else { await this._browser.session.send('Browser.removeBrowserContext', { browserContextId: this._browserContextId }); this._browser._contexts.delete(this._browserContextId); diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index d2117aafa9..221449e3dd 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -170,6 +170,7 @@ export class Page extends SdkObject { // Aiming at 25 fps by default - each frame is 40ms, but we give some slack with 35ms. // When throttling for tracing, 200ms between frames, except for 10 frames around the action. private _frameThrottler = new FrameThrottler(10, 35, 200); + _closeReason: string | undefined; constructor(delegate: PageDelegate, browserContext: BrowserContext) { super(browserContext, 'page'); @@ -596,10 +597,12 @@ export class Page extends SdkObject { this._timeoutSettings.timeout(options)); } - async close(metadata: CallMetadata, options?: { runBeforeUnload?: boolean }) { + async close(metadata: CallMetadata, options: { runBeforeUnload?: boolean, reason?: string } = {}) { if (this._closedState === 'closed') return; - const runBeforeUnload = !!options && !!options.runBeforeUnload; + if (options.reason) + this._closeReason = options.reason; + const runBeforeUnload = !!options.runBeforeUnload; if (this._closedState !== 'closing') { this._closedState = 'closing'; // This might throw if the browser context containing the page closes @@ -609,7 +612,7 @@ export class Page extends SdkObject { if (!runBeforeUnload) await this._closedPromise; if (this._ownedContext) - await this._ownedContext.close(metadata); + await this._ownedContext.close(options); } private _setIsError(error: Error) { diff --git a/packages/playwright-core/src/server/recorder/recorderApp.ts b/packages/playwright-core/src/server/recorder/recorderApp.ts index 1c970f6870..0ec712da64 100644 --- a/packages/playwright-core/src/server/recorder/recorderApp.ts +++ b/packages/playwright-core/src/server/recorder/recorderApp.ts @@ -75,7 +75,7 @@ export class RecorderApp extends EventEmitter implements IRecorderApp { } async close() { - await this._page.context().close(serverSideCallMetadata()); + await this._page.context().close({ reason: 'Recorder window closed' }); } private async _init() { @@ -105,7 +105,7 @@ export class RecorderApp extends EventEmitter implements IRecorderApp { this._page.once('close', () => { this.emit('close'); - this._page.context().close(serverSideCallMetadata()).catch(() => {}); + this._page.context().close({ reason: 'Recorder window closed' }).catch(() => {}); }); const mainFrame = this._page.mainFrame(); diff --git a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts index 70a6eab696..39a6c82c07 100644 --- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts +++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts @@ -157,7 +157,7 @@ export async function openTraceViewerApp(traceUrls: string[], browserName: strin await syncLocalStorageWithSettings(page, 'traceviewer'); if (isUnderTest()) - page.on('close', () => context.close(serverSideCallMetadata()).catch(() => {})); + page.on('close', () => context.close({ reason: 'Trace viewer closed' }).catch(() => {})); await page.mainFrame().goto(serverSideCallMetadata(), url); return page; diff --git a/packages/playwright-core/src/server/webkit/wkBrowser.ts b/packages/playwright-core/src/server/webkit/wkBrowser.ts index a9664e6a82..fbf0cda3a2 100644 --- a/packages/playwright-core/src/server/webkit/wkBrowser.ts +++ b/packages/playwright-core/src/server/webkit/wkBrowser.ts @@ -349,11 +349,11 @@ export class WKBrowserContext extends BrowserContext { }); } - async doClose() { + async doClose(reason: string | undefined) { if (!this._browserContextId) { await Promise.all(this._wkPages().map(wkPage => wkPage._stopVideo())); // Closing persistent context should close the browser. - await this._browser.close(); + await this._browser.close({ reason }); } else { await this._browser._browserSession.send('Playwright.deleteContext', { browserContextId: this._browserContextId }); this._browser._contexts.delete(this._browserContextId); diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index 37e30016f7..bec55023c5 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -2001,6 +2001,11 @@ export interface Page { * @param options */ close(options?: { + /** + * The reason to be reported to the operations interrupted by the page closure. + */ + reason?: string; + /** * Defaults to `false`. Whether to run the * [before unload](https://developer.mozilla.org/en-US/docs/Web/Events/beforeunload) page handlers. @@ -3627,7 +3632,8 @@ export interface Page { /** * If specified, updates the given HAR with the actual network information instead of serving from file. The file is * written to disk when - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) is called. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) is + * called. */ update?: boolean; @@ -7556,7 +7562,7 @@ export interface BrowserContext { * Emitted when Browser context gets closed. This might happen because of one of the following: * - Browser context is closed. * - Browser application is closed or crashed. - * - The [browser.close()](https://playwright.dev/docs/api/class-browser#browser-close) method was called. + * - The [browser.close([options])](https://playwright.dev/docs/api/class-browser#browser-close) method was called. */ on(event: 'close', listener: (browserContext: BrowserContext) => void): this; @@ -7748,7 +7754,7 @@ export interface BrowserContext { * Emitted when Browser context gets closed. This might happen because of one of the following: * - Browser context is closed. * - Browser application is closed or crashed. - * - The [browser.close()](https://playwright.dev/docs/api/class-browser#browser-close) method was called. + * - The [browser.close([options])](https://playwright.dev/docs/api/class-browser#browser-close) method was called. */ addListener(event: 'close', listener: (browserContext: BrowserContext) => void): this; @@ -7995,7 +8001,7 @@ export interface BrowserContext { * Emitted when Browser context gets closed. This might happen because of one of the following: * - Browser context is closed. * - Browser application is closed or crashed. - * - The [browser.close()](https://playwright.dev/docs/api/class-browser#browser-close) method was called. + * - The [browser.close([options])](https://playwright.dev/docs/api/class-browser#browser-close) method was called. */ prependListener(event: 'close', listener: (browserContext: BrowserContext) => void): this; @@ -8208,8 +8214,14 @@ export interface BrowserContext { * Closes the browser context. All the pages that belong to the browser context will be closed. * * **NOTE** The default browser context cannot be closed. + * @param options */ - close(): Promise; + close(options?: { + /** + * The reason to be reported to the operations interrupted by the context closure. + */ + reason?: string; + }): Promise; /** * If no URLs are specified, this method returns all cookies. If URLs are specified, only cookies that affect those @@ -8395,7 +8407,8 @@ export interface BrowserContext { /** * If specified, updates the given HAR with the actual network information instead of serving from file. The file is * written to disk when - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) is called. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) is + * called. */ update?: boolean; @@ -8587,7 +8600,7 @@ export interface BrowserContext { * Emitted when Browser context gets closed. This might happen because of one of the following: * - Browser context is closed. * - Browser application is closed or crashed. - * - The [browser.close()](https://playwright.dev/docs/api/class-browser#browser-close) method was called. + * - The [browser.close([options])](https://playwright.dev/docs/api/class-browser#browser-close) method was called. */ waitForEvent(event: 'close', optionsOrPredicate?: { predicate?: (browserContext: BrowserContext) => boolean | Promise, timeout?: number } | ((browserContext: BrowserContext) => boolean | Promise)): Promise; @@ -12968,8 +12981,8 @@ export interface BrowserType { /** * Enables [HAR](http://www.softwareishard.com/blog/har-12-spec) recording for all pages into `recordHar.path` file. * If not specified, the HAR is not recorded. Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for the HAR to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * the HAR to be saved. */ recordHar?: { /** @@ -13009,8 +13022,8 @@ export interface BrowserType { /** * Enables video recording for all pages into `recordVideo.dir` directory. If not specified videos are not recorded. * Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for videos to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * videos to be saved. */ recordVideo?: { /** @@ -14379,8 +14392,8 @@ export interface AndroidDevice { /** * Enables [HAR](http://www.softwareishard.com/blog/har-12-spec) recording for all pages into `recordHar.path` file. * If not specified, the HAR is not recorded. Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for the HAR to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * the HAR to be saved. */ recordHar?: { /** @@ -14420,8 +14433,8 @@ export interface AndroidDevice { /** * Enables video recording for all pages into `recordVideo.dir` directory. If not specified videos are not recorded. * Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for videos to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * videos to be saved. */ recordVideo?: { /** @@ -15963,7 +15976,7 @@ export interface Browser extends EventEmitter { * Emitted when Browser gets disconnected from the browser application. This might happen because of one of the * following: * - Browser application is closed or crashed. - * - The [browser.close()](https://playwright.dev/docs/api/class-browser#browser-close) method was called. + * - The [browser.close([options])](https://playwright.dev/docs/api/class-browser#browser-close) method was called. */ on(event: 'disconnected', listener: (browser: Browser) => void): this; @@ -15976,7 +15989,7 @@ export interface Browser extends EventEmitter { * Emitted when Browser gets disconnected from the browser application. This might happen because of one of the * following: * - Browser application is closed or crashed. - * - The [browser.close()](https://playwright.dev/docs/api/class-browser#browser-close) method was called. + * - The [browser.close([options])](https://playwright.dev/docs/api/class-browser#browser-close) method was called. */ addListener(event: 'disconnected', listener: (browser: Browser) => void): this; @@ -15994,7 +16007,7 @@ export interface Browser extends EventEmitter { * Emitted when Browser gets disconnected from the browser application. This might happen because of one of the * following: * - Browser application is closed or crashed. - * - The [browser.close()](https://playwright.dev/docs/api/class-browser#browser-close) method was called. + * - The [browser.close([options])](https://playwright.dev/docs/api/class-browser#browser-close) method was called. */ prependListener(event: 'disconnected', listener: (browser: Browser) => void): this; @@ -16012,14 +16025,20 @@ export interface Browser extends EventEmitter { * the browser server. * * **NOTE** This is similar to force quitting the browser. Therefore, you should call - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) on any {@link - * BrowserContext}'s you explicitly created earlier with + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) on + * any {@link BrowserContext}'s you explicitly created earlier with * [browser.newContext([options])](https://playwright.dev/docs/api/class-browser#browser-new-context) **before** - * calling [browser.close()](https://playwright.dev/docs/api/class-browser#browser-close). + * calling [browser.close([options])](https://playwright.dev/docs/api/class-browser#browser-close). * * The {@link Browser} object itself is considered to be disposed and cannot be used anymore. + * @param options */ - close(): Promise; + close(options?: { + /** + * The reason to be reported to the operations interrupted by the browser closure. + */ + reason?: string; + }): Promise; /** * Returns an array of all open browser contexts. In a newly created browser, this will return zero browser contexts. @@ -16054,10 +16073,10 @@ export interface Browser extends EventEmitter { * * **NOTE** If directly using this method to create {@link BrowserContext}s, it is best practice to explicitly close * the returned context via - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) when your code - * is done with the {@link BrowserContext}, and before calling - * [browser.close()](https://playwright.dev/docs/api/class-browser#browser-close). This will ensure the `context` is - * closed gracefully and any artifacts—like HARs and videos—are fully flushed and saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) when + * your code is done with the {@link BrowserContext}, and before calling + * [browser.close([options])](https://playwright.dev/docs/api/class-browser#browser-close). This will ensure the + * `context` is closed gracefully and any artifacts—like HARs and videos—are fully flushed and saved. * * **Usage** * @@ -16258,8 +16277,8 @@ export interface Browser extends EventEmitter { /** * Enables [HAR](http://www.softwareishard.com/blog/har-12-spec) recording for all pages into `recordHar.path` file. * If not specified, the HAR is not recorded. Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for the HAR to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * the HAR to be saved. */ recordHar?: { /** @@ -16299,8 +16318,8 @@ export interface Browser extends EventEmitter { /** * Enables video recording for all pages into `recordVideo.dir` directory. If not specified videos are not recorded. * Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for videos to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * videos to be saved. */ recordVideo?: { /** @@ -17093,8 +17112,8 @@ export interface Electron { /** * Enables [HAR](http://www.softwareishard.com/blog/har-12-spec) recording for all pages into `recordHar.path` file. * If not specified, the HAR is not recorded. Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for the HAR to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * the HAR to be saved. */ recordHar?: { /** @@ -17134,8 +17153,8 @@ export interface Electron { /** * Enables video recording for all pages into `recordVideo.dir` directory. If not specified videos are not recorded. * Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for videos to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * videos to be saved. */ recordVideo?: { /** @@ -19472,8 +19491,8 @@ export interface BrowserContextOptions { /** * Enables [HAR](http://www.softwareishard.com/blog/har-12-spec) recording for all pages into `recordHar.path` file. * If not specified, the HAR is not recorded. Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for the HAR to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * the HAR to be saved. */ recordHar?: { /** @@ -19513,8 +19532,8 @@ export interface BrowserContextOptions { /** * Enables video recording for all pages into `recordVideo.dir` directory. If not specified videos are not recorded. * Make sure to await - * [browserContext.close()](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for videos to - * be saved. + * [browserContext.close([options])](https://playwright.dev/docs/api/class-browsercontext#browser-context-close) for + * videos to be saved. */ recordVideo?: { /** diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index d2723aacce..20670b3462 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -112,7 +112,7 @@ const playwrightFixtures: Fixtures = ({ }); await use(browser); await (browser as any)._wrapApiCall(async () => { - await browser.close(); + await browser.close({ reason: 'Test ended.' }); }, true); return; } @@ -120,7 +120,7 @@ const playwrightFixtures: Fixtures = ({ const browser = await playwright[browserName].launch(); await use(browser); await (browser as any)._wrapApiCall(async () => { - await browser.close(); + await browser.close({ reason: 'Test ended.' }); }, true); }, { scope: 'worker', timeout: 0 }], @@ -331,10 +331,11 @@ const playwrightFixtures: Fixtures = ({ }); let counter = 0; + const closeReason = testInfo.status === 'timedOut' ? 'Test timeout of ' + testInfo.timeout + 'ms exceeded.' : 'Test ended.'; await Promise.all([...contexts.keys()].map(async context => { (context as any)[kStartedContextTearDown] = true; await (context as any)._wrapApiCall(async () => { - await context.close(); + await context.close({ reason: closeReason }); }, true); const testFailed = testInfo.status !== testInfo.expectedStatus; const preserveVideo = captureVideo && (videoMode === 'on' || (testFailed && videoMode === 'retain-on-failure') || (videoMode === 'on-first-retry' && testInfo.retry === 1)); @@ -374,7 +375,8 @@ const playwrightFixtures: Fixtures = ({ const context = await (browser as any)._newContextForReuse(defaultContextOptions); (context as any)[kIsReusedContext] = true; await use(context); - await (browser as any)._stopPendingOperations('Test ended'); + const closeReason = testInfo.status === 'timedOut' ? 'Test timeout of ' + testInfo.timeout + 'ms exceeded.' : 'Test ended.'; + await (browser as any)._stopPendingOperations(closeReason); }, page: async ({ context, _reuseContext }, use) => { diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts index d544e2a7e1..a20fbaeca6 100644 --- a/packages/protocol/src/channels.ts +++ b/packages/protocol/src/channels.ts @@ -1080,7 +1080,7 @@ export interface BrowserEventTarget { } export interface BrowserChannel extends BrowserEventTarget, Channel { _type_Browser: boolean; - close(params?: BrowserCloseParams, metadata?: CallMetadata): Promise; + close(params: BrowserCloseParams, metadata?: CallMetadata): Promise; killForTests(params?: BrowserKillForTestsParams, metadata?: CallMetadata): Promise; defaultUserAgentForTest(params?: BrowserDefaultUserAgentForTestParams, metadata?: CallMetadata): Promise; newContext(params: BrowserNewContextParams, metadata?: CallMetadata): Promise; @@ -1091,8 +1091,12 @@ export interface BrowserChannel extends BrowserEventTarget, Channel { stopTracing(params?: BrowserStopTracingParams, metadata?: CallMetadata): Promise; } export type BrowserCloseEvent = {}; -export type BrowserCloseParams = {}; -export type BrowserCloseOptions = {}; +export type BrowserCloseParams = { + reason?: string, +}; +export type BrowserCloseOptions = { + reason?: string, +}; export type BrowserCloseResult = void; export type BrowserKillForTestsParams = {}; export type BrowserKillForTestsOptions = {}; @@ -1426,7 +1430,7 @@ export interface BrowserContextChannel extends BrowserContextEventTarget, EventT addInitScript(params: BrowserContextAddInitScriptParams, metadata?: CallMetadata): Promise; clearCookies(params?: BrowserContextClearCookiesParams, metadata?: CallMetadata): Promise; clearPermissions(params?: BrowserContextClearPermissionsParams, metadata?: CallMetadata): Promise; - close(params?: BrowserContextCloseParams, metadata?: CallMetadata): Promise; + close(params: BrowserContextCloseParams, metadata?: CallMetadata): Promise; cookies(params: BrowserContextCookiesParams, metadata?: CallMetadata): Promise; exposeBinding(params: BrowserContextExposeBindingParams, metadata?: CallMetadata): Promise; grantPermissions(params: BrowserContextGrantPermissionsParams, metadata?: CallMetadata): Promise; @@ -1524,8 +1528,12 @@ export type BrowserContextClearCookiesResult = void; export type BrowserContextClearPermissionsParams = {}; export type BrowserContextClearPermissionsOptions = {}; export type BrowserContextClearPermissionsResult = void; -export type BrowserContextCloseParams = {}; -export type BrowserContextCloseOptions = {}; +export type BrowserContextCloseParams = { + reason?: string, +}; +export type BrowserContextCloseOptions = { + reason?: string, +}; export type BrowserContextCloseResult = void; export type BrowserContextCookiesParams = { urls: string[], @@ -1841,9 +1849,11 @@ export type PageAddInitScriptOptions = { export type PageAddInitScriptResult = void; export type PageCloseParams = { runBeforeUnload?: boolean, + reason?: string, }; export type PageCloseOptions = { runBeforeUnload?: boolean, + reason?: string, }; export type PageCloseResult = void; export type PageEmulateMediaParams = { diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index 615ccd6d4d..3bc8fa35ab 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -904,6 +904,8 @@ Browser: commands: close: + parameters: + reason: string? killForTests: @@ -1030,6 +1032,8 @@ BrowserContext: clearPermissions: close: + parameters: + reason: string? cookies: parameters: @@ -1282,6 +1286,7 @@ Page: close: parameters: runBeforeUnload: boolean? + reason: string? emulateMedia: parameters: diff --git a/tests/config/traceViewerFixtures.ts b/tests/config/traceViewerFixtures.ts index 16d58d761d..6c43f1a4d3 100644 --- a/tests/config/traceViewerFixtures.ts +++ b/tests/config/traceViewerFixtures.ts @@ -111,7 +111,7 @@ export const traceViewerFixtures: Fixtures { diff --git a/tests/playwright-test/playwright.spec.ts b/tests/playwright-test/playwright.spec.ts index 68f34122bb..686607530d 100644 --- a/tests/playwright-test/playwright.spec.ts +++ b/tests/playwright-test/playwright.spec.ts @@ -328,10 +328,7 @@ test('should report error and pending operations on timeout', async ({ runInline import { test, expect } from '@playwright/test'; test('timedout', async ({ page }) => { await page.setContent('
Click me
'); - await Promise.all([ - page.getByText('Missing').click(), - page.getByText('More missing').textContent(), - ]); + await page.getByText('Missing').click(); }); `, }, { workers: 1, timeout: 2000 }); @@ -339,8 +336,8 @@ test('should report error and pending operations on timeout', async ({ runInline expect(result.exitCode).toBe(1); expect(result.passed).toBe(0); expect(result.failed).toBe(1); - expect(result.output).toContain('Error: locator.textContent: Target page, context or browser has been closed'); - expect(result.output).toContain('a.test.ts:7:42'); + expect(result.output).toContain('Error: locator.click: Test timeout of 2000ms exceeded.'); + expect(result.output).toContain('a.test.ts:5:41'); }); test('should report error on timeout with shared page', async ({ runInlineTest }) => {