Skip to content

Instantly share code, notes, and snippets.

@benjamingr
Last active October 6, 2023 08:31
Show Gist options
  • Save benjamingr/0237932cee84712951a2 to your computer and use it in GitHub Desktop.
Save benjamingr/0237932cee84712951a2 to your computer and use it in GitHub Desktop.
Promise unhandled rejection tracking global handler hook

Possibly Unhandled Rejection NodeJS Promise Hook

###Unhandled Rejection Tracking

Several promise libraries such as bluebird and when as well as some native promise implementations offer potentially unhandled rejection tracking. This means that the following:

Promise.reject(new Error("err")); // never attach a `catch`

Does not get silently suppressed being an error but instead gets logged to the console or otherwise treated by the promise library. A common request is a way to handle rejection application wide. NodeJS code often includes several copies of the same promise library or different promise libraries and it is currently difficult to install a global hook on potentially unhandled rejections. For example code using Bluebird for concurrency, akamai for their API, bookshelf as an ORM and request-promise for web requests uses 4 promise libraries already - 3 Bluebirds and a WhenJS. If someone wants to log errors for all tracked unhandled rejections they'd have to add at least four handlers - if they miss any their code won't do what they expect.

This document attempts to offer a standard way for promise libraries and potentially native promises in Node to communicate they have detected an unhandled rejection.

##The hook

In order to handle rejections the following new events will be fired on process:

###Event 'unhandledRejection':

Emitted whenever a possibly unhandled rejection is detected. This event is emitted with the following arguments:

  • reason the rejection reason of the promise susprected in having an unhandled rejection
  • p the promise suspected in having an unhandled rejection itself.
process.on('unhandledRejection', function(reason, p){
    console.log("Possibly Unhandled Rejection at: Promise ", p, " reason: ", reason);
    // application specific logging here
});

###Event 'rejectionHandled':

Emitted whenever a possibly unhandled rejection was mistakenly identified and was eventually handled. For example if the promise library made a mistake considering p in the following case unhandled:

let p = Promise.reject(new Error("err"));

setTimeout(() => p.catch(function(){}), 86400);

This event is emitted with the following arguments:

  • p the promise who was believed to be an unhandled rejection.

This event is useful in order to recover from detection mistakes. While in my experience this rarely happens in practice this additional hook helps in such cases.

##Backwards incompatible changes

None that I am aware of. No one is currently firing events by these names in NodeJS code in GitHub.

##In other languages

Promises as an abstraction are indeed not unique to JavaScript - let's explore what other languages do.

So overall it seems like other programming environments support a unhandled rejection hander. Note that JavaScript is pretty special in that it is the only case I know of where a lot of different implementations of this abstract concept exist.

##Statistics

These are usage statistics about events current libraries already fire. These usage examples were collected from public GitHub projects using bluebird promises offering (non-global) hooks.

First a summary of bluebird's onPossiblyUnhandledRejection from GitHub. Large (>1000 stars) libraries typically don't change onPossiblyUnhandledRejection, the largest ORMS: Sequelize, Waterline and Bookshelf do not hook on it however users of those libraries sometimes do. It also very common for people to override it in test code.

Here are libraries and projects using onPossiblyUnhandledRejection in their code:

##Future work

Add a similar solution for promise libraries in browsers, possibly the WhatWG proposal.

##Related work:

@benjamingr
Copy link
Author

Relevant issues:

If requested - a sample implementation guide will be added.

Lie also added it.

@briancavalier
Copy link

@paulirish
Copy link

i feel like the proposal here is the right thing to do for now.

As domenic has mentioned there is a parallel effort happening inside the browser and this proposal currently conflicts with it.

However I'm not terribly concerned. How we end up dealing with the unhandled rejections in the browser... it'll take a few months until theres an implementation, and it feels wrong to ask the node promise scene to pause on this convention until it's hashed out upstairs. If the APIs end up differing then we'll just have to move to those, but we'll all be doing it all together.

It'd be ideal if the telemetry libraries only had to add listeners for the same thing, but i guess i'm not worried too much because this isn't developer facing. it's mostly a concern for library authors

👍

@appsforartists
Copy link

If this means errors won't be silently swallowed when someone forgets to add a catch call to a Promise chain, I'm all for it.

Requiring a developer to meticulously add .catch(error => console.error(error.stack)) to every Promise chain just to get the behavior that's standard for sync errors is wrong, and it's immensely frustrating to spend hours trying to figure out why something is silently broken, only to realize you forgot to add a catch call to one of your Promises.

Just like thrown Errors, unhandled rejected Promises should log to the console by default.

@nevf
Copy link

nevf commented Apr 21, 2015

Following you reply a few days back I left the following at SO which I assume you haven't seen.

ref: http://stackoverflow.com/questions/29689143/trap-when-js-unhandled-rejections/29689261

the Node.js process.on() example prevents the normal console/prettymonitor output from displaying. Is there a way to retain this. Also the docs show: process.on('unhandledRejection', function(reason, key) not ..(promise,reason). ref: github.com/cujojs/when/blob/master/docs/debug-api.md

@benjamingr
Copy link
Author

@nevf this is implementation defined, I suggest you open an issue on when's repo and ask

@nevf
Copy link

nevf commented Apr 29, 2015

@benjamingr Shall do, thanks.

@MicahZoltu
Copy link

I believe that this proposal discourages usage of promises in the way that I believe they were intended to be used. Fundamentally, promises are different from synchronous code.

When people supporting this sort of global error handling see this code:

new Promise(executor).then(result => {
    doStuff();
}, error => {
    handleException();
});

They think it is equivalent to:

try {
    executor();
    doStuff();
} catch {
    handleException();
}

However, it is actually equivalent to this:

let resultOrError = executor();
switch (typeof resultOrError) {
    case result:
        doStuff();
        break;
    case error:
        handleException;
        break;
}

In the first visualization, one is seeing the error as an exception that stopped them from executing their primary code path. In the second, the promise just provides a way to follow one of two code paths depending on success or failure.

What people desiring the first really want is async/await (ES7).

try {
    await executor();
    doStuff();
} catch {
    handleException();
}

Promises do lay the groundwork for async/await, but they are not intended to be just async await! They are intended to be a generalized solution to a problem that can be expanded on in the future (via async/await) but solves multiple problems at the moment.

My problem with implementors logging on unhandled is that it drives developers away from using promises as intended and towards an incorrect (IMO) pattern of treating them like exception handling blocks.

Personally, in my development it is incorrect behavior to log an error on unhandled rejection. That just means that I don't have any business logic to run in the case of failure. It does not mean I forgot something.

@benjamingr
Copy link
Author

If you don't have any logic to run in the case of an exception - that's a bug and your server should probably crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment