async-stacktrace icon indicating copy to clipboard operation
async-stacktrace copied to clipboard

Made callback truly optional; made ERR() return the err

Open prust opened this issue 12 years ago • 2 comments

@Pita,

I understand if you're not interested in this pull request, as it is a breaking change for any code that relied on the throwing behavior of ERR(). However, it makes ERR() more flexible by allowing it to do just one thing: augment the stack of the passed error object.

For instance, instead of requiring that it be called like this:

if (ERR(err, callback)) return;

it can also be called like this:

if (err) return callback(ERR(err));

which is arguably clearer (the call to callback is explicit and the decoration of the err object is implicit).

On a more important note, it allows callback to be referenced just once in both error and non-error scenarios, like this:

function getUsers(db, callback) {
  var sql = 'SELECT * FROM users';
  db.query(sql, function(err, data) {
    console.log('Query complete.');
    callback(ERR(err), data);
  });
}

And it allows callback to be called in non-standard ways:

function getUsers(db, callback) {
  var sql = 'SELECT * FROM users';
  db.query(sql, function(err, data) {
    console.log('Query complete.');
    callback.apply(obj, ERR(err), global_data.concat([data]));
  });
}

Ok, that was a contrived example, but the point is that it brings more flexibility by allowing the calling code to take responsibility for calling the callback how and when it sees fit and allowing ERR() to do just one thing: augment the error object's stack.

This pull request also still supports the previous behavior: if a callback is passed as the second argument, it will still be called by ERR() as before. So the only code that will break is code that does not pass a callback and expects ERR() to throw an exception in the case of an error.

If exception-throwing behavior is desired, it can still be done -- and in an (arguably) clearer way -- by calling ERR() like this:

if (err) throw ERR(err);

I updated the relevant tests and documentation in this pull request.

prust avatar Mar 02 '13 08:03 prust

@prust prolly worth bumping me via email about this and we can see what we can get sorted.

JohnMcLear avatar Dec 05 '13 22:12 JohnMcLear

Bump @Pita

JohnMcLear avatar Nov 01 '14 00:11 JohnMcLear