NodeJS Promise error handling gotcha

We have a NodeJS application that makes HTTP calls to another (.NET) application. Sometimes, when the .NET application returns a HTTP 500 code, our NodeJS application would crash.

Needless to say, this is not good. You don't want one application to crash, because another application encountered an exception. Of course, you don't want any application to crash, but it's inevitable. The best thing to do, is to make sure exceptions don't propagate and take down other applications.

The cause, in our case, was the way we were using Promises in our NodeJS code.

Take this piece of code for example:

let Q = require('q');  
let http = require('http');

getSomething()  
    .then((response) => {console.log(response)});

setTimeout(() => {  
    console.log("Process didn't crash!");
}, 5000);

function getSomething() {  
    var deferred = Q.defer();

    getError()
        .then((response) => { 
            deferred.resolve(response);
         })
         .done();

    return deferred.promise;
}

function getError() {  
    var deferred = Q.defer();

    deferred.reject('Something wrong happened.');

    return deferred.promise;
}

We're calling the getSomething function, that make a call to the getError function. The getError function fails and so the Promise is rejected.

When you run this, you won't see the Process didn't crash! message. This is because our exception wasn't handled, and our Node process was terminated. Not what you want when you're running a critical API.

Of course! We're not catching the error in our top-most function. So we change the call to:

getSomething()  
    .then((response) => {console.log(response)})
    .catch((err) => {console.log(err)});

However, our process is still terminated. Now, look closer and notice how, in the getSomething function, we're calling done(). According to the docs, this is

[m]uch like then, but with different behavior around unhandled rejection. If there is an unhandled rejection, [...] the resulting rejection reason is thrown as an exception in a future turn of the event loop.

So let's remove the done() call and run it again:

Now our process didn't crash, but our catch still isn't executed! This is because in getSomething, we're returning a new Promise without an error handler. This fixes it:

function getSomething() {  
    var deferred = Q.defer();

    getError()
        .then((response) => { 
            deferred.resolve(response);
         })
         .catch((err) => {
             deferred.reject(err);
         });

    return deferred.promise;
}

So should we then put error handlers everywhere? Heavens no! Instead of returning new Promises in our methods, we can better just add to the Promise chain:

function getSomething() {  
    return getError()
        .then((response) => { 
            // possibly do extra stuff here
            // if not necessary, omit the then() and just return getError()
            return response;
         });
}

To conclude, you should rarely have to create new Promises. Only in the bottom-most layers of your application will you need to do this. And often, it will already happen in the third-party libraries you use. Then just call those methods and add a chain of then() calls.