Crash when failed to run

[denis-sokolov]

Before the script silently returned success when the running has failed.

Workflows that relied on PR integration with Percy had received a failure from Percy,
but independent workflows would silently pass until anyone noticed a failure.

This is breaking backwards compatibility!

[Robdel12]

Hey @denis-sokolov! PercyScript is just a thin wrapper around Puppeteer. Unless you add assertions to your PercyScript (or it reaches an exception while running the script) it will never exit anything other than 0 (pass). If you would like to wait for the Percy build to finish processing and then pass/fail in CI, you will need to setup Webhooks in CI and wait for the build_fiinshed event. Then determine what to exit CI with from the result of the Percy build. Otherwise, you can install one of our SCM integrations which will add commit/PR status checks.

[denis-sokolov]

I may have been unclear about what is happening. I don’t want to catch the Percy build processing, I want to receive catastrophic errors during the process of running the Puppeteer. The claim that it will never exit with anything other than 0 is exactly the behavior I am trying to change.

For instance, if my page.goTo has a misconfigured URL, Puppeteer correctly crashes with a Connection Refused error. But instead of receiving a warning about a failing build and a report of the error, my build shows a nice green ✅, because percy-script swallows the error.

Besides, today already percy-script may return with non-0 return codes, if I have other catastrophic errors. For instance, if my node modules were not installed correctly, require on line 1 will throw and the script will exit with non-0 error code, signifying that it did not finish its job correctly. May we get the same behavior for more useful errors in Puppeteer?

[Robdel12]

Ah right, I replied way to early in the morning. If you update the PR to throw an error rather than deleting the catch, I’ll happily merge it through

[denis-sokolov]

try/finally without the catch does rethrow the error by itself already, though!

function crash(){ throw new Error('crash'); }

function main() {
  try {
    crash();
  } finally {
    console.log('finally');
  }
}

main();

[Robdel12]

I merged through a fix for this in #11. Thanks for raising this! And sorry about the confusion, I should have stayed away from GitHub on my day off :p