Assuming there was no fatal error, should the first argument to an asynchronous callback be `undefined` or `null`?

best-practices-for-passing-the-first-argument-to-an-asynchronous-callback.md

Best practices for passing the first argument to an asynchronous callback

When implementing an asynchronous function, you have to think about callbacks. In Node.js, that callback is conventionally written to have two arguments.

tldr: skip ahead

But let's dissect that a bit further. Here are the rules our team guarantees for the callbacks of asynchronous functions we write:

1. If the first argument is truthy, then you can assume there was a fatal error (something you'd normally use throw to handle, if this was a synchronous function).
2. If the first argument is truthy, then you shouldn't trust any of the other arguments I send back.
3. If the first argument is truthy, then it will always be an Error instance. So you can do .stack on it in your code (e.g. new Error('Oh hey this other error happened. Details: '+err.stack);)
4. Finally, if the first argument is not truthy, then you can assume there was no fatal error and the function worked as intended.

This way, in our userland code, we know to write stuff like this:

doSomething(function (err) {
  if (err) {
    // ...
    // I can safely do err.stack because if truthy, it should always be an Error instance.
    // ..
    return;
  }//-•

  // I should always treat this as a disparate branch of control flow.
  // If `err` is truthy, I can never trust that the supposed thing actually happened,
  // and I certainly can't trust whatever else I received in other args to the callback.
  // ...
});

The problem

But not everyone comes to Sails/Node.js/JavaScript on the same page about the conventions I mentioned above. And even as an implementor, it leaves some room for imagination. Maybe too much.

Imagine if, on the implementation side, I happened to do:

// In the implementation:
return cb(null, 'my cool result');

And then, someone passed in a userland callback that looks something like this:

// In userland:
doSomething(function (err) {
  if (typeof err === 'object') {
    console.log(err.stack);
    // ...
  }
  else if (err) {
    console.log(err);
    // ...
  }
  else {
    // ...
  }

}

^ see the problem?

In JavaScript:

typeof null === 'object'

So even though the code in both places looks correct, since the userland callback is not doing a truthiness check, and because, by some dark curse, typeof null === 'object' in JavaScript, we now have a situation where userland code is behaving as if there was an error, when there actually wasn't!

-_-

Best practice

Luckily, this can be mitigated by taking an easy step as an implementor: when you invoke the provided callback function, and want to indicate that no error occurred, use undefined for the first argument.

Not only is this more consistent (e.g. cb() is semantically more similar to cb(undefined) than it is to cb(null)), but it also helps avoid mistakes in userland code.

Conclusion

In your implementation, if you always do one of the following, you'll be good to go, and you'll provide a better experience for the people using your thing:

1

// Some kind of fatal error occurred-- something where I'd normally throw,
// except that I'm an asynchronous function.  So instead, I do this:
return cb(new Error('Something bad happened!'));

2

// Everything is AOK, and there's no other data to send back:
return cb();

3

// Everything is AOK, and there's this result I want to send back:
return cb(undefined, yourResult);