Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(page): introduce page.setDefaultTimeout #3854

Merged
merged 2 commits into from
Jan 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 42 additions & 18 deletions docs/api.md

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions lib/DOMWorld.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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' : ''}`;
Expand Down
20 changes: 7 additions & 13 deletions lib/FrameManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>} */
Expand All @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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([
Expand Down Expand Up @@ -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();
Expand Down
17 changes: 13 additions & 4 deletions lib/Page.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -79,11 +80,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);
Expand Down Expand Up @@ -269,7 +271,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);
}

/**
Expand Down Expand Up @@ -665,7 +674,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))
Expand All @@ -683,7 +692,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))
Expand Down
57 changes: 57 additions & 0 deletions lib/TimeoutSettings.js
Original file line number Diff line number Diff line change
@@ -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};
2 changes: 2 additions & 0 deletions lib/externs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 { }
Expand Down
19 changes: 19 additions & 0 deletions test/navigation.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
40 changes: 40 additions & 0 deletions test/page.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down Expand Up @@ -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([
Expand Down Expand Up @@ -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([
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions test/waittask.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down