-
Notifications
You must be signed in to change notification settings - Fork 289
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
Catch panics in tasks and convert to rejections #813
Conversation
The changes aren't quite as intimidating when excluding tests. 498 additions, 334 deletions and about half of those are moving lines from one file to another. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a few suggestions for clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
This PR builds on #808 and applies similar logic to tasks that use promises.
It also makes changes to match the latest neon-bindings/rfcs#35:
Deferred::settle_with
is removed. Manually resolving aDeferred
is considered low level. Adding a helper is unnecessary.Channel::settle_with
toDeferred::settle_with
. Now that there is room onDeferred
, it reads more clearly here.