Suggestion for standardised error handling in NodeJS - because at the moment its a bunch of &%*$

[Guy Halford-Thompson]

Hi All,

My biggest gripe with nodejs is that error handling can be tricky,
especially when dealing with nested callbacks on multiple levels.
There are some really great libraries out there, but I see little or
no error handling in many of them. What I would like to do, is
suggest a simple, standard way of handling errors in NodeJS, to
encourage better error handling across NodeJS apps and libraries.

Currently, if there _is_ any error handling, it is usually
accomplished by passing an error argument to a callback.

function doSomeAsyncStuff(cb) {
if {
// An error occured
cb(null, 'An error occured');
} else {
// No error
cb(someData);
}
}

The above requires you to call doSomeAsyncStuff as follows

doSomeAsyncStuff(function(data, err) { ...... });

The problem with the above should be obvious. I would estimate that
70% of apps out there just flat out ignore the err parameter. I would
estimate that another 20% call console.log(err) and leave it at that.
The remaining 10% will handle it in a sensible way and pass the error
message to some handling function.

One _correct_ way to handle this would be as follows

doSomeAsyncStuff(function(data, err) {
if(err) {
myErrorHandler.handleError(err);
} else {
//...... keep going
}
});

There are three inherent problems with this.

1) It is not easy to build good error handling functions. Every
level of callback requires an if / else statement to determine if an
error occured.
2) There is no way of determining the severity of the error. Was it
fatal? Can we continue? Is the world going to end? Seriously! We
have no idea.
3) Developers are lazy. I should know! Especially when it comes to
error handling. If we can cut corners, we will. Especially when
presented with a situation like the above, a lot of the time we just
think, fk it, an error isnt likely to occur here, and move on. If we
are in a good mood, we might put a console.log(err) line in. (Okay,
this is a bit harsh, but I am sure you see my point)

My suggestion is to put the error handling decisions in the hands of
the library throwing the error, not the application receiving it. If
a library wants to throw an error, it is trying to tell us that
something went wrong, and that it could have a negative effect on our
app. A library doesnt _have_ to do it. It could just pack up and
crash, but no, it tells our app that something bad happened and we
should tread carefully. We should at least have the decency to
acknowledge it!!

So, my suggestion is that functions that can throw errors take an
error handling function as a parameter. Consider the following

aFunctionThatCanError(err, cb) {
// Do some stuff
if {
// Error occured
err('An error occured');
} else {
// Keep up the good work
cb(data);
}
}

Now, you, as the developer of an app, just need to write a simple error handler.

function myErrorHandler(err) {
// Oops, an error occured
console.log(err);
myApp.redirect('404', err);
}

now

aFunctionThatCanError(myErrorHandler, function(data) {
// No error can have occurred, or this callback would never have
been invoked.
});

This has multiple benefits

1) It is simple!
2) Is allows developers to standardize error handling across their app
3) It FORCES developers to implement error handling where it is essential

So.... I am interested to hear what the community has to say on this
matter... even if its to tell me where to shove it... but whatever...
the fact is that error handling in its current state sucks.

G

--
Guy Halford-Thompson
GinkApps - http://ginkapps.com
Twitter               - https://twitter.com/mrwooster
Google Plus       - http://gplus.name/guy

[Adam Crabtree]

While I definitely agree with your sentiments regarding error handling, and personally find myself developing alternatives to doing it manually (see http://github.com/crabdude/trycatch), I don't think this makes a lot of sense. Your proposal is not functionally different from how things are done now except that you're forcing error handling response code to be separated from non-error response code, whereas the current state leaves it up to you. 

Honestly, just do the following:

if (err) return onError(err);

--
Job Board: http://jobs.nodejs.org/
Posting guidelines: 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 post to this group, send email to nod...@googlegroups.com
To unsubscribe from this group, send email to
nodejs+un...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/nodejs?hl=en?hl=en

--
Better a little with righteousness
       than much gain with injustice.
Proverbs 16:8

[Guy Halford-Thompson]

My argument is more that this method forces the developer to
acknowledge that an error can occur. If developers can get away with
ignoring errors... in a lot of cases they will, and this can lead to
very dangerous code.

--
Guy Halford-Thompson
Blog                  - http://www.cach.me/blog

[Adam Crabtree]

True, that's an important distinction, but honestly, people will probably continue doing exactly what they do right now:

async(function(err) {

  console.log(err);

}, function(data) {

  // do something

});

and it still makes it difficult to ignore the error. Sometimes there's an error, but we expect it and don't care.

[Guy Halford-Thompson]

Sure... and you could even do the following : async(function(err) {},
function(data) { ........ });

But anyone who looks at the above code will immediately see its very
badly written (whereas this would not be the case for current error
handling systems).

I am not saying that my suggestion is the best way... but that the
NodeJS community needs to come up with _something_, and that
_something_ should be governed by 2 fundamental rules

1) Where error handling is essential, it should be enforced
2) Error handling should be easy to implement for the developer

