Skip to content

Commit

Permalink
fix(page): fix "timeout: 0" to actually disable any navigation timeout (
Browse files Browse the repository at this point in the history
puppeteer#1435)

Since non-promise values always win the `Promise.race`, we shouldn't
return `null` for timeout promise in NavigationWatcher.

Instead, we can return a promise that never resolved. It should be
GC'd later with the navigation watcher itself.

Fixes puppeteer#1417.
  • Loading branch information
aslushnikov authored Nov 21, 2017
1 parent cafd040 commit 88eaede
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/NavigatorWatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class NavigatorWatcher {
*/
_createTimeoutPromise() {
if (!this._timeout)
return null;
return new Promise(() => {});
const errorMessage = 'Navigation Timeout Exceeded: ' + this._timeout + 'ms exceeded';
return new Promise(fulfill => this._maximumTimer = setTimeout(fulfill, this._timeout))
.then(() => new Error(errorMessage));
Expand Down
5 changes: 4 additions & 1 deletion test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1029,8 +1029,11 @@ describe('Page', function() {
}));
it('should disable timeout when its set to 0', SX(async function() {
let error = null;
await page.goto(PREFIX + '/grid.html', {timeout: 0}).catch(e => error = e);
let loaded = false;
page.once('load', () => loaded = true);
await page.goto(PREFIX + '/grid.html', {timeout: 0, waitUntil: ['load']}).catch(e => error = e);
expect(error).toBe(null);
expect(loaded).toBe(true);
}));
it('should work when navigating to valid url', SX(async function() {
const response = await page.goto(EMPTY_PAGE);
Expand Down

0 comments on commit 88eaede

Please sign in to comment.