fix: emit all arguments from worker console logs (#2697)

Log.entryAdded doesn't report all the arguments from console logs. This PR switches to use Runtime.consoleAPICalled.

fixes #2640

lib/Page.js

@@ -101,7 +101,7 @@ class Page extends EventEmitter {
         return;
       }
       const session = client._createSession(event.targetInfo.type, event.sessionId);
-      const worker = new Worker(session, event.targetInfo.url, this._onLogEntryAdded.bind(this, session));
+      const worker = new Worker(session, event.targetInfo.url, this._addConsoleMessage.bind(this));
       this._workers.set(event.sessionId, worker);
       this.emit(Page.Events.WorkerCreated, worker);
 
@@ -131,7 +131,7 @@ class Page extends EventEmitter {
     client.on('Security.certificateError', event => this._onCertificateError(event));
     client.on('Inspector.targetCrashed', event => this._onTargetCrashed());
     client.on('Performance.metrics', event => this._emitMetrics(event));
-    client.on('Log.entryAdded', event => this._onLogEntryAdded(this._client, event));
+    client.on('Log.entryAdded', event => this._onLogEntryAdded(event));
     this._target._isClosedPromise.then(() => {
       this.emit(Page.Events.Close);
       this._closed = true;
@@ -157,15 +157,14 @@ class Page extends EventEmitter {
   }
 
   /**
-   * @param {!Puppeteer.CDPSession} session
    * @param {!Protocol.Log.entryAddedPayload} event
    */
-  _onLogEntryAdded(session, event) {
-    const {level, text, args} = event.entry;
+  _onLogEntryAdded(event) {
+    const {level, text, args, source} = event.entry;
     if (args)
-      args.map(arg => helper.releaseObject(session, arg));
-
-    this.emit(Page.Events.Console, new ConsoleMessage(level, text));
+      args.map(arg => helper.releaseObject(this._client, arg));
+    if (source !== 'worker')
+      this.emit(Page.Events.Console, new ConsoleMessage(level, text));
   }
 
   /**
@@ -470,6 +469,9 @@ class Page extends EventEmitter {
     this.emit(Page.Events.PageError, err);
   }
 
+  /**
+   * @param {!Protocol.Runtime.consoleAPICalledPayload} event
+   */
   async _onConsoleAPI(event) {
     if (event.type === 'debug' && event.args.length && event.args[0].value === 'driver:page-binding') {
       const {name, seq, args} = JSON.parse(event.args[1].value);
@@ -483,20 +485,28 @@ class Page extends EventEmitter {
       }
       return;
     }
+    const values = event.args.map(arg => this._frameManager.createJSHandle(event.executionContextId, arg));
+    this._addConsoleMessage(event.type, values);
+  }
+
+  /**
+   * @param {string} type
+   * @param {!Array<!Puppeteer.JSHandle>} args
+   */
+  _addConsoleMessage(type, args) {
     if (!this.listenerCount(Page.Events.Console)) {
-      event.args.map(arg => helper.releaseObject(this._client, arg));
+      args.forEach(arg => arg.dispose());
       return;
     }
-    const values = event.args.map(arg => this._frameManager.createJSHandle(event.executionContextId, arg));
     const textTokens = [];
-    for (let i = 0; i < event.args.length; ++i) {
-      const remoteObject = event.args[i];
+    for (const arg of args) {
+      const remoteObject = arg._remoteObject;
       if (remoteObject.objectId)
-        textTokens.push(values[i].toString());
+        textTokens.push(arg.toString());
       else
         textTokens.push(helper.valueFromRemoteObject(remoteObject));
     }
-    const message = new ConsoleMessage(event.type, textTokens.join(' '), values);
+    const message = new ConsoleMessage(type, textTokens.join(' '), args);
     this.emit(Page.Events.Console, message);
   }
 
@@ -1115,7 +1125,7 @@ class ConsoleMessage {
   /**
    * @param {string} type
    * @param {string} text
-   * @param {!Array<*>} args
+   * @param {!Array<!Puppeteer.JSHandle>} args
    */
   constructor(type, text, args = []) {
     this._type = type;
@@ -1138,7 +1148,7 @@ class ConsoleMessage {
   }
 
   /**
-   * @return {!Array<string>}
+   * @return {!Array<!Puppeteer.JSHandle>}
    */
   args() {
     return this._args;

lib/Worker.js

@@ -21,23 +21,24 @@ class Worker extends EventEmitter {
   /**
    * @param {Puppeteer.CDPSession} client
    * @param {string} url
-   * @param {function(!Protocol.Log.entryAddedPayload)} logEntryAdded
+   * @param {function(!string, !Array<!JSHandle>)} consoleAPICalled
    */
-  constructor(client, url, logEntryAdded) {
+  constructor(client, url, consoleAPICalled) {
     super();
     this._client = client;
     this._url = url;
     this._executionContextPromise = new Promise(x => this._executionContextCallback = x);
+    /** @type {function(!Protocol.Runtime.RemoteObject):!JSHandle} */
+    let jsHandleFactory;
     this._client.once('Runtime.executionContextCreated', async event => {
-      const jsHandleFactory = remoteObject => new JSHandle(executionContext, client, remoteObject);
+      jsHandleFactory = remoteObject => new JSHandle(executionContext, client, remoteObject);
       const executionContext = new ExecutionContext(client, event.context, jsHandleFactory, null);
       this._executionContextCallback(executionContext);
     });
     // This might fail if the target is closed before we recieve all execution contexts.
     this._client.send('Runtime.enable', {}).catch(debugError);
 
-    this._client.on('Log.entryAdded', logEntryAdded);
-    this._client.send('Log.enable', {}).catch(debugError);
+    this._client.on('Runtime.consoleAPICalled', event => consoleAPICalled(event.type, event.args.map(jsHandleFactory)));
   }
 
   /**

test/worker.spec.js

@@ -34,6 +34,14 @@ module.exports.addTests = function({testRunner, expect}) {
       const log = await logPromise;
       expect(log.text()).toBe('1');
     });
+    it('should have JSHandles for console logs', async function({page}) {
+      const logPromise = new Promise(x => page.on('console', x));
+      await page.evaluate(() => new Worker(`data:text/javascript,console.log(1,2,3,this)`));
+      const log = await logPromise;
+      expect(log.text()).toBe('1 2 3 JSHandle@object');
+      expect(log.args().length).toBe(4);
+      expect(await (await log.args()[3].getProperty('origin')).jsonValue()).toBe('null');
+    });
     it('should have an execution context', async function({page}) {
       const workerCreatedPromise = new Promise(x => page.once('workercreated', x));
       await page.evaluate(() => new Worker(`data:text/javascript,console.log(1)`));