13

So the general convention for callback functions in Node.js is to "reserve" the first parameter for an error (if one exists). For example:

callSomeBlockingFcn( function callbackWhenDone(err, result) {
  if( err ) ...
});

If you need to return more than one error--say multiple data validation errors, for example--is it considered poor form to pass an array of error objects? Example:

var callSomeBlockingFcn = function(callback) {
  // multiple errors to report back...
  callback( [ err1, err2, ...] );
}

Or is it preferable to avoid arrays and return a single object with a property referencing an array (if necessary)? Example:

var callSomeBlockingFcn = function(callback) {
  // multiple errors to report back...
  callback( { errors: [ err1, err2, ...] } );
}
clint
  • 14,402
  • 12
  • 70
  • 79

2 Answers2

10

3 years later:

Anyone that puts an array in a callback will make me mad.

The correct solution is to return an error as the first argument. If you want to return multiple errors you are probably using errors for non-exceptional cases.

In which case it should go in the "value" slot of the callback, i.e. the second argument. The first argument is for a single, unexpected operational error.

If you have multiple unexpected operational errors (unlikely) you can do something like this MultiError

Original:

I think there's nothing wrong with returning an array of errors.

Although you could return a new custom ValidationError which has a property "messages" which is an array.

a)

function validateX(x, cb) {
  ...
  if (errorMessages) {
    return cb(errorMessages);
  }
}

b)

function ValidationError(msgs) {
  this.messages = msgs;
}

function validateX(x, cb) {
  ...
  if (errorMessages) {
    return cb(new ValidationError(errorMessages));
  }
}
Raynos
  • 166,823
  • 56
  • 351
  • 396
  • 1
    I'm downvoting you for the *"I think there's nothing wrong with returning an array of errors"*, but awarding you a 100 point bounty *(as no one else answered when I bountied it to draw more attention so I have no one else to give the points to)*. Maybe the 98 point net gain will be a small incentive to revisit and re-ponder the issue :-P ...because I do think the canon is that an array of errors is not a valid err parameter in Node. – HostileFork says dont trust SE Jul 29 '14 at 06:44
  • @HostileFork challenge accepted :) Fixed the answer. – Raynos Aug 25 '14 at 01:55
  • @Raynos Vote corrected accordingly. The validation was worth the points. :-) – HostileFork says dont trust SE Aug 25 '14 at 03:06
  • What about an AggregateError object? – Killroy Dec 30 '19 at 09:43
4

Found this question via a search for the same issue. Though I looked around and came to the conclusion that I do not believe that err should be anything but an Error or null.

The best "authoritative" source I've found is Nodejitsu's help topic:

http://docs.nodejitsu.com/articles/errors/what-are-the-error-conventions

In node.js, it is considered standard practice to handle errors in asynchronous functions by returning them as the first argument to the current function's callback. If there is an error, the first parameter is passed an Error object with all the details. Otherwise, the first parameter is null.

But I think that you can sort of make an argument from intuition as to why it should be so. Although there are a lot of if (err) tests in code to decide if something was or wasn't an error, you shouldn't pass 0 or false or undefined or NaN or an empty string. You should be able to test with if (err == null) if you wished.

Passing back something in the err field that is non-null but doesn't match if (err instanceof Error) seems dodgy. So I'd suggest not using arrays or objects. If you did, note also that none of the errors in your array will identify the location where the aggregate error was created. That's the point where the "real error" occurred, because it was the moment of decision that the errors it was given were not something it could handle.

However, this means you'd need a bit more work to get that:

function MultipleError (errs) {
    // http://stackoverflow.com/a/13294728/211160

    if (!(this instanceof MultipleError)) {
        return new MultipleError(errs);
    }

    Error.call(this);
    this.errs = errs;

    // captureStackTrace is V8-only (so Node, Chrome)
    // https://code.google.com/p/v8/wiki/JavaScriptStackTraceApi

    Error.captureStackTrace(this, MultipleError);
};

MultipleError.prototype.__proto__ = Error.prototype;
MultipleError.prototype.name = 'MultipleError';
MultipleError.prototype.toString = function () {
   return 'MultipleError: [\n\t' + this.errs.join(',\n\t') + '\n]';
}

A bit overkill, perhaps. But if you really can't pick an error to represent the aggregation, and think someone might be interested in the set of errors instead of just one, it would seem (?) that's what you'd want to do...allows the caller to examine the errs array if they want.

  • Instead of `MultipleError.prototype.__proto__`, which can [prevent JS optimization](https://developer.mozilla.org/en-US/docs/Web/JavaScript/The_performance_hazards_of__%5B%5BPrototype%5D%5D_mutation), you may want to do `MultipleError.prototype = Object.create(Error.prototype, { constructor: { value: MultipleError }, name: { value: 'MultipleError' }, toString: { value: function () { //... } } });`, with the added bonus of making those properties on `MultipleError` non-configurable and non-enumerable. – snickle Mar 17 '17 at 18:23