On 16 May 2015 at 18:35:15, Owen Allen ([email protected]) wrote:
There's a lot of blog posts around the internet talking about the mechanics of
handling errors, be it domains or process.on("uncaughtException"). I can't find
much definitive talk about the philosophy of error handling though.
Specifically when should a function return an error cb(err), or return an error
state such as cb(null, { success : false, message : "Some useful message", resp
: httpResponse }).
In example, lets imagine we're creating a social media NPM package which has an
async function which queries a social media API via an HTTP call using the
popular request module and returns some posts. Our function takes 3 arguments a
userid and a count and a callback. There are a variety of exit scenarios for
this function:
So I've responded with what I think.
1. Malformed arguments sent to the function such as a missing cb or a
null/undefined/incorrectly formatted userid.
Throw -- use a library like aproba or otherwise validate args synchronously.
2. Unable to reach the service via HTTP.
Call back with error.
3. Service returned a non-200 status code for some reason, maybe we're being
rate-limited.
If a library that exposes HTTP semantics, call back with no error but object
with status.
If a library that hides HTTP semantics, call back with error.
It will really depend on what layer you're trying to expose; mixing them
usually leads to confusing APIs. Clarifying what layer you are targeting can
really help clarify how the API should work.
4. Reached the service, but the call failed at the service due to invalid
parameters, some services return a 200 status code for this type of failure
other's return a non-200 status code.
Services that report errors with 200 are awful. Fix that up as if the service
behaves.
If it's a protocol concept that's exposed, call back with status object. If
you're wrapping the protocol so the details aren't exposed, call back with
error.
5. Reached the service, the call succeeded but returned an empty array of posts.
Empty success is still success. Call back with the empty array.
6. Reached the service, the call succeeded and returned data.
Call back with it.
7. The function throws an unhandled error due to programmer mistake.
Bummer. Catch that internally and call back with it. An async throw due to that
kind of mistake in a callback is awful and crashes the whole process
unrecoverably. This is the worst side-effect of error handling in node.
Static analysis to find cases, remove the catch when you get it right.
Usually my philosophy for handling errors in this case would be:
1. cb(new Error("Invalid parameters, must pass a callback"));
2. cb(err); // the error that was returned by the http request
3. cb(null, { success : false, message : "Service returned a " +
resp.statusCode + " status code", resp : resp });
4. cb(null, { success : false, message : serviceResponse.message, resp : resp
});
5. cb(null, { success : true, data : [] });
6. cb(null, { success : true, data : posts });
7. Whoops, let error bubble up to the application employing our package and it
will handle it or crash.
In general I find the success true/false to be a bad pattern: Either expose the
protocol _and all the details_ for it, or don't, and hide them completely into
the error path as debugging data, but don't expect the users of your API to act
on that information.
I attach code properties to errors where I can. Use the VError class to attach
context to inner exceptions, and to make formatting error messages easier with
format strings.
That's just one way to approach the problem. It could be argued that cases 1 -
4 should all cb an error and that the error object should be packed with the
additional data to help debug or be a custom error type. Yet at the same time
some people argue it's a bad idea to pack additional non-standard keys on to
Error objects because it results in inconsistency between Error objects and
modules. If every npm module packed it's Error objects with custom keys
wouldn't we end up in a pretty chaotic situation? In addition, if that error is
thrown or console.error() those extra keys will not show up because of the
native behavior of Error.prototype.toString().
Use the code property for consistent, machine-matchable tokens on errors.
Custom subclasses are just annoying to define and give little useful
information that a code property won't. Use the message for human readable
message.
Yet another approach would be for 1 - 4 to return an error AND the additional
data. Such as cb(new Error(serviceResp.message), { success : false, message :
serviceResp.message, resp : resp }). This way downstream modules have the
useful debugging information available, but we still end up treading down the
error pathway of control flow structures such as async and promises. In
example, in async if an error is returned it short-circuits all the way to the
end.
I rarely see that go well -- sometimes it's clever and nice, but generally, you
don't need it, and it's only confusing. And it's not particularly composable,
in that not all error handlers do this, and passing two parameters through a
complex chain of functions is far more difficult to get right. You end up
throwing away the extra data if it's not attached to the error object, so just
do that instead.
Lots of this is subjective, or just plain "better if people tend to do it the
same way", but I find this approach works for me.
Aria
--
Job board: http://jobs.nodejs.org/
New group rules:
https://gist.github.com/othiym23/9886289#file-moderation-policy-md
Old group rules:
https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
---
You received this message because you are subscribed to the Google Groups
"nodejs" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/nodejs/etPan.5557ca1b.1688145c.ddfd%40Corona.local.
For more options, visit https://groups.google.com/d/optout.
