Node.js error handling | Hacker News Reader

Node.js error handling

snmaynard.com

78 pointsbeastmcbeast23 comments

Unfortunately error handling in Node.js is a total mess.

As with most new technologies there are few standards and whatever standards there are are subject to change. It was only a short time ago that many Node.js libs were being produced with promises.

The most common style I see today is this callback style:

  doAsyncCall(function(err, result){
    if(err){ // handle err }
  });

Unfortunately this is prone to error. When writing any sort of asynchronous function - that is, 'doAsyncCall' itself, you need to write it in the following way:

  function doAsyncCall(cb){
    // .. do some async stuff
    if(// error has been thrown){
      callback(err);
    } else {
      callback(null /* this is where confusion happens */, result);
    }
  }

When writing such code you need to be very consistent and very aware of what error handling style is being used. It can be jarring for some to intentionally pass null as the first argument.

I am personally a fan of the `if(result typeof Error)` style but it is not common. Many frequently used libraries, like Mongoose, use the (err, result) style.

Because any async call can theoretically fail, and because you need to catch every potential error, it is common for starting programmers (and even experienced ones) to miss an edge case or two. I advocate using upstart or a similar scheme to make sure your Node server stays alive, even if you happen to have missed an error somewhere.

One of the things I love about node.js is that it forces you to come up with a style and stick with it for things like this. At http://ratchet.io our API servers are written in node.js and we chose the foo(err, callback) method.

I personally love this style even though it makes you write more boiler-plate code, it forces you to write exception-safe code from the start. Also, it forces a structure on all of your code that you can instantly recognize as missing if someone forgets to check for err in the callback.

If you can stick with the style, it's fairly difficult to write code that doesn't deal with errors gracefully.

I agree. One of the advantages of the (err, result) style is that it puts error handling directly in your face. The `result typeof Error` style does not.

In any case all I care about is that we choose one, and (err, result) is gaining traction, which is fine. While many are excited about domains, I am not. From my perspective, in practice, it will just add another incompatible style onto the pile.

Yeah, domains seem like a desperate solution. Will be interesting to see what eventually becomes the standard.

How do you deal with code where the author forgot to handle all of the failure cases and invoke the callback?

To be honest... since you can't try/catch, you can't do much.

You can catch the exception with something like this:

  process.on('uncaughtException', function (err) {
    console.err("Uncaught exception: " + err);
    console.err("Uncaught exception stack: " + err.stack);
  });

I use the built-in clustering module (implementation is very simple) so a simple worker crash will result in the error being logged & the worker being restarted.

That's about the best you can do, unfortunately. You could always monkey patch the lib if you can figure out where it's crashing.

I recommend Longjohn(https://github.com/mattinsler/longjohn) for these sorts of situations; the extra stack trace lines become invaluable.

Fix the code.

Is this really the most common style? It seems to me that this is more common:

  doAsyncCall(function onSuccess(result) {
    // do things
  }, function onError(err) {
    // No need to check, it's really an error.
  });

This is much cleaner and lets you reuse error handling functions.

What a great pattern! I have to disagree though, (err, data) seems much more popular. Until now I'd never seen this (fantastic) style.

I don't think that this style is fantastic. This means that the pair of success/error callbacks is not reified, so there isn't a single object to pass around or manipulate. The result is that composition is harder. In order to create higher order asynchronous operations, like those in Async.js [1], you need to either pass two arguments or use an array or object to pair the error handlers together. This is particularly important if you want to return a pair of handlers from a function.

In the two-callback model, the most common way to reify the handler pair is with a {success:...,error:...} object. This is much better than function(x,y,z,success,error), but still has the problem that you now need to override two handlers if you want to wrap some completion logic, regardless of success/failure. For example, if you want to close a file handle, win or lose, you need to provide two new functions.

Composition greatly benefits from a singular object. The Node.js approach is the (err,data) callback. It's an extremely simple and successful approach. The other approach are promises/futures, which internally can be implemented with one or more callbacks, but provide a nice clean interface for composition. You could get that with success/error objects, but you're exposing the internals rather than an interface, like you should be doing. A good promise interface would let you choose an implementation without tying you to any particular callback model.

[1] https://github.com/caolan/async

My experience of the f(onerror, onsuccess) pattern is that it does not scale well when multiple functions are nested, as the error-handling code gets pushed further and further down the page from the actual error site it handles.

with (er, result) -> you may find yourself polling both er and result.status

in practice I find I this means relatively ugly or unhelpful error messages through the first parameter are successively refined by conditionally setting helpful status and message attributes on the second

this style is kinda consistent with ajax calls on the client that may either fail, or return data containing an error.

That way is longer without reason, you must first define in the function scope both variables error and result; the latter only receives assignation when no error has been generated.

    function doAsyncCall(callback){
        var error, result;
        // .. do some async stuff
        // assign error if there was one or result when everything went fine
        if( !error ){ result = data; }
        callback(error, result);
    }

I actually just wrote a Node + Express + Backbone error handling system today.

I've been staying sane through the use of the 'Q' promise package (https://github.com/kriskowal/q). For example, in the below code chunk any errors thrown in makeEmailConfirmation or sendEmailConfirmation will land in the .fail() catch block.

I believe using a promise system is the only way to avoid callback hell.

  var deferred = Q.defer();

  ...  
  
  Storage.CustomLinks.makeEmailConfirmationLink(user)
  .then(function(link) {
    return Email.sendEmailConfirmation(user, link);
  })
  .then(function(result) {
    deferred.resolve(result);
  })
  .fail(function(err) {
    deferred.reject(new ServerError(err));
  })
  .end();

  ...

  return deferred.promise;

I've recently written an application extensively using Q, and was very pleased at how clean my code ended up being. Particularly in comparison to how node.js code tends to end up looking once your callback pyramids start to grow.

+1 for promises based exception handling. I have used Q for a number of applications. Q really simplifies error handling and by chaining error propagation forward to asynchronous result consumers.

You can also use fibers and make the code appear synchronous, which gives you the ability to use exceptions for error handling.

Will these examples protect you from thrown exceptions? Null reference exception? Oops, your callback isn't getting called -- and you're probably leaking resources or leaving things in an inconsistent state.

So the recommendation is to restart the process - but let's not forget that hundreds+ of other requests could currently be in the pipeline - are we just going to abort those too?

Ok, we'll clearly the solution is to wrap all our callbacks with try/catch! Shoot me now...

For large, sprawling applications used to support a business, something like https://github.com/scriby/asyncblock can resolve most of the headaches with async code, including error handling. It's worked out well for my team at work, which is building a "very large" node application.

Disclaimer: Asyncblock is my pet project.

Finally a good explanation of error handling in node.js. Cheers!

Seems like promises (e.g. Q) use a other sort of error handling similar to domains, but win a cleaner syntax the addition of the promises functionality.

Wow, I should proof read on my iphone.

There seems to be an error in the final code snippet. Instead of d.intercept it should be mainDomain.intercept. Here's the original example: http://nodejs.org/api/domain.html#domain_example_1

Apologies for that, I've updated the example. Thanks for pointing it out.

Reply on news.ycombinator.com