[Marco Rogers]

I certainly understand your frustration. But the simple fact is, no matter where you put the error indicator, people who don't want to deal with it just won't deal with it. Contorting the api to make it more annoying to handle errors will only gain you a few percentage points. That's just my personal opinion based on my experiences.

That being said, you have the node convention backwards anyway. The callback parameters are function(err, data, ...){ }. When you look at things this way, it's not so different from your proposal. Put the error handling first so you can't simply ignore it. But people still do. The difference between your proposal and the current convention is that yours requires functions to be passed around everywhere instead of error objects. Neither is ideal, and I like that we're still looking for better alternatives. But I don't think this proposal has enough gains.

:Marco

[Bruno Jouhier]

I can see some advantages to Guy's proposal: it does not just make it
harder to ignore errors, it also makes it easier to implement try/
catch style error handling. If you have a stack like C1 <- C2 <- C3
and you want to catch in C1 all the errors that are raised below (by
C2, C2), you can just declare an error handler in C1 and pass it to
all calls below it. You don't have to insert "if (e) return cb(e)"
tests in every callback to propagate the error.

On the other hand, the proposal comes really late in the game and
would be very disruptive. Having two API conventions for EH will
probably drive developers crazy (extra code to write to switch from
one style to another).

My 2 cents (I'm not really concerned anyway as I'm using a
preprocessor that screens me from all this).

Bruno

[Ryan Schmidt]

On Sep 30, 2011, at 14:41, Bruno Jouhier wrote:

> (I'm not really concerned anyway as I'm using a
> preprocessor that screens me from all this).

Can you give more details on this?

[Bruno Jouhier]

I'm the author of streamline.js, a tool that tries to ease the pain
with callbacks. I did not want to advertise it loudly one more time
here but if you are asking for details, here is how you do exception
handling with streamline:

function asyncF1(name, _) {
var res = allocResourceAsync(name, _);
try {
asyncF2(res, _);
while (asyncF2(_)) {
asyncF3(res, _);
}
return asyncF4(res, _);
}
catch (ex) {
reportAsync(ex, _);
return null;
}
finally {
releaseResourceAsync(res, _);
}
}

The preprocessor transform the code so that good old JS try/catch/
finally constructs work as expected.

demo: http://sage.github.com/streamlinejs/examples/streamlineMe/streamlineMe.html
blog intro: http://bjouhier.wordpress.com/2011/01/09/asynchronous-javascript-the-tale-of-harry/
ugly details: http://bjouhier.wordpress.com/2011/05/24/yield-resume-vs-asynchronous-callbacks/
github repo: https://github.com/Sage/streamlinejs

The fibers/futures combination will give you a similar feel (with a
very different implementation technique). See https://github.com/laverdet/node-fibers

Bruno

Bruno


Bruno

[Ryan Schmidt]

On Sep 30, 2011, at 15:24, Bruno Jouhier wrote:

> On Sep 30, 9:56 pm, Ryan Schmidt wrote:
>> On Sep 30, 2011, at 14:41, Bruno Jouhier wrote:
>>
>>> (I'm not really concerned anyway as I'm using a
>>> preprocessor that screens me from all this).
>>
>> Can you give more details on this?
>
> I'm the author of streamline.js, a tool that tries to ease the pain
> with callbacks. I did not want to advertise it loudly one more time
> here but if you are asking for details, here is how you do exception
> handling with streamline:

