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

Unintuitive error handling #151

Closed
humanchimp opened this Issue Mar 18, 2014 · 3 comments

Comments

Projects
None yet
3 participants
@humanchimp
Contributor

humanchimp commented Mar 18, 2014

Bluebird is a really nice project. I especially like the error handling semantics it adopts by default.

However, when using .nodeify I have encountered some troubling behavior. I brought this up yesterday on IRC, and @spion thought it could be a bug. So I am raising the issue here for further discussion

var log = console.log.bind(console)

function doStuff(callback) {
  return new Promise(function (resolve, reject) {
    reject(new Error("example error"));
  })
  .nodeify(callback);
}

function doStuff2(callback) {
  return doStuff(callback).finally(log);  // or ".then"
}

doStuff(log);

doStuff2(log);  // possibly unhandled exception

When I call doStuff(log), the log becomes my error handler, as it is passed into .nodeify. Bluebird considers this error to be handled in this case.

doStuff2(log) internally calls doStuff(log) and so I would expect that the error would be satisfied by the log callback, but upon attaching finally (or then) handlers, it seems to somehow revive the error from doStuff, and logs a "possibly unhandled exception" stack trace to the console.

I'm not sure if it's a bug, but I would like to understand why this occurs, if it is determined to be the expected behavior, so that I can avoid writing code that might cause this behavior.

@petkaantonov

This comment has been minimized.

Show comment
Hide comment
@petkaantonov

petkaantonov Mar 18, 2014

Owner

There is unfortunately a hidden assumption in .nodeify - that you won't use both the promise and callback interface at the same time. This could be more explicit by returning undefined when nodeify is used with a callback so that doStuff2 function simply wouldn't work at all.

At the time I didn't bother to make nodeify return undefined as I thought that nobody will try to use callback and promise at the same time as it simply doesn't make any sense.

Owner

petkaantonov commented Mar 18, 2014

There is unfortunately a hidden assumption in .nodeify - that you won't use both the promise and callback interface at the same time. This could be more explicit by returning undefined when nodeify is used with a callback so that doStuff2 function simply wouldn't work at all.

At the time I didn't bother to make nodeify return undefined as I thought that nobody will try to use callback and promise at the same time as it simply doesn't make any sense.

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr Mar 18, 2014

Collaborator

+1 for returning undefined

Collaborator

benjamingr commented Mar 18, 2014

+1 for returning undefined

@humanchimp

This comment has been minimized.

Show comment
Hide comment
@humanchimp

humanchimp Mar 18, 2014

Contributor

OK, I agree that .nodeify should return undefined in the presence of a callback. Thanks for responding.

Contributor

humanchimp commented Mar 18, 2014

OK, I agree that .nodeify should return undefined in the presence of a callback. Thanks for responding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment