4

It is a convention in node to pass an error parameter to asynchronous operations:

async.someMagicalDust(function(callback) {
     // some asynchronous task

     // […]
     callback();

}, function(err) {
     // final callback

    if(err) throw err;    
    // […]

});

Maybe I'm too naive, but I've never been a big fan of the if(variable) notation — probably inherited from C, for reasons that have already been discussed many times in the past.

On the other hand, I have sometimes encountered a null parameter and this error check:

if(typeof err !== 'undefined' && err !== null)

is a bit too verbose.

Another solution would be

if(err != null)

but I think the non-strict check can be tricky, even though I consider it normal when comparing with null.

What's the best way to check error parameters in node?

Community
  • 1
  • 1
Pierre Arlaud
  • 4,040
  • 3
  • 28
  • 42

3 Answers3

2

Use if(err).

It is designed to be used in this fashion. Node-style callbacks are supposed to set the error to a non-falsy value only in case of actual error. You won't find any sane example of setting err to '' or 0 to signify error.

Just like YlinaGreed has noted, if module conventions cnahge from null to undefined to 0 to maybe even NaN, you are still safe. I've never been hit by this having only used if(err).

On the other hand, you may want to use coffescript, which would translate for you

unless err? then...

into

if (typeof err === "undefined" || err === null) {

mimicking the most common pattern.

Some links to corroborate the if(err) approach:

The convention seems to be passing an error object as the first argument and null for no error so even if you pass an empty object, it's still an error.

If you use the popular express framework, you should have used the next callback to return from middleware, which follows the errback convention.

I believe that most people prefer more concise next() than next(null), which means that the first argument will evaluate to undefined rather than null, and this is certainly perfectly normal usage.

punund
  • 4,321
  • 3
  • 34
  • 45
  • Would you however recommend the second example over err == null (non-strict equality, bringing the same result) – Pierre Arlaud Jan 28 '15 at 16:31
  • I would recommend it only if it's auto-generated (e.g. by coffeescript). Otherwise, the code legibility outweighs other considerations. – punund Jan 28 '15 at 22:22
  • I think this is the right approach. Can you find official source/reference for "it is designed to be used in this fashion"? – Pierre Arlaud Jan 29 '15 at 08:54
0

To me, the best way to handle error is the "if (err == null)" This is the only case I am using the non strict operator, for these reasons:

  • The only over solution which always works is very verbose, as you said before
  • You could also check only on "null" or "undefined", but I did this once, and a few month later, I just updated my dependencies and... The convention changed, and the module was sending null instead of undefined.

This is mostly a matters of "convention", I have mine, and you certairnly have your's too... Just be careful to chose one of the two "good" ways.

YlinaGreed
  • 239
  • 1
  • 2
0

Node's primary callback convention is to pass a function with err as the first parameter. In my experience its always been safe to check if error is truthy - in practice if your error comes out null when there IS an error then the problem lies more with the implementation. I would always expect if err is null that no error occured. It may be confusing due to the use of separate functions for errors and successes, something that is more in the style of JQuery.Ajax and promises. I tend to find double callbacks to be a bit too wordy to call.

Given your example it seems you're using the async library which is excellent. If I am looking to perform a parallel option this is how I set it up:

function doAThing(callback) {
   var err;

   // do stuff here, maybe fill the err var

   callback(err);
}

function doAsyncThings(callback) {
  var tasks = [function(done) { // stuff to do in async
     doAThing(function(err) {
       done(err);
     });
  }];

  async.parallel(tasks, function(err) { // single callback function
       callback(err); // I send the error back up
  });
}

Note that instead of throwing the error I bubbled it back up the request chain. There are few instances which I'd want to actually throw an error since you're basically saying "crash the whole app".

I find it to be simpler and reduces the amount of parameters you have to use to call your functions. When you use this convention throughout you you can simplify by simply passing the callback as a parameter instead of creating a new anonymous function, like so:

function doAThing(callback) {
   var err;

   // do stuff here, maybe fill the err var

   callback(err);
}

function doAsyncThings(callback) {
  var tasks = [function(done) { // stuff to do in async
     doAThing(done);
  }];

  async.parallel(tasks, callback); // the error is sent back to the original function
}

I find that generally you want to handle those errors in the functions they are called in. So in this case the caller of doAsyncThings can check if there is an error and handle it appropriate to its own scope (and perhaps provide better information to the user if it is say an API).

Kelz
  • 494
  • 4
  • 9