Detecting errors during resource load

[jchappell82]

Hi - I've been looking at switching to this library from one that uses an older PhantomJS, but noticed that it doesn't seem to handle resource load errors at all.

For example:

An SVG containing an inner <image /> tag, and the image fails to load, with perhaps a 404, or a 500 response (because something is going to fail eventually). Currently that results in a render of Chrome's lovely "broken image" icon, scaled up to whatever dimensions the SVG defined. - like this:

In PhantomJS, it was easy to detect by monitoring onResourceReceived similar to this:

    page.onResourceReceived = function (response) {
      if (response.status >= 400) {
        console.error("Error during retrieval of " + response.url);
        phantom.exit()
        return;
      }
    }

I'm open to looking into helping to add this functionality, but would be curious whether you have a preferred approach. (e.g. An option to failOnResourceError, or the making it the default behavior, or something else)

It's not immediately obvious whether Puppeteer exposes enough information to determine a failure on something like this, but it looks like request.failure() might be the first place to start. Or also perhaps the requestFailed event on Page

[jchappell82]

I actually have this mostly working now in my fork (without an option, currently), though I'm struggling getting most of the conversion tests to pass even from the master (unmodified) branch. Not sure if if that's due to my OS (macOS 10.13.4) or something else I'm missing just yet, but I'll let you know, assuming you're interested in taking a look and getting started towards a pull request.

[neocotic]

@jchappell82 I'm definitely interested this. I believe that an error should result in an error. Although it's breaking change technically. Not an issue since we're still in 0.x releases though. I'll just need to bump the minor version.

The problem with the tests is an annoying one. Basically, the tests compare actual converted images against an expected image output. Unfortunately, each different version of Chromium appears to produce difference different output, although visually identical, to me at least. I don't have a huge experience with image conversion testing, so I'm not sure if there's a better means of doing this that wouldn't result in continuously updating the expected files. It's effectively breaking the tests because I can only fix it be regenerating the actual files, comparing them manually by eye, and then updating the expected images. Not ideal.

This is especially a problem since we just depend on puppeteer which could update different versions of Chromium. Perhaps we need to depend on a fixed version of puppeteer instead of a version range.

[jchappell82]

Sounds great - I'll open a pull request a bit later on for discussion and iteration purposes. It wasn't immediately obvious to me how to solve an issue around throwing during an event handler generating an unhandled promise rejection (it seems almost as if it's causing something outside of the scope of the library's code to reject), but it's at least a starting point. In particular, and somewhat selfishly, its current state solves an issue on my end where I simply must not ever allow an incomplete render to happen.