Thanks. I did take note of your earlier message about streamline and am looking into it.

> function asyncF1(name, _) {
> var res = allocResourceAsync(name, _);
> try {
> asyncF2(res, _);
> while (asyncF2(_)) {
> asyncF3(res, _);
> }
> return asyncF4(res, _);
> }
> catch (ex) {
> reportAsync(ex, _);
> return null;
> }
> finally {
> releaseResourceAsync(res, _);
> }
> }
>
> The preprocessor transform the code so that good old JS try/catch/
> finally constructs work as expected.

Hmm.... try/catch/finally has never fit into my brain properly so I may be better off doing standard node error handling.

[Liam]

The correct default error handling pattern is:

asyncThing(input, function(err, output) {
if (err) throw err; // halt on unexpected error
...
});

The correct intelligent pattern is:

asyncThing(input, function(err, output) {
if (err && err.member !== expected_value) throw err;
...
});

[Ryan Schmidt]

On Sep 30, 2011, at 19:15, Liam wrote:

> The correct default error handling pattern is:
>
> asyncThing(input, function(err, output) {
> if (err) throw err; // halt on unexpected error
> ...
> });

Could you discuss this further, and why this is correct/better? In most examples, I've seen:

if (err) return next(err);

But I've also seen what you showed:

if (err) throw err;

And I don't understand how to decide which to use, or what the implications would be for other code that calls this code.

[Liam]

I was describing the user/client pattern. The library pattern is:

Class.prototype.method = function(input, callback) {
...
if (problem)
return callback(new Error(problem));
...
callback(null, results);
};

[strager]

My main problems with the proposal are that they don't do anything new
and they make writing code which wants to ignore errors more difficult
to write.

There's also the question of whether the 'normal' callback can be
called even if the errback has been called (e.g. some library author
may call the errback on a timeout, but still call the callback when
data does finally arrive). I think the convention of "callback is
only called once (if ever), with at least one argument 'err'" is
simpler and less error-prone in terms of just 'getting right'.

I suspect that amateur code written in the proposed style will just
print the exception and the program would just hang.

On try/catch: I don't see the reason to try/catch around every
callback (or have a preprocessor or programming pattern do it for
you). Usually, I want the program to crash if I make a type error (as
most exceptions I have are type errors (and syntax errors)); it
shouldn't really go through my error path. Maybe someone more
experienced with library authorship can comment on this.

And of course, the current Node style has momentum already, so that
must be overcome with an alternative. I think using something like
promises is a better approach than revising the current Node
convention.

On Sep 30, 9:24 am, Guy Halford-Thompson <g...@cach.me> wrote:

[Guy Halford-Thompson]

On 30 September 2011 19:49, strager <strag...@gmail.com> wrote:
> My main problems with the proposal are that they don't do anything new
> and they make writing code which wants to ignore errors more difficult
> to write.

In my opinion making writing code that wants to ignore errors more
difficult is a good thing.

>
> There's also the question of whether the 'normal' callback can be
> called even if the errback has been called (e.g. some library author
> may call the errback on a timeout, but still call the callback when
> data does finally arrive).  I think the convention of "callback is
> only called once (if ever), with at least one argument 'err'" is
> simpler and less error-prone in terms of just 'getting right'.

Agreed. But again, this puts the decision making in the hands of the
library throwing the error, not in the developer using the library

Yes, convention is a problem, but there is currently no real
'standard' way to throw errors, and many libraries do it slightly
differently.

I would urge people to give this proposal a bit of thought, and maybe
just play around with it with some test code. I have been using it
for a while now, and it makes the error handling process easy to
implement, robust, and standard across all callbacks. Something that
NodeJS hasn't seen yet,

G

> --
> Job Board: http://jobs.nodejs.org/
> Posting guidelines: 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 post to this group, send email to nod...@googlegroups.com
> To unsubscribe from this group, send email to
> nodejs+un...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/nodejs?hl=en?hl=en
>

--
Guy Halford-Thompson
Blog                  - http://www.cach.me/blog