feat(page): introduce page.setDefaultTimeout (#3854)

Method `page.setDefaultTimeout` overrides default 30 seconds timeout
for all `page.waitFor*` methods, including navigation and waiting
for selectors.

Fix #3319.

lib/DOMWorld.js

@@ -27,10 +27,12 @@ class DOMWorld {
   /**
    * @param {!Puppeteer.FrameManager} frameManager
    * @param {!Puppeteer.Frame} frame
+   * @param {!Puppeteer.TimeoutSettings} timeoutSettings
    */
-  constructor(frameManager, frame) {
+  constructor(frameManager, frame, timeoutSettings) {
     this._frameManager = frameManager;
     this._frame = frame;
+    this._timeoutSettings = timeoutSettings;
 
     /** @type {?Promise<!Puppeteer.ElementHandle>} */
     this._documentPromise = null;
@@ -190,7 +192,7 @@ class DOMWorld {
   async setContent(html, options = {}) {
     const {
       waitUntil = ['load'],
-      timeout = 30000,
+      timeout = this._timeoutSettings.navigationTimeout(),
     } = options;
     // We rely upon the fact that document.open() will reset frame lifecycle with "init"
     // lifecycle event. @see https://crrev.com/608658
@@ -452,7 +454,7 @@ class DOMWorld {
   waitForFunction(pageFunction, options = {}, ...args) {
     const {
       polling = 'raf',
-      timeout = 30000
+      timeout = this._timeoutSettings.timeout(),
     } = options;
     return new WaitTask(this, pageFunction, 'function', polling, timeout, ...args).promise;
   }
@@ -474,7 +476,7 @@ class DOMWorld {
     const {
       visible: waitForVisible = false,
       hidden: waitForHidden = false,
-      timeout = 30000,
+      timeout = this._timeoutSettings.timeout(),
     } = options;
     const polling = waitForVisible || waitForHidden ? 'raf' : 'mutation';
     const title = `${isXPath ? 'XPath' : 'selector'} "${selectorOrXPath}"${waitForHidden ? ' to be hidden' : ''}`;

lib/FrameManager.js

@@ -29,13 +29,14 @@ class FrameManager extends EventEmitter {
    * @param {!Protocol.Page.FrameTree} frameTree
    * @param {!Puppeteer.Page} page
    * @param {!Puppeteer.NetworkManager} networkManager
+   * @param {!Puppeteer.TimeoutSettings} timeoutSettings
    */
-  constructor(client, frameTree, page, networkManager) {
+  constructor(client, frameTree, page, networkManager, timeoutSettings) {
     super();
     this._client = client;
     this._page = page;
     this._networkManager = networkManager;
-    this._defaultNavigationTimeout = 30000;
+    this._timeoutSettings = timeoutSettings;
     /** @type {!Map<string, !Frame>} */
     this._frames = new Map();
     /** @type {!Map<number, !ExecutionContext>} */
@@ -55,13 +56,6 @@ class FrameManager extends EventEmitter {
     this._handleFrameTree(frameTree);
   }
 
-  /**
-   * @param {number} timeout
-   */
-  setDefaultNavigationTimeout(timeout) {
-    this._defaultNavigationTimeout = timeout;
-  }
-
   /**
    * @param {!Puppeteer.Frame} frame
    * @param {string} url
@@ -73,7 +67,7 @@ class FrameManager extends EventEmitter {
     const {
       referer = this._networkManager.extraHTTPHeaders()['referer'],
       waitUntil = ['load'],
-      timeout = this._defaultNavigationTimeout,
+      timeout = this._timeoutSettings.navigationTimeout(),
     } = options;
 
     const watcher = new LifecycleWatcher(this, frame, waitUntil, timeout);
@@ -120,7 +114,7 @@ class FrameManager extends EventEmitter {
     assertNoLegacyNavigationOptions(options);
     const {
       waitUntil = ['load'],
-      timeout = this._defaultNavigationTimeout,
+      timeout = this._timeoutSettings.navigationTimeout(),
     } = options;
     const watcher = new LifecycleWatcher(this, frame, waitUntil, timeout);
     const error = await Promise.race([
@@ -373,8 +367,8 @@ class Frame {
     this._loaderId = '';
     /** @type {!Set<string>} */
     this._lifecycleEvents = new Set();
-    this._mainWorld = new DOMWorld(frameManager, this);
-    this._secondaryWorld = new DOMWorld(frameManager, this);
+    this._mainWorld = new DOMWorld(frameManager, this, frameManager._timeoutSettings);
+    this._secondaryWorld = new DOMWorld(frameManager, this, frameManager._timeoutSettings);
 
     /** @type {!Set<!Frame>} */
     this._childFrames = new Set();

lib/Page.js

@@ -30,6 +30,7 @@ const {Coverage} = require('./Coverage');
 const {Worker} = require('./Worker');
 const {createJSHandle} = require('./JSHandle');
 const {Accessibility} = require('./Accessibility');
+const {TimeoutSettings} = require('./TimeoutSettings');
 const writeFileAsync = helper.promisify(fs.writeFile);
 
 class Page extends EventEmitter {
@@ -78,11 +79,12 @@ class Page extends EventEmitter {
     this._target = target;
     this._keyboard = new Keyboard(client);
     this._mouse = new Mouse(client, this._keyboard);
+    this._timeoutSettings = new TimeoutSettings();
     this._touchscreen = new Touchscreen(client, this._keyboard);
     this._accessibility = new Accessibility(client);
     this._networkManager = new NetworkManager(client);
     /** @type {!FrameManager} */
-    this._frameManager = new FrameManager(client, frameTree, this, this._networkManager);
+    this._frameManager = new FrameManager(client, frameTree, this, this._networkManager, this._timeoutSettings);
     this._networkManager.setFrameManager(this._frameManager);
     this._emulationManager = new EmulationManager(client);
     this._tracing = new Tracing(client);
@@ -268,7 +270,14 @@ class Page extends EventEmitter {
    * @param {number} timeout
    */
   setDefaultNavigationTimeout(timeout) {
-    this._frameManager.setDefaultNavigationTimeout(timeout);
+    this._timeoutSettings.setDefaultNavigationTimeout(timeout);
+  }
+
+  /**
+   * @param {number} timeout
+   */
+  setDefaultTimeout(timeout) {
+    this._timeoutSettings.setDefaultTimeout(timeout);
   }
 
   /**
@@ -664,7 +673,7 @@ class Page extends EventEmitter {
    */
   async waitForRequest(urlOrPredicate, options = {}) {
     const {
-      timeout = 30000
+      timeout = this._timeoutSettings.timeout(),
     } = options;
     return helper.waitForEvent(this._networkManager, Events.NetworkManager.Request, request => {
       if (helper.isString(urlOrPredicate))
@@ -682,7 +691,7 @@ class Page extends EventEmitter {
    */
   async waitForResponse(urlOrPredicate, options = {}) {
     const {
-      timeout = 30000
+      timeout = this._timeoutSettings.timeout(),
     } = options;
     return helper.waitForEvent(this._networkManager, Events.NetworkManager.Response, response => {
       if (helper.isString(urlOrPredicate))

lib/TimeoutSettings.js

@@ -0,0 +1,57 @@
+/**
+ * Copyright 2019 Google Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+const DEFAULT_TIMEOUT = 30000;
+
+class TimeoutSettings {
+  constructor() {
+    this._defaultTimeout = null;
+    this._defaultNavigationTimeout = null;
+  }
+
+  /**
+   * @param {number} timeout
+   */
+  setDefaultTimeout(timeout) {
+    this._defaultTimeout = timeout;
+  }
+
+  /**
+   * @param {number} timeout
+   */
+  setDefaultNavigationTimeout(timeout) {
+    this._defaultNavigationTimeout = timeout;
+  }
+
+  /**
+   * @return {number}
+   */
+  navigationTimeout() {
+    if (this._defaultNavigationTimeout !== null)
+      return this._defaultNavigationTimeout;
+    if (this._defaultTimeout !== null)
+      return this._defaultTimeout;
+    return DEFAULT_TIMEOUT;
+  }
+
+  timeout() {
+    if (this._defaultTimeout !== null)
+      return this._defaultTimeout;
+    return DEFAULT_TIMEOUT;
+  }
+}
+
+module.exports = {TimeoutSettings};

lib/externs.d.ts

@@ -7,6 +7,7 @@ import {Mouse as RealMouse, Keyboard as RealKeyboard, Touchscreen as RealTouchsc
 import {Frame as RealFrame, FrameManager as RealFrameManager}  from './FrameManager.js';
 import {JSHandle as RealJSHandle, ElementHandle as RealElementHandle}  from './JSHandle.js';
 import {DOMWorld as RealDOMWorld}  from './DOMWorld.js';
+import {TimeoutSettings as RealTimeoutSettings}  from './TimeoutSettings.js';
 import {ExecutionContext as RealExecutionContext}  from './ExecutionContext.js';
 import { NetworkManager as RealNetworkManager, Request as RealRequest, Response as RealResponse } from './NetworkManager.js';
 import * as child_process from 'child_process';
@@ -30,6 +31,7 @@ declare global {
     export class ElementHandle extends RealElementHandle {}
     export class JSHandle extends RealJSHandle {}
     export class DOMWorld extends RealDOMWorld {}
+    export class TimeoutSettings extends RealTimeoutSettings {}
     export class ExecutionContext extends RealExecutionContext {}
     export class Page extends RealPage { }
     export class Response extends RealResponse { }

test/navigation.spec.js

@@ -118,6 +118,25 @@ module.exports.addTests = function({testRunner, expect}) {
       expect(error.message).toContain('Navigation Timeout Exceeded: 1ms');
       expect(error).toBeInstanceOf(TimeoutError);
     });
+    it('should fail when exceeding default maximum timeout', async({page, server}) => {
+      // Hang for request to the empty.html
+      server.setRoute('/empty.html', (req, res) => { });
+      let error = null;
+      page.setDefaultTimeout(1);
+      await page.goto(server.PREFIX + '/empty.html').catch(e => error = e);
+      expect(error.message).toContain('Navigation Timeout Exceeded: 1ms');
+      expect(error).toBeInstanceOf(TimeoutError);
+    });
+    it('should prioritize default navigation timeout over default timeout', async({page, server}) => {
+      // Hang for request to the empty.html
+      server.setRoute('/empty.html', (req, res) => { });
+      let error = null;
+      page.setDefaultTimeout(0);
+      page.setDefaultNavigationTimeout(1);
+      await page.goto(server.PREFIX + '/empty.html').catch(e => error = e);
+      expect(error.message).toContain('Navigation Timeout Exceeded: 1ms');
+      expect(error).toBeInstanceOf(TimeoutError);
+    });
     it('should disable timeout when its set to 0', async({page, server}) => {
       let error = null;
       let loaded = false;

test/page.spec.js

@@ -17,6 +17,7 @@ const fs = require('fs');
 const path = require('path');
 const utils = require('./utils');
 const {waitEvent} = utils;
+const {TimeoutError} = utils.requireRoot('Errors');
 
 const DeviceDescriptors = utils.requireRoot('DeviceDescriptors');
 const iPhone = DeviceDescriptors['iPhone 6'];
@@ -421,6 +422,17 @@ module.exports.addTests = function({testRunner, expect, headless}) {
       ]);
       expect(request.url()).toBe(server.PREFIX + '/digits/2.png');
     });
+    it('should respect timeout', async({page, server}) => {
+      let error = null;
+      await page.waitForRequest(() => false, {timeout: 1}).catch(e => error = e);
+      expect(error).toBeInstanceOf(TimeoutError);
+    });
+    it('should respect default timeout', async({page, server}) => {
+      let error = null;
+      page.setDefaultTimeout(1);
+      await page.waitForRequest(() => false).catch(e => error = e);
+      expect(error).toBeInstanceOf(TimeoutError);
+    });
     it('should work with no timeout', async({page, server}) => {
       await page.goto(server.EMPTY_PAGE);
       const [request] = await Promise.all([
@@ -448,6 +460,17 @@ module.exports.addTests = function({testRunner, expect, headless}) {
       ]);
       expect(response.url()).toBe(server.PREFIX + '/digits/2.png');
     });
+    it('should respect timeout', async({page, server}) => {
+      let error = null;
+      await page.waitForResponse(() => false, {timeout: 1}).catch(e => error = e);
+      expect(error).toBeInstanceOf(TimeoutError);
+    });
+    it('should respect default timeout', async({page, server}) => {
+      let error = null;
+      page.setDefaultTimeout(1);
+      await page.waitForResponse(() => false).catch(e => error = e);
+      expect(error).toBeInstanceOf(TimeoutError);
+    });
     it('should work with predicate', async({page, server}) => {
       await page.goto(server.EMPTY_PAGE);
       const [response] = await Promise.all([
@@ -617,6 +640,23 @@ module.exports.addTests = function({testRunner, expect, headless}) {
       const result = await page.content();
       expect(result).toBe(`${doctype}${expectedOutput}`);
     });
+    it('should respect timeout', async({page, server}) => {
+      const imgPath = '/img.png';
+      // stall for image
+      server.setRoute(imgPath, (req, res) => {});
+      let error = null;
+      await page.setContent(`<img src="${server.PREFIX + imgPath}"></img>`, {timeout: 1}).catch(e => error = e);
+      expect(error).toBeInstanceOf(TimeoutError);
+    });
+    it('should respect default navigation timeout', async({page, server}) => {
+      page.setDefaultNavigationTimeout(1);
+      const imgPath = '/img.png';
+      // stall for image
+      server.setRoute(imgPath, (req, res) => {});
+      let error = null;
+      await page.setContent(`<img src="${server.PREFIX + imgPath}"></img>`).catch(e => error = e);
+      expect(error).toBeInstanceOf(TimeoutError);
+    });
     it('should await resources to load', async({page, server}) => {
       const imgPath = '/img.png';
       let imgResponse = null;

test/waittask.spec.js

@@ -170,6 +170,14 @@ module.exports.addTests = function({testRunner, expect, product}) {
       expect(error.message).toContain('waiting for function failed: timeout');
       expect(error).toBeInstanceOf(TimeoutError);
     });
+    it('should respect default timeout', async({page}) => {
+      page.setDefaultTimeout(1);
+      let error = null;
+      await page.waitForFunction('false').catch(e => error = e);
+      expect(error).toBeTruthy();
+      expect(error.message).toContain('waiting for function failed: timeout');
+      expect(error).toBeInstanceOf(TimeoutError);
+    });
     it('should disable timeout when its set to 0', async({page}) => {
       const watchdog = page.waitForFunction(() => {
         window.__counter = (window.__counter || 0) + 1;