The Dark Side of Promises

Since the release of es6 many new features have found their way into NodeJS, but non had quite the same impact as promises. Promises have been developed for the browser before es6 was even a thing. There were several implementations that have been used like jQuery's deferred object before the standard made them obsolete. Promises were quite useful on the client especially if you had to make a lot of async calls, or if your API was a complete mess and you had to gather your async calls from all over the place. For me the later was usually the case or at least that was when I've found promises the most useful. The ability to pass around any promise and attach as many callbacks to it as well as chaining them as many times as you wanted made promises highly versatile, but that was for the client. The server is different. On the server you need to make an insane amount of async calls compared to the client. The client normally only needed to call your api server asynchronously, but the server needs to talk to the database, the file system, external APIs like payment and communication and any core service you might have to use. Essentially: a lot of stuff. Any issues we might have on the client due to promises will be amplified on the server due to the higher rate of usage and increased chance to make mistakes.

If we look at the code we use to make promises at first, they don't seem very different from normal functions, but there is one key feature that makes them unique. Promises catches all exceptions that are raised inside them synchronously. This, while very useful in most cases, can cause some issues if you are not prepared to handle them. When an exception is thrown the promise gets rejected and will call its rejected callback, if there's any. But what happens, if we don't handle the rejected state of the promise? It depends on the NodeJS version but generally a warning will be printed out and the function that raised the exception will exit. Rejecting promises via throwing exceptions is something that were often used in the old browser days of promise libraries and is considered normal, but is it actually a good thing. It is good or at least okay if you actually want to reject a promise, however what if you throw an error not because you wanted but because you made a mistake? In that case you need to find the bug and fix it and it is in that specific case when letting a exception crash your server and print out a stack trace would be really useful. So what do we get instead of that? In NodeJS 6 and 7 we will get a UnhandledPromiseRejectionWarning which in most cases will tell you what caused the error, but not where. In node 8 we will also get a short stack trace as well. So upgrading to node 8 could potentially resolve our issues, so as long as you can do that you might think that's all we have to do to solve this issue. Unfortunately, node 8 is not yet used by most companies and makes up less than 10% of the market.

Since node 7 a promise rejection warning will also give you another warning:

"DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code."

Note that this warning doesn't say that it will raise an exception, but that it will crash your server no matter what. That's quite harsh, don't you think? This change would definitely break some code if it were implemented today. Interest in UnhandledPromiseRejectionWarning has risen in conjunction of the popularity and use of promises. We can even measure how much using google trends.

The people who had searched for this particular warning have increased significantly since native promises and this warning were introduced to node. During 2017 the number of searches doubled which also probably means that the number of people using promises in NodeJS has also doubled. Perhaps this is the reason the node team wants to completely purge the warning from its stack.

It is understandable that in case a promise rejection is not handled it is better to crash the server than just issuing a warning. Imagine what would happen to an API route if a rejection was not handled. In that cases the response would not be sent to the client, since the function would exit before it reached that point, but it would also not close the socket since the server would not crash, and it would just wait there until it gets timeout after two minutes. If several such requests were made to the server in the span of two minutes we could run out of sockets very quickly which would block our service for good. If on the other hand we crash and restart, we should be able to serve some requests for a little while at least. Clearly neither case is desirable, so we should put a catch rejection handler to the end of every promise chain we create. This would prevent the server from crashing or raising a warning which would also allow us to reply to API requests in some fashion. The problem with the catch method is that it is only a glorified reject callback no different from the ones supplied via the second parameter of the then promise method.

The biggest issue that I have with promises is that all exceptions are caught by the rejection handler regardless of the reason they were raised. It is normal to except that async calls may fail and it is normal to handle that possibility but catching all exceptions will also catch the errors in your code as well. When normally the system would crash and give you a stack trace with promises the code will try to handle the exception and possibly fail that async call silently letting the rest of your code run uninterrupted. It is very difficult to differentiate promise rejection that was thrown by the system and an exception thrown by the code, and even if you could that it would just be over engineering. The only way to handle promises properly is to write a massive number of tests, but the fact that you simply must do that is not a positive feature in of itself. Not everyone does that and not everyone is allowed to, and there's no good reason to make things difficult for them.

Exceptions raised in any Async call cannot be caught by a try catch block so it makes sense to catch them if necessary. The keyword here is "necessary". It is not necessary to catch them during development just as expressJS will not catch them except in production, but even if the later catches them it will at the very least stop the code execution for that particular call, which you cannot do for promises. The proper way to handle exceptions in promises or for any other async calls is (a) to provide them with an exception handler, which if provided will be executed if an exception is thrown and (b) stop the promise chain or the rest of the code from executing. This handler can be propagated down the promise chain and if not set will allow the exception to bubble up and crash the server.

