Carefully manage unhandled rejections for navigation (#741)

Currently, navigation watcher throws exception if timeout
is exceeded.

Due to the way it is used in `page.navigate`, the promise
get's rejected before it is awaited, which is considered to
be "unhandled promise rejection".

Fixes #738
This commit is contained in:
Andrey Lushnikov 2017-09-11 16:43:37 -07:00 committed by GitHub
parent 0db6165d73
commit e5c17eecb9
3 changed files with 17 additions and 14 deletions

View File

@ -33,7 +33,7 @@ class NavigatorWatcher {
}
/**
* @return {!Promise<!Map<string, !Response>>}
* @return {!Promise<?Error>}
*/
async waitForNavigation() {
this._requestIds = new Set();
@ -70,8 +70,7 @@ class NavigatorWatcher {
const error = await Promise.race(navigationPromises);
this._cleanup();
if (error)
throw new Error(error);
return error ? new Error(error) : null;
}
cancel() {

View File

@ -372,7 +372,7 @@ class Page extends EventEmitter {
const watcher = new NavigatorWatcher(this._client, this._ignoreHTTPSErrors, options);
const responses = new Map();
const listener = helper.addEventListener(this._networkManager, NetworkManager.Events.Response, response => responses.set(response.url, response));
const result = watcher.waitForNavigation();
const navigationPromise = watcher.waitForNavigation();
const referrer = this._networkManager.extraHTTPHeaders()['referer'];
try {
@ -382,8 +382,10 @@ class Page extends EventEmitter {
watcher.cancel();
throw e;
}
await result;
const error = await navigationPromise;
helper.removeEventListeners([listener]);
if (error)
throw error;
if (this._frameManager.isMainFrameLoadingFailed())
throw new Error('Failed to navigate: ' + url);
return responses.get(this.mainFrame().url()) || null;
@ -410,9 +412,10 @@ class Page extends EventEmitter {
const responses = new Map();
const listener = helper.addEventListener(this._networkManager, NetworkManager.Events.Response, response => responses.set(response.url, response));
await watcher.waitForNavigation();
const error = await watcher.waitForNavigation();
helper.removeEventListeners([listener]);
if (error)
throw error;
return responses.get(this.mainFrame().url()) || null;
}

View File

@ -598,15 +598,16 @@ describe('Page', function() {
expect(error.message).toContain('Failed to navigate');
}));
it('should fail when exceeding maximum navigation timeout', SX(async function() {
let error = null;
let hasUnhandledRejection = false;
const unhandledRejectionHandler = () => hasUnhandledRejection = true;
process.on('unhandledRejection', unhandledRejectionHandler);
// Hang for request to the empty.html
server.setRoute('/empty.html', (req, res) => { });
try {
await page.goto(PREFIX + '/empty.html', {timeout: 59});
} catch (e) {
error = e;
}
expect(error.message).toContain('Navigation Timeout Exceeded: 59ms');
let error = null;
await page.goto(PREFIX + '/empty.html', {timeout: 1}).catch(e => error = e);
expect(hasUnhandledRejection).toBe(false);
expect(error.message).toContain('Navigation Timeout Exceeded: 1ms');
process.removeListener('unhandledRejection', unhandledRejectionHandler);
}));
it('should work when navigating to valid url', SX(async function() {
const response = await page.goto(EMPTY_PAGE);