cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Expose unhandled rejections through core api #394

Closed rymohr closed 9 years ago

rymohr commented 9 years ago

Right now it's possible to add a custom handler but it feels wrong since you have to reach through the promise to do it and then access the grab the error value from r.value:

when.Promise.onPotentiallyUnhandledRejection = function(r) {
  error = r.value
  ...
}

How about adding something like this instead?

// where error is the result of r.value
when.catch = function(error) {...}
briancavalier commented 9 years ago

Hey @rymohr, those APIs (onUnhandledRejection and friends) are intended specifically for debuggers and tools, like the when/monitor/console long-stack trace tool, or @AriaMinaei's pretty-monitor. They're intentionally hidden from the view of application developers because they should really only be called once, at init time, by a debugger or other tooling.

The object that is passed also intentionally carries more information that can be useful to a debugger. It carries a unique rejection id, as well as any additional context data that a debugger might have added (via the Promise.createContext debugger hook). For example, when/monitor/console uses it to attach stack frame information that it later stitches together in order to report long, async stack traces.

Application devs should handle promise errors at the promise instance level using the catch, finally, else, etc. methods.

rymohr commented 9 years ago

Thanks for the detailed write up Brian. I agree that errors should be handled at the instance level.

In my case I just wanted a way to make sure that these errors got reported to bugsnag. With the way we use promises, any error that escapes the instance stack is an application error that needs to be fixed.

As long as the api is fixed it's no biggie. I just didn't see it documented anywhere and was scared a future update might sidestep my error reporter.

briancavalier commented 9 years ago

In my case I just wanted a way to make sure that these errors got reported to bugsnag

That's really cool! I'd love to hear how it turns out.

As long as the api is fixed it's no biggie. I just didn't see it documented anywhere and was scared a future update might sidestep my error reporter.

It's been pretty stable in the 3.x line, and the plan is to treat it like any other API with semver. If there's a breaking change, it'll require a version bump. So, I think as long as you're keeping your dependency versions relatively tight, you should be ok.

As for documenting it, yeah, I need to do that :) How about this: let's close this issue, and I'll open another to document the unhandled rejection reporting API.

briancavalier commented 9 years ago

See #395

briancavalier commented 9 years ago

Closing this, and we can track progress on documenting the debug APIs in #395

rymohr commented 9 years ago

Thanks Brian, sounds good.