Some people think that throwing inside promises is necessary to invoke the reject callback, but that was never true. Even today you can just return a Promise.reject(someError) to fail any promise where you would normally do a throw. If you asked why throwing errors are used to reject promises not many could answer. I'm not sure if there is an answer to begin with other than that this was the way promises were implemented for the browser many years ago, and ECMA just reimplemented this somewhat broken standard into ES6 and Node took it from there. Was it a good idea to introduce this version of promises to the standard and to migrate it to the server side? The fact that Node is moving away from the standard should give us some doubt. It's not even true that promises are the only way to handle the dreaded callback hell. There are other solutions like the async and RQ libraries for example which include methods like parallel and waterfall that allow coders to execute async calls in a more organized manner. At least on the server side it is quite rare to need more than some combination of the methods these libraries provide. The reason why promises were introduced in the standard might have been simply because they were popular thanks to jQuery. Implementing exception handling would be easier with a traditional async library, but that doesn't mean it cannot be done with promises. Even today you could override the then method on the Promise prototype and the Promise constructor to do that.

Promise.prototype.then = (function () {
  const then = Promise.prototype.then;
  const fixCall = function(promise, next){
    if (!next) {
      return null;
    }
    return function (val) {
      try {
        let newPromise = next.call(promise, val);
        if(newPromise){
          newPromise.error = promise.error;
        }
        return newPromise;
      } catch (exception) {
        setTimeout(function () {
          if (promise.error) {
            promise.error(exception);
          } else {
            throw(exception);
          }
        }, 0);
        return new Promise(()=>{});
      }
    }
  };
  return function (success, fail, error) {
    this.error = this.error || error;
    let promise = then.call(this, fixCall(this, success), fixCall(this, fail));
    promise.error = this.error;
    return promise;
  }
}());
function createPromise(init, error){
  let promise = new Promise(init);
  promise.error = error;
  return promise;
}  

I mentioned before that async calls cannot be caught by a try catch block and that is true even inside a promise, so it is possible to break out from a promise using a setTimeout or a setImmediate call. So, if we catch an exception we just do that unless an exception handler was provided in which case we call that instead. In both cases we want to stop the rest of the promise chain from executing and we can do that by simply returning an empty promise that never gets resolved. Obviously, this code is only here to demonstrate that it can be done, and even though now you can handle exceptions properly you haven't lost any of the original functionality.

One major problem of promises is that you might be using them without realizing it. There are some popular libraries out there that use promises behind the scenes and at the same time allow you to specify traditional callbacks but will execute them inside of the promises they use. What this means is that any exception will be caught without your knowledge or ability to add a reject handler for them, so they will raise the UnhandledPromiseRejectionWarning for now. You will certainly scratch you head if you see this warning without having a single promise in your code, the same way I did some time ago. Now normally you would get a relatively useful error message in the warning, but if you are executing the bad code inside a method of a async library, then it will probably fail in way most of us can't comprehend. Once you enter a promise all of your callbacks will be executed in the context of that promise and unless you break out of that using something like setTimeout it will take over all of your code without you realizing it. I will put here an example which uses an older version of the Monk MongoDB module. This bug has been fixed but you can never know if another library will do something similar. So, knowing that monk uses promises, what do you believe will happen if I execute this code on an empty database?

async.parallel({
  value: cb => collection.find({}, cb)
}, function (err, result) {
  console.log(result.test.test); //this line throws an exception because result is an empty object
});

The answer is:

(node:29332) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Callback was already called.

Unless you are using Node 8, in which case you will get:

(node:46955) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:46955) UnhandledPromiseRejectionWarning: Error: Callback was already called.
    at /node_modules/async/dist/async.js:955:32
    at /node_modules/async/dist/async.js:3871:13
    at /node_modules/monk-middleware-handle-callback/index.js:13:7
    at <anonymous>
at process._tickCallback (internal/process/next_tick.js:188:7)

Good luck finding the cause of that 😊.

Sources:

1. https://semaphoreci.com/blog/2017/11/22/nodejs-versions-used-in-commercial-projects-in-2017.html
2. https://trends.google.com/trends/explore?date=2016-03-30%202018-03-30&q=UnhandledPromiseRejectionWarning
3. https://github.com/nekdolan/promise-tests

About Daniel Boros

I am a full stack remote web developer plus Javascript and NodeJS enthusiast. I have experience in building hybrid apps, nodeJS api servers for the backend and VueJS for web based applications. I have worked around the globe for several companies small and large, at location and remotely as well. I have picked up experience in the marketing and online betting industries in general.

github.comPosts