From 0ba823dd6fbdf51739e5971f166ccb14b6c41e44 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 15 Apr 2020 00:04:35 -0700 Subject: [PATCH] feat: introduce `page.on('crash')` event (#1782) Currently, whenever the page crashes, it emits an `'error'` event. Error event is a special type of event in node.js; if unhandled, it crashes the process. Instead of emitting `'error'` event, this patch switches to emitting `'crash'` event. Playwright users are free to handle the event however they like, or just to ignore it. --- docs/api.md | 5 +++++ src/chromium/crConnection.ts | 7 +++++++ src/chromium/crPage.ts | 3 ++- src/events.ts | 1 + src/firefox/ffConnection.ts | 7 +++++++ src/firefox/ffPage.ts | 1 + src/page.ts | 5 +---- src/webkit/wkConnection.ts | 7 +++++++ src/webkit/wkPage.ts | 4 +++- test/page.spec.js | 23 ++++++++++++++++------- 10 files changed, 50 insertions(+), 13 deletions(-) diff --git a/docs/api.md b/docs/api.md index 4f5733d87a..6736f12aa0 100644 --- a/docs/api.md +++ b/docs/api.md @@ -632,6 +632,7 @@ page.removeListener('request', logRequest); - [event: 'close'](#event-close-1) - [event: 'console'](#event-console) +- [event: 'crash'](#event-crash) - [event: 'dialog'](#event-dialog) - [event: 'domcontentloaded'](#event-domcontentloaded) - [event: 'download'](#event-download) @@ -726,6 +727,10 @@ page.on('console', msg => { page.evaluate(() => console.log('hello', 5, {foo: 'bar'})); ``` +#### event: 'crash' + +Emitted when the page crashes. Browser pages might crash if they try to allocate too much memory. + #### event: 'dialog' - <[Dialog]> diff --git a/src/chromium/crConnection.ts b/src/chromium/crConnection.ts index eeb1fe9c58..46d89a03eb 100644 --- a/src/chromium/crConnection.ts +++ b/src/chromium/crConnection.ts @@ -121,6 +121,7 @@ export class CRSession extends EventEmitter { private readonly _targetType: string; private readonly _sessionId: string; private readonly _rootSessionId: string; + private _crashed: boolean = false; on: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; addListener: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; off: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; @@ -141,10 +142,16 @@ export class CRSession extends EventEmitter { this.once = super.once; } + _markAsCrashed() { + this._crashed = true; + } + async send( method: T, params?: Protocol.CommandParameters[T] ): Promise { + if (this._crashed) + throw new Error('Target crashed'); if (!this._connection) throw new Error(`Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`); const id = this._connection._rawSend(this._sessionId, method, params); diff --git a/src/chromium/crPage.ts b/src/chromium/crPage.ts index da09c67343..c64250f828 100644 --- a/src/chromium/crPage.ts +++ b/src/chromium/crPage.ts @@ -613,7 +613,8 @@ class FrameSession { this._page.emit(Events.Page.PageError, exceptionToError(exceptionDetails)); } - _onTargetCrashed() { + async _onTargetCrashed() { + this._client._markAsCrashed(); this._page._didCrash(); } diff --git a/src/events.ts b/src/events.ts index 20a2729565..589f19c273 100644 --- a/src/events.ts +++ b/src/events.ts @@ -31,6 +31,7 @@ export const Events = { Page: { Close: 'close', + Crash: 'crash', Console: 'console', Dialog: 'dialog', Download: 'download', diff --git a/src/firefox/ffConnection.ts b/src/firefox/ffConnection.ts index 8286796ff3..da92214024 100644 --- a/src/firefox/ffConnection.ts +++ b/src/firefox/ffConnection.ts @@ -140,6 +140,7 @@ export class FFSession extends EventEmitter { private _targetType: string; private _sessionId: string; private _rawSend: (message: any) => void; + private _crashed: boolean = false; on: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; addListener: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; off: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; @@ -161,10 +162,16 @@ export class FFSession extends EventEmitter { this.once = super.once; } + markAsCrashed() { + this._crashed = true; + } + async send( method: T, params?: Protocol.CommandParameters[T] ): Promise { + if (this._crashed) + throw new Error('Page crashed'); if (this._disposed) throw new Error(`Protocol error (${method}): Session closed. Most likely the ${this._targetType} has been closed.`); const id = this._connection.nextMessageId(); diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 97c03069be..d14dc964ff 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -251,6 +251,7 @@ export class FFPage implements PageDelegate { } async _onCrashed(event: Protocol.Page.crashedPayload) { + this._session.markAsCrashed(); this._page._didCrash(); } diff --git a/src/page.ts b/src/page.ts index 28626d6e5b..8dc74b867f 100644 --- a/src/page.ts +++ b/src/page.ts @@ -159,10 +159,7 @@ export class Page extends ExtendedEventEmitter { } _didCrash() { - const error = new Error('Page crashed!'); - // Do not report node.js stack. - error.stack = 'Error: ' + error.message; // Stack is supposed to contain error message as the first line. - this.emit('error', error); + this.emit(Events.Page.Crash); } _didDisconnect() { diff --git a/src/webkit/wkConnection.ts b/src/webkit/wkConnection.ts index 1b6ba05cd2..b381f62f89 100644 --- a/src/webkit/wkConnection.ts +++ b/src/webkit/wkConnection.ts @@ -97,6 +97,7 @@ export class WKSession extends EventEmitter { private _disposed = false; private readonly _rawSend: (message: any) => void; private readonly _callbacks = new Map void, reject: (e: Error) => void, error: Error, method: string}>(); + private _crashed: boolean = false; on: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; addListener: (event: T, listener: (payload: T extends symbol ? any : Protocol.Events[T extends keyof Protocol.Events ? T : never]) => void) => this; @@ -122,6 +123,8 @@ export class WKSession extends EventEmitter { method: T, params?: Protocol.CommandParameters[T] ): Promise { + if (this._crashed) + throw new Error('Target crashed'); if (this._disposed) throw new Error(`Protocol error (${method}): ${this.errorText}`); const id = this.connection.nextMessageId(); @@ -133,6 +136,10 @@ export class WKSession extends EventEmitter { }); } + markAsCrashed() { + this._crashed = true; + } + isDisposed(): boolean { return this._disposed; } diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index cbea2b0ce3..c2be00600a 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -194,8 +194,10 @@ export class WKPage implements PageDelegate { } else if (this._session.sessionId === targetId) { this._session.dispose(); helper.removeEventListeners(this._sessionListeners); - if (crashed) + if (crashed) { + this._session.markAsCrashed(); this._page._didCrash(); + } } } diff --git a/test/page.spec.js b/test/page.spec.js index 9d9e33edd1..74b81ae138 100644 --- a/test/page.spec.js +++ b/test/page.spec.js @@ -104,19 +104,28 @@ describe('Async stacks', () => { }); }); -describe('Page.Events.error', function() { - it('should throw when page crashes', async({page}) => { - await page.setContent(`
This page should crash
`); - let error = null; - page.on('error', err => error = err); +describe('Page.Events.Crash', function() { + function crash(page) { if (CHROMIUM) page.goto('chrome://crash').catch(e => {}); else if (WEBKIT) page._delegate._session.send('Page.crash', {}).catch(e => {}); else if (FFOX) page._delegate._session.send('Page.crash', {}).catch(e => {}); - await new Promise(f => page.on('error', f)); - expect(error.message).toBe('Page crashed!'); + } + + it('should emit crash event when page crashes', async({page}) => { + await page.setContent(`
This page should crash
`); + crash(page); + await new Promise(f => page.on('crash', f)); + }); + it('should throw on any action after page crashes', async({page}) => { + await page.setContent(`
This page should crash
`); + crash(page); + await page.waitForEvent('crash'); + const err = await page.evaluate(() => {}).then(() => null, e => e); + expect(err).toBeTruthy(); + expect(err.message).toContain('crash'); }); });