From 63f16a9ef8a8d87b913e93a579be7391fdb15ff6 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 10 Jan 2020 17:25:28 -0800 Subject: [PATCH] fix(screenshot): provide nice error message during navigation (#456) --- src/firefox/ffPage.ts | 5 ++++ src/screenshotter.ts | 55 +++++++++++++++++++++++----------- test/assets/redirectloop1.html | 9 ++++++ test/assets/redirectloop2.html | 5 ++++ test/screenshot.spec.js | 12 ++++++++ 5 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 test/assets/redirectloop1.html create mode 100644 test/assets/redirectloop2.html diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 86cf6a0c16..cf1adc5ff8 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -32,6 +32,7 @@ import * as network from '../network'; import * as types from '../types'; import * as accessibility from '../accessibility'; import * as platform from '../platform'; +import { kScreenshotDuringNavigationError } from '../screenshotter'; export class FFPage implements PageDelegate { readonly rawMouse: RawMouseImpl; @@ -278,6 +279,10 @@ export class FFPage implements PageDelegate { mimeType: ('image/' + format) as ('image/png' | 'image/jpeg'), fullPage: options.fullPage, clip: options.clip, + }).catch(e => { + if (e instanceof Error && e.message.includes('document.documentElement is null')) + e.message = kScreenshotDuringNavigationError; + throw e; }); return platform.Buffer.from(data, 'base64'); } diff --git a/src/screenshotter.ts b/src/screenshotter.ts index 0fbdea1bf1..296dc60721 100644 --- a/src/screenshotter.ts +++ b/src/screenshotter.ts @@ -43,24 +43,36 @@ export class Screenshotter { const viewport = this._page.viewport(); let viewportSize: types.Size | undefined; if (!viewport) { - viewportSize = await this._page.evaluate(() => ({ - width: Math.max(document.body.offsetWidth, document.documentElement.offsetWidth), - height: Math.max(document.body.offsetHeight, document.documentElement.offsetHeight) - })); + viewportSize = await this._page.evaluate(() => { + if (!document.body || !document.documentElement) + return null; + return { + width: Math.max(document.body.offsetWidth, document.documentElement.offsetWidth), + height: Math.max(document.body.offsetHeight, document.documentElement.offsetHeight), + }; + }); + if (!viewportSize) + throw new Error(kScreenshotDuringNavigationError); } if (options.fullPage && !this._page._delegate.canScreenshotOutsideViewport()) { - const fullPageRect = await this._page.evaluate(() => ({ - width: Math.max( - document.body.scrollWidth, document.documentElement.scrollWidth, - document.body.offsetWidth, document.documentElement.offsetWidth, - document.body.clientWidth, document.documentElement.clientWidth - ), - height: Math.max( - document.body.scrollHeight, document.documentElement.scrollHeight, - document.body.offsetHeight, document.documentElement.offsetHeight, - document.body.clientHeight, document.documentElement.clientHeight - ) - })); + const fullPageRect = await this._page.evaluate(() => { + if (!document.body || !document.documentElement) + return null; + return { + width: Math.max( + document.body.scrollWidth, document.documentElement.scrollWidth, + document.body.offsetWidth, document.documentElement.offsetWidth, + document.body.clientWidth, document.documentElement.clientWidth + ), + height: Math.max( + document.body.scrollHeight, document.documentElement.scrollHeight, + document.body.offsetHeight, document.documentElement.offsetHeight, + document.body.clientHeight, document.documentElement.clientHeight + ), + }; + }); + if (!fullPageRect) + throw new Error(kScreenshotDuringNavigationError); overridenViewport = viewport ? { ...viewport, ...fullPageRect } : fullPageRect; await this._page.setViewport(overridenViewport); } else if (options.clip) { @@ -76,7 +88,7 @@ export class Screenshotter { await this._page._delegate.resetViewport(viewportSize); } return result; - }); + }).catch(rewriteError); } async screenshotElement(handle: dom.ElementHandle, options: types.ElementScreenshotOptions = {}): Promise { @@ -115,7 +127,7 @@ export class Screenshotter { await this._page.setViewport(viewport); return result; - }); + }).catch(rewriteError); } private async _screenshot(format: 'png' | 'jpeg', options: types.ScreenshotOptions, viewport: types.Viewport): Promise { @@ -201,3 +213,10 @@ function enclosingIntRect(rect: types.Rect): types.Rect { const y2 = Math.ceil(rect.y + rect.height - 1e-3); return { x, y, width: x2 - x, height: y2 - y }; } + +export const kScreenshotDuringNavigationError = 'Cannot take a screenshot while page is navigating'; +function rewriteError(e: any) { + if (typeof e === 'object' && e instanceof Error && e.message.includes('Execution context was destroyed')) + e.message = kScreenshotDuringNavigationError; + throw e; +} diff --git a/test/assets/redirectloop1.html b/test/assets/redirectloop1.html new file mode 100644 index 0000000000..27fab61ca8 --- /dev/null +++ b/test/assets/redirectloop1.html @@ -0,0 +1,9 @@ +> diff --git a/test/assets/redirectloop2.html b/test/assets/redirectloop2.html new file mode 100644 index 0000000000..87c7194798 --- /dev/null +++ b/test/assets/redirectloop2.html @@ -0,0 +1,5 @@ + diff --git a/test/screenshot.spec.js b/test/screenshot.spec.js index ca5d023fd0..d2afa03e92 100644 --- a/test/screenshot.spec.js +++ b/test/screenshot.spec.js @@ -181,6 +181,18 @@ module.exports.describe = function({testRunner, expect, product, FFOX, CHROME, W const screenshot = await page.screenshot(); expect(screenshot).toBeGolden('screenshot-webgl.png'); }); + it('should work while navigating', async({page, server}) => { + await page.setViewport({width: 500, height: 500}); + await page.goto(server.PREFIX + '/redirectloop1.html'); + for (let i = 0; i < 10; i++) { + const screenshot = await page.screenshot({ fullPage: true }).catch(e => { + if (e.message.includes('Cannot take a screenshot while page is navigating')) + return Buffer.from(''); + throw e; + }); + expect(screenshot).toBeInstanceOf(Buffer); + } + }); }); describe('ElementHandle.screenshot', function() {