You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I think the implementation should use map directly, instead of get, executing the callback and returning the same value. This keeps the tap side effect in the same chain as normal error handling.
@TheSpyder Thanks for the report and the suggestion. I've added a reproducing test case in 356dd59, and your fix. It's now in npm as reason-promise 1.1.4. Apologies for the pretty big delay.
I updated only Promise.Js.tap, but not Promise.tap, Promise.tapOk, Promise.tapError, or Promise.tapSome. In an idealized type-safe world, these functions would never face promise rejections, and it might actually be good for them to cause rejection reports, if they do. Please let me know if you have any issues with that, and would like them changed, as well.
Replication code:
This prints
regular error handling
but also triggers an unhandled promise rejection.The cause, as far as I can tell, is that
tap
forks the promise and moves the callback out of the error handling chain:https://github.com/aantron/promise/blob/master/src/js/promise.re#L122-L125
I think the implementation should use
map
directly, instead ofget
, executing the callback and returning the same value. This keeps the tap side effect in the same chain as normal error handling.The text was updated successfully, but these errors were encountered: