fix(page): consoleMessage.location() should work with workers (#3752)

This patch:
- refactors consoleMessage.location() implementation to
make it work for workers
- re-writes tests to avoid 5 second delay
This commit is contained in:
Andrey Lushnikov 2019-01-10 18:05:28 -08:00 committed by GitHub
parent 0c867631b0
commit 89fc2adff5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 50 additions and 63 deletions

View File

@ -2318,9 +2318,9 @@ puppeteer.launch().then(async browser => {
#### consoleMessage.location() #### consoleMessage.location()
- returns: <[Object]> - returns: <[Object]>
- `url` <[string]> URL of the resource if known - `url` <[string]> URL of the resource if known or `undefined` otherwise.
- `lineNumber` <[number]> line number in the resource - `lineNumber` <[number]> line number in the resource if known or `undefined` otherwise.
- `columnNumber` <[number]> line number in the resource - `columnNumber` <[number]> column number in the resource if known or `undefined` otherwise.
#### consoleMessage.text() #### consoleMessage.text()
- returns: <[string]> - returns: <[string]>

View File

@ -191,7 +191,7 @@ class Page extends EventEmitter {
if (args) if (args)
args.map(arg => helper.releaseObject(this._client, arg)); args.map(arg => helper.releaseObject(this._client, arg));
if (source !== 'worker') if (source !== 'worker')
this.emit(Page.Events.Console, new ConsoleMessage(level, text, undefined, { url, lineNumber })); this.emit(Page.Events.Console, new ConsoleMessage(level, text, [], {url, lineNumber}));
} }
/** /**
@ -510,14 +510,7 @@ class Page extends EventEmitter {
async _onConsoleAPI(event) { async _onConsoleAPI(event) {
const context = this._frameManager.executionContextById(event.executionContextId); const context = this._frameManager.executionContextById(event.executionContextId);
const values = event.args.map(arg => createJSHandle(context, arg)); const values = event.args.map(arg => createJSHandle(context, arg));
const location = {}; this._addConsoleMessage(event.type, values, event.stackTrace);
if (event.stackTrace && event.stackTrace.callFrames) {
const {url, lineNumber, columnNumber} = event.stackTrace.callFrames[0];
if (url) location.url = url;
if (lineNumber) location.lineNumber = lineNumber;
if (columnNumber) location.columnNumber = columnNumber;
}
this._addConsoleMessage(event.type, values, location);
} }
/** /**
@ -574,9 +567,9 @@ class Page extends EventEmitter {
/** /**
* @param {string} type * @param {string} type
* @param {!Array<!Puppeteer.JSHandle>} args * @param {!Array<!Puppeteer.JSHandle>} args
* @param {ConsoleMessage.Location} location * @param {Protocol.Runtime.StackTrace=} stackTrace
*/ */
_addConsoleMessage(type, args, location) { _addConsoleMessage(type, args, stackTrace) {
if (!this.listenerCount(Page.Events.Console)) { if (!this.listenerCount(Page.Events.Console)) {
args.forEach(arg => arg.dispose()); args.forEach(arg => arg.dispose());
return; return;
@ -589,6 +582,11 @@ class Page extends EventEmitter {
else else
textTokens.push(helper.valueFromRemoteObject(remoteObject)); textTokens.push(helper.valueFromRemoteObject(remoteObject));
} }
const location = stackTrace && stackTrace.callFrames.length ? {
url: stackTrace.callFrames[0].url,
lineNumber: stackTrace.callFrames[0].lineNumber,
columnNumber: stackTrace.callFrames[0].columnNumber,
} : {};
const message = new ConsoleMessage(type, textTokens.join(' '), args, location); const message = new ConsoleMessage(type, textTokens.join(' '), args, location);
this.emit(Page.Events.Console, message); this.emit(Page.Events.Console, message);
} }
@ -1248,7 +1246,7 @@ class ConsoleMessage {
* @param {!Array<!Puppeteer.JSHandle>} args * @param {!Array<!Puppeteer.JSHandle>} args
* @param {ConsoleMessage.Location} location * @param {ConsoleMessage.Location} location
*/ */
constructor(type, text, args = [], location = {}) { constructor(type, text, args, location = {}) {
this._type = type; this._type = type;
this._text = text; this._text = text;
this._args = args; this._args = args;

View File

@ -21,7 +21,7 @@ class Worker extends EventEmitter {
/** /**
* @param {Puppeteer.CDPSession} client * @param {Puppeteer.CDPSession} client
* @param {string} url * @param {string} url
* @param {function(!string, !Array<!JSHandle>)} consoleAPICalled * @param {function(!string, !Array<!JSHandle>, Protocol.Runtime.StackTrace=)} consoleAPICalled
* @param {function(!Protocol.Runtime.ExceptionDetails)} exceptionThrown * @param {function(!Protocol.Runtime.ExceptionDetails)} exceptionThrown
*/ */
constructor(client, url, consoleAPICalled, exceptionThrown) { constructor(client, url, consoleAPICalled, exceptionThrown) {
@ -39,7 +39,7 @@ class Worker extends EventEmitter {
// This might fail if the target is closed before we recieve all execution contexts. // This might fail if the target is closed before we recieve all execution contexts.
this._client.send('Runtime.enable', {}).catch(debugError); this._client.send('Runtime.enable', {}).catch(debugError);
this._client.on('Runtime.consoleAPICalled', event => consoleAPICalled(event.type, event.args.map(jsHandleFactory))); this._client.on('Runtime.consoleAPICalled', event => consoleAPICalled(event.type, event.args.map(jsHandleFactory), event.stackTrace));
this._client.on('Runtime.exceptionThrown', exception => exceptionThrown(exception.exceptionDetails)); this._client.on('Runtime.exceptionThrown', exception => exceptionThrown(exception.exceptionDetails));
} }

View File

@ -1,11 +0,0 @@
<!DOCTYPE html>
<html>
<head>
<title>Log Entry Test</title>
</head>
<body>
<script>
fetch('http://wat')
</script>
</body>
</html>

View File

@ -1,11 +0,0 @@
<!DOCTYPE html>
<html>
<head>
<title>Log Entry Test</title>
</head>
<body>
<script>
console.warn('wat')
</script>
</body>
</html>

View File

@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
<head>
<title>console.log test</title>
</head>
<body>
<script>
console.log('yellow')
</script>
</body>
</html>

View File

@ -322,16 +322,12 @@ module.exports.addTests = function({testRunner, expect, headless}) {
expect(message.text()).toContain('No \'Access-Control-Allow-Origin\''); expect(message.text()).toContain('No \'Access-Control-Allow-Origin\'');
expect(message.type()).toEqual('error'); expect(message.type()).toEqual('error');
}); });
it('should show correct additional info when console event emitted for logEntry', async({page, server}) => { it('should have location when fetch fails', async({page, server}) => {
let message = null; await page.goto(server.EMPTY_PAGE);
page.on('console', msg => { const [message] = await Promise.all([
message = msg;
});
await Promise.all([
page.goto(server.PREFIX + '/console-message-1.html'),
waitEvent(page, 'console'), waitEvent(page, 'console'),
page.setContent(`<script>fetch('http://wat');</script>`),
]); ]);
await new Promise(resolve => setTimeout(resolve, 5000));
expect(message.text()).toContain(`ERR_NAME_NOT_RESOLVED`); expect(message.text()).toContain(`ERR_NAME_NOT_RESOLVED`);
expect(message.type()).toEqual('error'); expect(message.type()).toEqual('error');
expect(message.location()).toEqual({ expect(message.location()).toEqual({
@ -339,22 +335,18 @@ module.exports.addTests = function({testRunner, expect, headless}) {
lineNumber: undefined lineNumber: undefined
}); });
}); });
it('should show correct additional info when console event emitted for consoleAPI', async({page, server}) => { it('should have location for console API calls', async({page, server}) => {
let message = null; await page.goto(server.EMPTY_PAGE);
page.on('console', msg => { const [message] = await Promise.all([
message = msg;
});
await Promise.all([
page.goto(server.PREFIX + '/console-message-2.html'),
waitEvent(page, 'console'), waitEvent(page, 'console'),
page.goto(server.PREFIX + '/consolelog.html'),
]); ]);
await new Promise(resolve => setTimeout(resolve, 5000)); expect(message.text()).toBe('yellow');
expect(message.text()).toContain(`wat`); expect(message.type()).toBe('log');
expect(message.type()).toEqual('warning');
expect(message.location()).toEqual({ expect(message.location()).toEqual({
url: `http://localhost:${server.PORT}/console-message-2.html`, url: server.PREFIX + '/consolelog.html',
lineNumber: 7, lineNumber: 7,
columnNumber: 16 columnNumber: 14,
}); });
}); });
}); });

View File

@ -1,3 +1,5 @@
const utils = require('./utils');
const {waitEvent} = utils;
module.exports.addTests = function({testRunner, expect}) { module.exports.addTests = function({testRunner, expect}) {
const {describe, xdescribe, fdescribe} = testRunner; const {describe, xdescribe, fdescribe} = testRunner;
@ -29,10 +31,16 @@ module.exports.addTests = function({testRunner, expect}) {
expect(error.message).toContain('Most likely the worker has been closed.'); expect(error.message).toContain('Most likely the worker has been closed.');
}); });
it('should report console logs', async function({page}) { it('should report console logs', async function({page}) {
const logPromise = new Promise(x => page.on('console', x)); const [message] = await Promise.all([
await page.evaluate(() => new Worker(`data:text/javascript,console.log(1)`)); waitEvent(page, 'console'),
const log = await logPromise; page.evaluate(() => new Worker(`data:text/javascript,console.log(1)`)),
expect(log.text()).toBe('1'); ]);
expect(message.text()).toBe('1');
expect(message.location()).toEqual({
url: 'data:text/javascript,console.log(1)',
lineNumber: 0,
columnNumber: 8,
});
}); });
it('should have JSHandles for console logs', async function({page}) { it('should have JSHandles for console logs', async function({page}) {
const logPromise = new Promise(x => page.on('console', x)); const logPromise = new Promise(x => page.on('console', x));