fix(Network): be prepared to miss requestWillBeSent events (#1420)
With the addition of `browser.targets()` api, we now can connect to in-flight targets. For Puppeteer, it means that it can "miss" certain events happenning while it wasn't attached to the target. This patch: - fixes this problem with NetworkManager, preparing it for the missed `requestWillBeSent` event. - adds a new test to ensure that not a single unhandled promise rejection has happened during test execution. Fixes #1363.
This commit is contained in:
parent
ea70ac9003
commit
cafd040bf2
@ -218,7 +218,9 @@ class NetworkManager extends EventEmitter {
|
|||||||
}
|
}
|
||||||
if (event.redirectResponse) {
|
if (event.redirectResponse) {
|
||||||
const request = this._requestIdToRequest.get(event.requestId);
|
const request = this._requestIdToRequest.get(event.requestId);
|
||||||
this._handleRequestRedirect(request, event.redirectResponse.status, event.redirectResponse.headers);
|
// If we connect late to the target, we could have missed the requestWillBeSent event.
|
||||||
|
if (request)
|
||||||
|
this._handleRequestRedirect(request, event.redirectResponse.status, event.redirectResponse.headers);
|
||||||
}
|
}
|
||||||
this._handleRequestStart(event.requestId, null, event.request.url, event.type, event.request);
|
this._handleRequestStart(event.requestId, null, event.request.url, event.type, event.request);
|
||||||
}
|
}
|
||||||
|
51
test/test.js
51
test/test.js
@ -26,6 +26,7 @@ const SimpleServer = require('./server/SimpleServer');
|
|||||||
const GoldenUtils = require('./golden-utils');
|
const GoldenUtils = require('./golden-utils');
|
||||||
|
|
||||||
const YELLOW_COLOR = '\x1b[33m';
|
const YELLOW_COLOR = '\x1b[33m';
|
||||||
|
const RED_COLOR = '\x1b[31m';
|
||||||
const RESET_COLOR = '\x1b[0m';
|
const RESET_COLOR = '\x1b[0m';
|
||||||
|
|
||||||
const GOLDEN_DIR = path.join(__dirname, 'golden');
|
const GOLDEN_DIR = path.join(__dirname, 'golden');
|
||||||
@ -64,6 +65,25 @@ else
|
|||||||
console.assert(revisionInfo.downloaded, `Chromium r${chromiumRevision} is not downloaded. Run 'npm install' and try to re-run tests.`);
|
console.assert(revisionInfo.downloaded, `Chromium r${chromiumRevision} is not downloaded. Run 'npm install' and try to re-run tests.`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Hack to get the currently-running spec name.
|
||||||
|
let specName = null;
|
||||||
|
jasmine.getEnv().addReporter({
|
||||||
|
specStarted: result => specName = result.fullName
|
||||||
|
});
|
||||||
|
|
||||||
|
// Setup unhandledRejectionHandlers
|
||||||
|
let hasUnhandledRejection = false;
|
||||||
|
process.on('unhandledRejection', error => {
|
||||||
|
hasUnhandledRejection = true;
|
||||||
|
const textLines = [
|
||||||
|
'',
|
||||||
|
`${RED_COLOR}[UNHANDLED PROMISE REJECTION]${RESET_COLOR} "${specName}"`,
|
||||||
|
error.stack,
|
||||||
|
'',
|
||||||
|
];
|
||||||
|
console.error(textLines.join('\n'));
|
||||||
|
});
|
||||||
|
|
||||||
let server;
|
let server;
|
||||||
let httpsServer;
|
let httpsServer;
|
||||||
beforeAll(SX(async function() {
|
beforeAll(SX(async function() {
|
||||||
@ -1001,16 +1021,11 @@ describe('Page', function() {
|
|||||||
expect(error.message).toContain('net::ERR_CONNECTION_REFUSED');
|
expect(error.message).toContain('net::ERR_CONNECTION_REFUSED');
|
||||||
}));
|
}));
|
||||||
it('should fail when exceeding maximum navigation timeout', SX(async function() {
|
it('should fail when exceeding maximum navigation timeout', SX(async function() {
|
||||||
let hasUnhandledRejection = false;
|
|
||||||
const unhandledRejectionHandler = () => hasUnhandledRejection = true;
|
|
||||||
process.on('unhandledRejection', unhandledRejectionHandler);
|
|
||||||
// Hang for request to the empty.html
|
// Hang for request to the empty.html
|
||||||
server.setRoute('/empty.html', (req, res) => { });
|
server.setRoute('/empty.html', (req, res) => { });
|
||||||
let error = null;
|
let error = null;
|
||||||
await page.goto(PREFIX + '/empty.html', {timeout: 1}).catch(e => error = e);
|
await page.goto(PREFIX + '/empty.html', {timeout: 1}).catch(e => error = e);
|
||||||
expect(hasUnhandledRejection).toBe(false);
|
|
||||||
expect(error.message).toContain('Navigation Timeout Exceeded: 1ms');
|
expect(error.message).toContain('Navigation Timeout Exceeded: 1ms');
|
||||||
process.removeListener('unhandledRejection', unhandledRejectionHandler);
|
|
||||||
}));
|
}));
|
||||||
it('should disable timeout when its set to 0', SX(async function() {
|
it('should disable timeout when its set to 0', SX(async function() {
|
||||||
let error = null;
|
let error = null;
|
||||||
@ -3239,9 +3254,35 @@ describe('Page', function() {
|
|||||||
await evaluatePromise;
|
await evaluatePromise;
|
||||||
await newPage.close();
|
await newPage.close();
|
||||||
}));
|
}));
|
||||||
|
it('should not crash while redirecting if original request was missed', SX(async function() {
|
||||||
|
let serverResponse = null;
|
||||||
|
server.setRoute('/one-style.css', (req, res) => serverResponse = res);
|
||||||
|
// Open a new page. Use window.open to connect to the page later.
|
||||||
|
await Promise.all([
|
||||||
|
page.evaluate(url => window.open(url), PREFIX + '/one-style.html'),
|
||||||
|
server.waitForRequest('/one-style.css')
|
||||||
|
]);
|
||||||
|
// Connect to the opened page.
|
||||||
|
const target = browser.targets().find(target => target.url().includes('one-style.html'));
|
||||||
|
const newPage = await target.page();
|
||||||
|
// Issue a redirect.
|
||||||
|
serverResponse.writeHead(302, { location: '/injectedstyle.css' });
|
||||||
|
serverResponse.end();
|
||||||
|
// Wait for the new page to load.
|
||||||
|
await waitForEvents(newPage, 'load');
|
||||||
|
|
||||||
|
expect(hasUnhandledRejection).toBe(false);
|
||||||
|
|
||||||
|
// Cleanup.
|
||||||
|
await newPage.close();
|
||||||
|
}));
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('Unhandled promise rejections should not be thrown', function() {
|
||||||
|
expect(hasUnhandledRejection).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
if (process.env.COVERAGE) {
|
if (process.env.COVERAGE) {
|
||||||
describe('COVERAGE', function(){
|
describe('COVERAGE', function(){
|
||||||
const coverage = helper.publicAPICoverage();
|
const coverage = helper.publicAPICoverage();
|
||||||
|
Loading…
Reference in New Issue
Block a user