Closed robertrossmann closed 10 years ago
I changed it to callback(new Error(....));
I'll publish to NPM tomorrow..
Why does it have to be an Error
instance? The callback functions all expect strings - would it not be easier to simply return the string to indicate the error?
Now we would always have to check whether the err
in the callback is a string or an instance of Error, which makes error handling again more difficult.
AFAIK usually callback(err) takes an error object and not a string..
see the answer for async code - http://stackoverflow.com/questions/7310521/node-js-best-practice-exception-handling
On Sun, Mar 23, 2014 at 12:50 AM, Robert Rossmann notifications@github.comwrote:
Why does it have to be an Error instance? The callback functions all expect strings - would it not be easier to simply return the string to indicate the error?
Now we would always have to check whether the err in the callback is a string or an instance of Error, which makes error handling again more difficult.
Reply to this email directly or view it on GitHubhttps://github.com/bsphere/node-gapitoken/pull/8#issuecomment-38366702 .
Hmm, I might be doing it wrong, then... Thanks for the link!
To make the behaviour consistent across the library, errors should be passed to callbacks instead of throwing them; otherwise it causes some confusion and potentially breaks error handling in other libraries that use this module.
PS. It would be great if you could also release this as a patch update on NPM. Thank you!