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

Configurable default catch handler #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

askot
Copy link

@askot askot commented Mar 18, 2016

This patch allows initializing reflux-promise with default promise catch handler.

If function is not provided, no catch handler is not attached by reflux-promise. Which is important, because you might want to use error handling of your promise implementation instead.

For example, Bluebird fires an unhandledRejection event on unhandled exceptions. With current catch handler implementation this is never thrown.

Thoughts?

@devinivy
Copy link
Contributor

I like this approach. It will be a breaking change, so I'd like to consider some other positive API changes as well– let me know if you have any ideas. Also– do you have the time to add tests for this feature? If not, we can make an issue for it and I can add the tests when I find some time.

@ponelat
Copy link

ponelat commented Mar 24, 2016

+1 can we get this merged?

@devinivy
Copy link
Contributor

devinivy commented May 4, 2016

Anyone down to write tests for this?

@askot
Copy link
Author

askot commented May 8, 2016

sorry for the long delay, finally managed to play around a bit with tests. I was able to construct a pretty understandable test case (IMO), please review.

@semikolon
Copy link
Contributor

Looks good to me!

Would like to see this merged before I use this plugin more in my current project...

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.

4 participants