Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix errors inside spawnPromise being ignored #2939

Merged

Conversation

alexhorn
Copy link
Contributor

If an error inside spawnPromise occurs it is being ignored as the async function is passed to new Promise(...) as a regular function.

This leads to the following output if you pass an empty string for chromePath:

(node:19352) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Bad argument
(node:19352) DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

great catch thanks very much for the PR @alexhorn!

would you mind adding a test for this behavior in chrome-launcher/test/chrome-launcher-test.ts so we don't regress?

@alexhorn
Copy link
Contributor Author

I have now added a test

@alexhorn
Copy link
Contributor Author

@patrickhulce Is this ok?

@patrickhulce
Copy link
Collaborator

@alexhorn yep looks great thanks for the ping and the contribution! :)

@patrickhulce patrickhulce merged commit e958c97 into GoogleChrome:master Aug 18, 2017
paulirish pushed a commit to GoogleChrome/chrome-launcher that referenced this pull request Aug 29, 2017
…#2939)

* Fix errors inside spawnPromise being ignored

* Add unit test for empty chromePath

* Fix formatting

* Trigger rebuild
paulirish pushed a commit to GoogleChrome/chrome-launcher that referenced this pull request Aug 29, 2017
…#2939)

* Fix errors inside spawnPromise being ignored

* Add unit test for empty chromePath

* Fix formatting

* Trigger rebuild
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