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

Add unhandledRejectionHandler #3

Merged
merged 5 commits into from
Aug 3, 2017
Merged

Add unhandledRejectionHandler #3

merged 5 commits into from
Aug 3, 2017

Conversation

simonepri
Copy link

Unhandled rejections shouldn't be handled in the same callback of uncaught exceptions for 2 main reasons:

  1. It's undocumented!!!
  2. Users cannot easly distinguish them

This PR solves the problem adding another dedicated handler function

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 97.436% when pulling 1053e16 on simonepri:patch-2 into c9605dd on Tapppi:master.

1 similar comment
@coveralls
Copy link

coveralls commented Aug 2, 2017

Coverage Status

Coverage increased (+0.1%) to 97.436% when pulling 1053e16 on simonepri:patch-2 into c9605dd on Tapppi:master.

@simonepri
Copy link
Author

@Tapppi please enable Issues in the settings of this repo.
Are some ideas that I want to propose, but I'm unable to open an Issue. 👎

@Tapppi
Copy link
Owner

Tapppi commented Aug 3, 2017

@simonepri I have enabled issues, feel free to propose changes.

As for this PR, I like the idea, but can you give your thought process for removing unhandled rejections from the error handler? I am not calling it a bad change, just want to verify the justification as it makes this a major version bump ☺️

@Tapppi
Copy link
Owner

Tapppi commented Aug 3, 2017

Actually, you already gave your justification. I will merge this and release it as a new major version!

@Tapppi Tapppi merged commit 96a194f into Tapppi:master Aug 3, 2017
@simonepri
Copy link
Author

simonepri commented Aug 3, 2017

@Tapppi Thank you.

@Tapppi
Copy link
Owner

Tapppi commented Aug 3, 2017

@simonepri v2.0.0 has been released to npm

@simonepri
Copy link
Author

@Tapppi the build is failing.
I don't think adding the package-lock.json file is a good idea, although it is recommended.

@Tapppi
Copy link
Owner

Tapppi commented Aug 3, 2017

@simonepri I updated the style of the repository, which caused node v4 to fail tests because of a missing 'use strict';. Fixed it already.

@simonepri
Copy link
Author

simonepri commented Aug 3, 2017

Good, currently I'm improving the documentation a bit.
I will open a PR when I'm ready 👍

@simonepri simonepri deleted the patch-2 branch August 3, 2017 10:28
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.

3 participants