$q reports errors in promise callbacks even when they are caught

[domenic]

Plunker: http://plnkr.co/edit/5vMj4AJibWcnTKYR5z73?p=preview

I have caught the error and used it to display useful information to my user in the UI. However, Angular still logs a big ol' stack trace to the console.

This isn't a spec compliance issue (since the error isn't thrown, just logged), but it's a developer ergonomics issue. I don't understand how using $q on any reasonably large promise project would not just result in a console filled with spurious errors.

As always, the correct time to report a rejection as unhandled is debatable. Bluebird does it at the end of turn, whereas Q doesn't do it at all, relying on .done() instead. But this isn't even about unhandled rejections---it's about all rejections!

The Bluebird strategy seems like a pure win: it would remove this spurious reporting, but keep it for any cases that could potentially be real.

[caitp]

This doesn't seem unreasonable, I can put together a patch for this tomorrow

[pkozlowski-opensource]

Agreed that the systematic logging of exceptions thrown from callbacks is a bit "noisy". This was already discussed here: #7187 (comment)

[caitp]

So, I don't think I fully understand Bluebird's way of doing this --- they manage to determine if a promise is handled synchronously and asynchronously (example http://jsfiddle.net/8ga3U/).

I think this is fixable by adding some extra state to $q Promise objects, so that tracking whether a rejection is handled or not can happen when then() is called, rather than during resolution (which my current prototype does)

My current prototype "solves" this by adding an extra task queue to be consumed after the nextTick() queue, so that handled rejections can turn off their 'logging'. Only thrown exceptions behave this way, so normal rejections are just processed regularly (talking about my code here, although I believe the code in the tree behaves the same way). Thrown errors are wrapped, and error handlers can mark errors as handled, so that a scheduled error logger doesn't need to do anything. This works, but obviously only hides messages when rejections are handled in the same turn that they're resolved in, which is basically never

---

So there's a current implementation (by which I mean, my work-in-progress, not code in the tree) which doesn't work very well in a lot of cases, and a proposed solution (which I think should be good enough, but I'm not sure without trying it). There were plans a while ago to change the Promise implementation to use prototype chains to avoid creating so much garbage, but not much progress has been made there yet. However, finishing that would make it a bit easier to add extra state to the promise objects in a manageable way, I think.

It's a bit sad that it's turning out to be harder than expected to improve the error logging in $q, but I can keep hacking on it.

[domenic]

Only thrown exceptions behave this way, so normal rejections are just processed regularly

This is a bit frustrating; they should be symmetric.

Tagging in @petkaantonov to see if he can help explain Bluebird's technique.

[caitp]

I think I sorted out my understanding of that particular part of it while writing my little summary there -- I guess Bluebird must be deciding of a rejection is handled at the time handlers are registered (eg then), and not during resolution. That would make sense.

So angular doesn't really have a single then-able, but more like 3 different ones, and it seems not very fun to try and make them all behave the same way in this case, which is why I think unifying those into a single class might make sense first

[petkaantonov]

The simple version goes like this:

1. When a promise gets rejected and it doesn't have any rejection handlers attached to it, turn on a flag on the promise and queue a method check to be called after the current queue is completely flushed.

Adding a rejection handler to a promise sets the flag off if it's on. (In cases like promise.all which is optimized to not even attach any handlers on a promise, this is simulated).

In function check

1. If the promise's flag is still on, trigger a possibly unhandled rejection event with the promise and the rejection reason as data.

Then the default handler for the event is to log stack trace to stderr or console.error.

The actual implementation used in bluebird is a bit different because it provides another event for possibly unhandled rejection handled which requires another flag. There is no default handler for that event - but if something like console.unlog existed, it could be used in response to this event.

[prescod]

The issue report says: "This isn't a spec compliance issue (since the error isn't thrown, just logged), but it's a developer ergonomics issue."

But this is incorrect. The error is not just logged. The error is sent to whatever exceptionHandler service is configured, which by default logs. But in some apps it might pop up a modal for the user, or send a message to the server or log to localStorage.