Default Unhandled Rejection Detection Behavior

[benjamingr]

With #758 (hopefully) soon landing to address #256 we will soon have unhandled promise rejections reported to the process via a process.on("unhandledRejection" event.

One issue that was deliberately deferred from that issue and PR is what the default behaviour of an unhandled promise rejection detected be.

Just to be clear - this is about the default behaviour of a rejected promise with no catch handler (or then handler with a function second argument) attached.

The different approaches

• Logging the rejection to the console.
• Throwing an error for process.on("uncaughtException" to deal with.
• Causing the process to exit.
• Do nothing by default.

There are several good arguments for each approach. I'll write down the arguments I'm aware of and will edit arguments as people make them in comments.

Logging

• No silent failure by default.
• Does not break any existing code - no "surprises". (See comment by @vkurchatkin on this)
• Been a default in several userland libraries like bluebird and users have been very satisfied with it.

Throwing an error

• A promise rejection is the asynchronous version of a thrown exception.
• Does not ignore logical errors or leave the app in an inconsistent state.
• False positives are extremely rare - and code causing them can arguably easily be refactored to not cause them by not attaching error handlers too late.

Process Exit

• Differentiation between uncaught exceptions and promise rejections might be useful.

Do nothing

• Reliable promise rejection detection is non-deciable, it's possible to report an unhandled rejection that actually is handled at a later point if the catch handler is attached at a very late time.
• Does not break any existing workflows for anyone. No backwards compatibility issues for native promise users at all.
• Does not make any assumptions on how people use native promises.

---

What do people think the default behaviour should be?

[vkurchatkin]

Does not break any existing code - no "surprises".

Actually, logging to stdout (or stderr, I guess) can cause crash: nodejs/node-v0.x-archive#6612

[golyshevd]

Logging

IMHO emitting some event it is already enough logging. Direct writing to console is dirty and inconvenient to use with custom loggers, it may violate a format of application logs.
Sometimes i think about some built-in flexible logging facility (log levels, handlers, formatters, etc.) like logging in python

Throwing an error

Much more transparent way. An exception which was caught with promise but was not handled should be thrown. But it is painful breaking behaviour. Developers should promise.then(fn).then(null, noop) to explicitly ignore exceptions

Causing the process to exit.

Does thrown error will not be caught by uncaughtException event handler? Is not is the same as previous approach?
Do I understand correctly that the difference is that in one case the exception will be sent directly in the uncaughtException event handler, and in the second case the exception will be rethrown, but will still be caught in the uncaughtException handler if specified?

Do nothing

As variant to give developers choose how to handle unhandled promise rejections

[Havvy]

Of those options, I'd argue for doing nothing. The end user can figure out what they'd like as a default, and add the handling there. If you want to be really strict about catching them, you can do process.on("unhandledRejection", function (err) { throw err; }). Personally, using Bluebird, I've only seen it once, and it was a false positive.

[benjamingr]

I'd just like to emphasise that this discussion is about defaults - all options will remain open to developers and they'll still be able to decide all this.

@Havvy Thanks for your thoughts - I find those statistics very different from what bluebird users have been reporting. False positives are very rare but true positives are extremely common when people have typos or pass invalid JSON around etc.

@golyshevd - just to be clear - are you for throwing or for no default?

[petkaantonov]

The problem with doing nothing is that the feature might as well not exist as it will be undiscoverable to most developers.

IMHO emitting some event it is already enough logging. Direct writing to console is dirty and inconvenient to use with custom loggers, it may violate a format of application logs.

This is exactly what the default uncaught exception handler does minus exiting the process....

[tellnes]

I would say emitting an event eg unhandledRejection and throw if there is no event handler. Similar to how uncaughtException works. Or maybe we could emit it as an uncaughtException if there is no handler for unhandledRejection.

At least, don't choose to do nothing. Errors which are silenced is a nightmare to debug.

[golyshevd]

@benjamingr

@golyshevd - just to be clear - are you for throwing or for no default?

I am for no default as emitting event as @petkaantonov already implemented. It might be simply changed to throw like

process.on('unhandledRejection', function (err) {
    throw err;
});

process.on('uncaughtException', function (err) {
   log(err);
});

[aheckmann]

-1 for throwing bc not everyone agrees. we cannot break anyone's programs and this potentially would.

+1 for logging by default. as EventEmitter by default logs a message when the number of max listeners has been exceeded but supports overriding the default behavior through setMaxListeners etc, and as uncaughtException crashes the program by default yet supports overriding that behavior by setting a listener, we could log by default here and support overriding that behavior when a user sets an unhandledRejection listener.

+1 for documenting this at the top of the Promises page in iojs.com

—
Sent from Mailbox

On Fri, Feb 13, 2015 at 6:48 AM, Dmitry notifications@github.com wrote:

@benjamingr

@golyshevd - just to be clear - are you for throwing or for no default?
I am for no default as emitting event as @petkaantonov already implemented. It might be simply changed to throw like

process.on('unhandledRejection', function (err) {
    throw err;
});
process.on('uncaughtException', function (err) {
   log(err);
});

---

Reply to this email directly or view it on GitHub:
#830 (comment)

[meandmycode]

+1 for throwing personally, but perhaps I'm not understanding the current support problem. If (native) promises are new- how are people using them to any degree yet? surely this has zero impact on promise libraries. If you decide to upgrade to native promises then you deal with the behavior change. Swallowing errors by default? seems like an unbelievable discussion for such a low level platform.

[domenic]

To be clear to those thinking this is about swallowing errors "by default": not handling a promise error is the same as forgetting to check the err parameter passed to your callback, which of course we see happening all the time in the real world as well as in example code. The fact is that promises actually have a better story than callback-errors here, because since promises have two callbacks (one for fulfilled, one for rejected), we can detect if you didn't handle the error, and emit an event.

So promises swallow errors "by default" to the same extent callbacks do. This is about how we can be even better.

[domenic]

... and that actually leads to a good way of addressing the question posed in the OP.

Let's say you had a magic way of detecting if someone forgot to handle the err parameter to their callback, and instead just proceeded as normal---either on purpose or because of forgetfulness. Would you like io.js to use this magic power to log to the console when that happens? Crash your program? Do nothing?

The tradeoffs are very similar, so I hope your answer to that question can help you contextualize the question posed in the OP.

[benjamingr]

Let's say you had a magic way of detecting if someone forgot to handle the err parameter to their callback

Forgot to handle the err parameter and an actual scenario occurred where the err parameter was not null (that is, an error happened).

I like the comparison by the way.

[petkaantonov]

not handling a promise error is the same as forgetting to check the err parameter passed to your callback

It's not the same at all, callback errors are always/generally operational while promise rejections can be either programmer errors or operational errors.