Skip to content

Conversation

@homaily
Copy link
Contributor

@homaily homaily commented Aug 30, 2017

Hi,

When Chromium download is failed (no internet connection for example) downloadRevision will not emit an error and therefore onError function in the install file will not be triggered and the user will not see the message:

ERROR: Failed to download chromium ....

I'm using Node v6.11.2

@aslushnikov
Copy link
Contributor

This would play nice with #606

Codewise, we should probably process.exit(1) to terminate the script with non-zero exit code

@homaily
Copy link
Contributor Author

homaily commented Aug 31, 2017

We can do the process.exit(1) from the onError function

@aslushnikov
Copy link
Contributor

@homaily any updates on this? I'm very interested in this PR.

@homaily
Copy link
Contributor Author

homaily commented Sep 1, 2017

@aslushnikov Sorry didn't get your point. Do you want the process.exit in the onError on int the place of throw Error?

Copy link
Contributor

@aslushnikov aslushnikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, I got confused. I thought something else is going on here.

.catch(err => err)
.catch(err => { throw err; })
.then(() => {
if (fs.existsSync(zipPath))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, there's no "finally" in promise chains, so I'd fake it like this:

  .catch(err => err)
  .then(err => {
    if (fs.existsSync(zipPath))
      fs.unlinkSync(zipPath);
    if (err)
      throw err;
  });

This way we'll throw an error and cleanup zip if any

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better love it. I've updated my code to do that.

Finishing the promise chain to remove the partially downloaded zip file (if exists)
Copy link
Contributor

@aslushnikov aslushnikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@aslushnikov aslushnikov merged commit e95fb96 into puppeteer:master Sep 1, 2017
@homaily
Copy link
Contributor Author

homaily commented Sep 1, 2017

You're welcome Andrey. Appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants