Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

prust
Copy link

@prust prust commented Mar 2, 2013

@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.

@JohnMcLear
Copy link
Collaborator

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

@JohnMcLear
Copy link
Collaborator

Bump @Pita

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants