Evaluate silently masking errors

[aaronmefford]

Steps to reproduce

Tell us about your environment:

• Puppeteer version: 1.5.0
• Platform / OS version: Amazon Linux 4.14.33-51.37.amzn1.x86_64
• URLs (if applicable): https://news.google.com
• Node.js version: v8.11.3
• Chrome version: HeadlessChrome/68.0.3438.0

What steps will reproduce the problem?

Please include code that reproduces the issue.

const puppeteer = require('puppeteer');

options = {
    executablePath: '/opt/chrome/headless_shell'
};

( async () => {
    const browser = await puppeteer.launch(options);

    let page = await browser.newPage();
    await page.goto('https://news.google.com/');
    let articles = await page.evaluate(()=>{
        return Array.from(document.querySelectorAll('article'))
    });
    console.log("Articles: ", articles);
    await page.close();
    await browser.close();
})();

What is the expected result?
A list of article elements converted to JavaScript objects via JSON.

What happens instead?
An undefined value is returned without error.

The error
(node:25073) UnhandledPromiseRejectionWarning: Error: Protocol error (Runtime.callFunctionOn): Object reference chain is too long undefined

is being captured and silently discarded in the evaluate method.

  async evaluate(pageFunction, ...args) {
    const handle = await this.evaluateHandle(pageFunction, ...args);
    const result = await handle.jsonValue().catch(error => {
      if (error.message.includes('Object reference chain is too long'))
        return;
      if (error.message.includes('Object couldn\'t be returned by value'))
        return;
      throw error;
    });
    await handle.dispose();
    return result;
  }

This cost me a substantial amount of time converting from 0.12 to 1.50, working code suddenly did not work and did not work silently masking the errors that were occuring.

I have to say I am not a huge fan of the the *Handle changes. I can see their value in some cases, but the extent to which their use has been propagated throughout the Puppeteer codebase has made what used to be straight forward a nightmare. Simple things like dumping a variable to see if you selected the right element is complicated where it should be simple.

The method above is a perfect example. I should have the option to use the evaluateHandle method if I want to get a *Handle result, or use the evaluate method otherwise when I need raw results. However rather than evaluateHandle being a wrapper around what should be the raw method, instead evaluate is a wrapper around the more complicated evaluateHandle. The evaluate method should not use the evaluateHandle method or other complicated intermediaries, but rather should behave more like the devtools Runtime.evaluate method it is intended to wrap.

Because of this situation I was forced to write my own low level evaluate to do what evaluate used to do so well. I cannot say that it is as robust as what might be needed in puppeteer but it has allowed me to continue working.

    async raw_evaluate(obj,func) {
        if ( typeof func == 'function') {
            func = func.toString();
            func = "("+func+")();";
        }
        const { exceptionDetails, result: remoteObject} = await obj._client.send('Runtime.evaluate', {
            expression:    func,
            contextId:     obj._contextId,
            awaitPromise:  true,
            returnByValue: true,
        });
        if ( exceptionDetails)  {
            console.error(exceptionDetails);
            throw new Error('Evaluation Failed: '+ exceptionDetails.message);
        }
        return remoteObject.value;
    }

The main issue I am running into with the *Handle comes back to jsonValue(), which is failing because the chain is too long or because of circular references. Unfortunately we do not always control the code we are processing, and circular references are common in the modern DOM.

[aaronmefford]

If it is helpful I will write a pull request that either exposes the errors, or modifies the evaluate to work more like the Runtime.evaluate method.

[aslushnikov]

is being captured and silently discarded in the evaluate method.

@aaronmefford We'll start throwing in 2.0 release since it'll be a breaking change. See #2079.

The main issue I am running into with the *Handle comes back to jsonValue(), which is failing because the chain is too long or because of circular references.

How's jsonValue different from what you're doing in your raw_evaluate?