fix(crash): improve documentation for crash, reject waitForEvent (#2694)

docs/api.md

@@ -783,7 +783,31 @@ 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.
+Emitted when the page crashes. Browser pages might crash if they try to allocate too much memory. When the page crashes, ongoing and subsequent operations will throw.
+
+The most common way to deal with crashes is to catch an exception:
+```js
+try {
+  // Crash might happen during a click.
+  await page.click('button');
+  // Or while waiting for an event.
+  await page.waitForEvent('popup');
+} catch (e) {
+  // When the page crashes, exception message contains 'crash'.
+}
+```
+
+However, when manually listening to events, it might be useful to avoid stalling when the page crashes. In this case, handling `crash` event helps:
+
+```js
+await new Promise((resolve, reject) => {
+  page.on('requestfinished', async request => {
+    if (await someProcessing(request))
+      resolve(request);
+  });
+  page.on('crash', error => reject(error));
+});
+```
 
 #### event: 'dialog'
 - <[Dialog]>

src/frames.ts

@@ -983,6 +983,7 @@ class SignalBarrier {
     const frameTask = new FrameTask(frame, this._progress);
     await Promise.race([
       frame._page._disconnectedPromise,
+      frame._page._crashedPromise,
       frame._detachedPromise,
       frameTask.waitForNewDocument(),
       frameTask.waitForSameDocumentNavigation(),
@@ -1114,6 +1115,7 @@ class FrameTask {
 }
 
 function abortProgressOnFrameDetach(controller: ProgressController, frame: Frame) {
-  frame._page._disconnectedPromise.then(() => controller.abort(new Error('Navigation failed because browser has disconnected!')));
+  frame._page._disconnectedPromise.then(() => controller.abort(new Error('Navigation failed because page was closed!')));
+  frame._page._crashedPromise.then(() => controller.abort(new Error('Navigation failed because page crashed!')));
   frame._detachedPromise.then(() => controller.abort(new Error('Navigating frame was detached!')));
 }

src/page.ts

@@ -94,6 +94,8 @@ export class Page extends EventEmitter {
   private _disconnected = false;
   private _disconnectedCallback: (e: Error) => void;
   readonly _disconnectedPromise: Promise<Error>;
+  private _crashedCallback: (e: Error) => void;
+  readonly _crashedPromise: Promise<Error>;
   readonly _browserContext: BrowserContextBase;
   readonly keyboard: input.Keyboard;
   readonly mouse: input.Mouse;
@@ -121,6 +123,8 @@ export class Page extends EventEmitter {
     this._closedPromise = new Promise(f => this._closedCallback = f);
     this._disconnectedCallback = () => {};
     this._disconnectedPromise = new Promise(f => this._disconnectedCallback = f);
+    this._crashedCallback = () => {};
+    this._crashedPromise = new Promise(f => this._crashedCallback = f);
     this._browserContext = browserContext;
     this._state = {
       viewportSize: browserContext._options.viewport ? { ...browserContext._options.viewport } : null,
@@ -153,12 +157,13 @@ export class Page extends EventEmitter {
 
   _didCrash() {
     this.emit(Events.Page.Crash);
+    this._crashedCallback(new Error('Page crashed'));
   }
 
   _didDisconnect() {
     assert(!this._disconnected, 'Page disconnected twice');
     this._disconnected = true;
-    this._disconnectedCallback(new Error('Target closed'));
+    this._disconnectedCallback(new Error('Page closed'));
   }
 
   async _onFileChooserOpened(handle: dom.ElementHandle) {
@@ -335,6 +340,8 @@ export class Page extends EventEmitter {
     const options = typeof optionsOrPredicate === 'function' ? { predicate: optionsOrPredicate } : optionsOrPredicate;
     const progressController = new ProgressController(this._logger, this._timeoutSettings.timeout(options), 'page.waitForEvent');
     this._disconnectedPromise.then(error => progressController.abort(error));
+    if (event !== Events.Page.Crash)
+      this._crashedPromise.then(error => progressController.abort(error));
     return progressController.run(progress => helper.waitForEvent(progress, this, event, options.predicate));
   }
 

test/launcher.spec.js

@@ -154,7 +154,7 @@ describe('Browser.disconnect', function() {
     await server.waitForRequest('/one-style.css');
     await remote.close();
     const error = await navigationPromise;
-    expect(error.message).toContain('Navigation failed because browser has disconnected!');
+    expect(error.message).toContain('Navigation failed because page was closed!');
     await browserServer._checkLeaks();
     await browserServer.close();
   });
@@ -211,7 +211,7 @@ describe('Browser.close', function() {
     ]);
     for (let i = 0; i < 2; i++) {
       const message = results[i].message;
-      expect(message).toContain('Target closed');
+      expect(message).toContain('Page closed');
       expect(message).not.toContain('Timeout');
     }
   });

test/page.spec.js

@@ -78,7 +78,7 @@ describe('Page.close', function() {
     ]);
     for (let i = 0; i < 2; i++) {
       const message = results[i].message;
-      expect(message).toContain('Target closed');
+      expect(message).toContain('Page closed');
       expect(message).not.toContain('Timeout');
     }
   });
@@ -130,6 +130,28 @@ describe.fail(FFOX && WIN)('Page.Events.Crash', function() {
     expect(err).toBeTruthy();
     expect(err.message).toContain('crash');
   });
+  it('should cancel waitForEvent when page crashes', async({page}) => {
+    await page.setContent(`<div>This page should crash</div>`);
+    const promise = page.waitForEvent('response').catch(e => e);
+    crash(page);
+    const error = await promise;
+    expect(error.message).toContain('Page crashed');
+  });
+  it('should cancel navigation when page crashes', async({page, server}) => {
+    await page.setContent(`<div>This page should crash</div>`);
+    server.setRoute('/one-style.css', () => {});
+    const promise = page.goto(server.PREFIX + '/one-style.html').catch(e => e);
+    await page.waitForNavigation({ waitUntil: 'domcontentloaded' });
+    crash(page);
+    const error = await promise;
+    expect(error.message).toContain('Navigation failed because page crashed');
+  });
+  it('should be able to close context when page crashes', async({page}) => {
+    await page.setContent(`<div>This page should crash</div>`);
+    crash(page);
+    await page.waitForEvent('crash');
+    await page.context().close();
+  });
 });
 
 describe('Page.opener', function() {
@@ -353,7 +375,7 @@ describe('Page.waitForEvent', function() {
     const waitForPromise = page.waitForEvent('download').catch(e => error = e);
     await page.close();
     await waitForPromise;
-    expect(error.message).toContain('Target closed');
+    expect(error.message).toContain('Page closed');
   });
 });