refactor(test runner): use closeReason from server

Currently, when the server closes a BrowerContext/Page (e.g. because the page crashed), there's no way for the server to pass back a _closeReason. The client will always show the fallback error message 'Target page, context or browser has been closed'.

This PR makes the server send back the close reason as part of a new `reason` parameter on the `close` event, allowing both client- and server-originating closes to have a specific reason.
This commit is contained in:
Simon Knott 2024-08-01 13:24:46 +02:00
parent 47714d6559
commit 461818dfd2
No known key found for this signature in database
GPG key ID: 8CEDC00028084AEC
9 changed files with 30 additions and 18 deletions

View file

@ -88,7 +88,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
this.clock = new Clock(this); this.clock = new Clock(this);
this._channel.on('bindingCall', ({ binding }) => this._onBinding(BindingCall.from(binding))); this._channel.on('bindingCall', ({ binding }) => this._onBinding(BindingCall.from(binding)));
this._channel.on('close', () => this._onClose()); this._channel.on('close', ({ reason }) => this._onClose(reason));
this._channel.on('page', ({ page }) => this._onPage(Page.from(page))); this._channel.on('page', ({ page }) => this._onPage(Page.from(page)));
this._channel.on('route', ({ route }) => this._onRoute(network.Route.from(route))); this._channel.on('route', ({ route }) => this._onRoute(network.Route.from(route)));
this._channel.on('backgroundPage', ({ page }) => { this._channel.on('backgroundPage', ({ page }) => {
@ -431,7 +431,8 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
return CDPSession.from(result.session); return CDPSession.from(result.session);
} }
_onClose() { _onClose(reason?: string) {
this._closeReason = reason;
if (this._browser) if (this._browser)
this._browser._contexts.delete(this); this._browser._contexts.delete(this);
this._browserType?._contexts?.delete(this); this._browserType?._contexts?.delete(this);
@ -447,7 +448,6 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
async close(options: { reason?: string } = {}): Promise<void> { async close(options: { reason?: string } = {}): Promise<void> {
if (this._closeWasCalled) if (this._closeWasCalled)
return; return;
this._closeReason = options.reason;
this._closeWasCalled = true; this._closeWasCalled = true;
await this._wrapApiCall(async () => { await this._wrapApiCall(async () => {
await this.request.dispose(options); await this.request.dispose(options);

View file

@ -129,7 +129,7 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
this._opener = Page.fromNullable(initializer.opener); this._opener = Page.fromNullable(initializer.opener);
this._channel.on('bindingCall', ({ binding }) => this._onBinding(BindingCall.from(binding))); this._channel.on('bindingCall', ({ binding }) => this._onBinding(BindingCall.from(binding)));
this._channel.on('close', () => this._onClose()); this._channel.on('close', ({ reason }) => this._onClose(reason));
this._channel.on('crash', () => this._onCrash()); this._channel.on('crash', () => this._onCrash());
this._channel.on('download', ({ url, suggestedFilename, artifact }) => { this._channel.on('download', ({ url, suggestedFilename, artifact }) => {
const artifactObject = Artifact.from(artifact); const artifactObject = Artifact.from(artifact);
@ -218,8 +218,9 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
this.emit(Events.Page.Worker, worker); this.emit(Events.Page.Worker, worker);
} }
_onClose() { _onClose(reason?: string) {
this._closed = true; this._closed = true;
this._closeReason = reason;
this._browserContext._pages.delete(this); this._browserContext._pages.delete(this);
this._browserContext._backgroundPages.delete(this); this._browserContext._backgroundPages.delete(this);
this._disposeHarRouters(); this._disposeHarRouters();
@ -598,11 +599,10 @@ export class Page extends ChannelOwner<channels.PageChannel> implements api.Page
} }
async close(options: { runBeforeUnload?: boolean, reason?: string } = {}) { async close(options: { runBeforeUnload?: boolean, reason?: string } = {}) {
this._closeReason = options.reason;
this._closeWasCalled = true; this._closeWasCalled = true;
try { try {
if (this._ownedContext) if (this._ownedContext)
await this._ownedContext.close(); await this._ownedContext.close(options);
else else
await this._channel.close(options); await this._channel.close(options);
} catch (e) { } catch (e) {

View file

@ -815,7 +815,9 @@ scheme.BrowserContextConsoleEvent = tObject({
}), }),
page: tChannel(['Page']), page: tChannel(['Page']),
}); });
scheme.BrowserContextCloseEvent = tOptional(tObject({})); scheme.BrowserContextCloseEvent = tObject({
reason: tOptional(tString),
});
scheme.BrowserContextDialogEvent = tObject({ scheme.BrowserContextDialogEvent = tObject({
dialog: tChannel(['Dialog']), dialog: tChannel(['Dialog']),
}); });
@ -1045,7 +1047,9 @@ scheme.PageInitializer = tObject({
scheme.PageBindingCallEvent = tObject({ scheme.PageBindingCallEvent = tObject({
binding: tChannel(['BindingCall']), binding: tChannel(['BindingCall']),
}); });
scheme.PageCloseEvent = tOptional(tObject({})); scheme.PageCloseEvent = tObject({
reason: tOptional(tString),
});
scheme.PageCrashEvent = tOptional(tObject({})); scheme.PageCrashEvent = tOptional(tObject({}));
scheme.PageDownloadEvent = tObject({ scheme.PageDownloadEvent = tObject({
url: tString, url: tString,

View file

@ -252,7 +252,7 @@ export abstract class BrowserContext extends SdkObject {
if (this._isPersistentContext) if (this._isPersistentContext)
this.onClosePersistent(); this.onClosePersistent();
this._closePromiseFulfill!(new Error('Context closed')); this._closePromiseFulfill!(new Error('Context closed'));
this.emit(BrowserContext.Events.Close); this.emit(BrowserContext.Events.Close, { reason: this._closeReason });
} }
// BrowserContext methods. // BrowserContext methods.

View file

@ -82,8 +82,8 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
this.addObjectListener(BrowserContext.Events.Page, page => { this.addObjectListener(BrowserContext.Events.Page, page => {
this._dispatchEvent('page', { page: PageDispatcher.from(this, page) }); this._dispatchEvent('page', { page: PageDispatcher.from(this, page) });
}); });
this.addObjectListener(BrowserContext.Events.Close, () => { this.addObjectListener(BrowserContext.Events.Close, params => {
this._dispatchEvent('close'); this._dispatchEvent('close', params);
this._dispose(); this._dispose();
}); });
this.addObjectListener(BrowserContext.Events.PageError, (error: Error, page: Page) => { this.addObjectListener(BrowserContext.Events.PageError, (error: Error, page: Page) => {

View file

@ -70,8 +70,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel, Brows
this.adopt(mainFrame); this.adopt(mainFrame);
this._page = page; this._page = page;
this.addObjectListener(Page.Events.Close, () => { this.addObjectListener(Page.Events.Close, params => {
this._dispatchEvent('close'); this._dispatchEvent('close', params);
this._dispose(); this._dispose();
}); });
this.addObjectListener(Page.Events.Crash, () => this._dispatchEvent('crash')); this.addObjectListener(Page.Events.Crash, () => this._dispatchEvent('crash'));

View file

@ -221,7 +221,7 @@ export class Page extends SdkObject {
// in that case we fire another Close event to ensure that each reported Page will have // in that case we fire another Close event to ensure that each reported Page will have
// corresponding Close event after it is reported on the context. // corresponding Close event after it is reported on the context.
if (this.isClosed()) if (this.isClosed())
this.emit(Page.Events.Close); this.emit(Page.Events.Close, { reason: this._closeReason });
else else
this.instrumentation.onPageOpen(this); this.instrumentation.onPageOpen(this);
} }
@ -280,7 +280,7 @@ export class Page extends SdkObject {
this._frameThrottler.dispose(); this._frameThrottler.dispose();
assert(this._closedState !== 'closed', 'Page closed twice'); assert(this._closedState !== 'closed', 'Page closed twice');
this._closedState = 'closed'; this._closedState = 'closed';
this.emit(Page.Events.Close); this.emit(Page.Events.Close, { reason: this._closeReason });
this._closedPromise.resolve(); this._closedPromise.resolve();
this.instrumentation.onPageClose(this); this.instrumentation.onPageClose(this);
this.openScope.close(new TargetClosedError()); this.openScope.close(new TargetClosedError());

View file

@ -1545,7 +1545,9 @@ export type BrowserContextConsoleEvent = {
}, },
page: PageChannel, page: PageChannel,
}; };
export type BrowserContextCloseEvent = {}; export type BrowserContextCloseEvent = {
reason?: string,
};
export type BrowserContextDialogEvent = { export type BrowserContextDialogEvent = {
dialog: DialogChannel, dialog: DialogChannel,
}; };
@ -1963,7 +1965,9 @@ export interface PageChannel extends PageEventTarget, EventTargetChannel {
export type PageBindingCallEvent = { export type PageBindingCallEvent = {
binding: BindingCallChannel, binding: BindingCallChannel,
}; };
export type PageCloseEvent = {}; export type PageCloseEvent = {
reason?: string,
};
export type PageCrashEvent = {}; export type PageCrashEvent = {};
export type PageDownloadEvent = { export type PageDownloadEvent = {
url: string, url: string,

View file

@ -1285,6 +1285,8 @@ BrowserContext:
page: Page page: Page
close: close:
parameters:
reason: string?
dialog: dialog:
parameters: parameters:
@ -1737,6 +1739,8 @@ Page:
binding: BindingCall binding: BindingCall
close: close:
parameters:
reason: string?
crash: crash: