'error' event proposal
r...@tinyclouds.org
around, we agreed that errors should be thrown if no one was listening
for them. I think the same should be done for EventEmitters in
general. That is, if an event 'error' is emitted but there are no
listeners, then it is thrown. Does anyone have concerns with this
behavior?
Matthew Ranney
Felix Geisendörfer
> listeners, then it is thrown. Does anyone have concerns with this
> behavior?
Silent failures are evil - no concerns, +1.
--fg
Isaac Schlueter
on the "error" event is probably worthwhile.
Could it maybe check to make sure it's an instanceof Error so that it
doesn't throw unless it's something that is worth throwing? Having
the application crash with an obscure string or "[object Object]" and
no stack trace is pretty lame.
--i
2010/3/29 Felix Geisendörfer <fe...@debuggable.com>:
> --
> 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.
>
>
Mikeal Rogers
+1
Ray Morgan
Felix Geisendörfer
> doesn't throw unless it's something that is worth throwing? Having
> the application crash with an obscure string or "[object Object]" and
> no stack trace is pretty lame.
Passing non-errors to an 'error' event deserves punishment, I agree.
But have mercy - killing a misbehaving program seems more ethical than
a life sentence of silent failure to me ; ).
--fg
Aaron Heckmann
Aaron
Isaac Schlueter
> Passing non-errors to an 'error' event deserves punishment, I agree.
> But have mercy - killing a misbehaving program seems more ethical than
> a life sentence of silent failure to me ; ).
Passing a non-Error to an "error" event on an EventEmitter, to me,
more likely implies that the purpose is not to throw, but to do some
other behavior.
For instance, maybe you create a logger with various logging levels,
and emits events ("info", "log", "warn", "error", let's say) for the
various calls to logging functions. It'd be very surprising if you
removed the "error" log handler, and suddenly the program crashes
without a stack trace, when you expected it to ignore those errors.
Only throwing on emitter.emit("error", new Error(...)) seems more
reasonable, and keeps the magic to a minimum. In that case, it's
reasonable to assume that it was supposed to be caught and handled, or
that it should crash the program.
--i
Chris Winberry
for EventEmitters? It would allow logging/reporting of uncaught errors
to be more robust and one would also be able to provide logic to
determine if an error is truly a fatal one or just more of a warning.
Node's behavior right now reminds me of how ASP.Net exceptions would
take down the whole IIS process if they weren't caught, which would
cause availabilty problems in production if there was, say, a
datastore down, and someone forgot to handle it correctly. Minor
problems would snowball into huge ones, due to an "overreaction".
> --
> 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.
>
>
--
Sent from my mobile device
Howard Rauscher
Elijah Insua
Robert Kieffer
that is fault tolerant. It implies that you have to identify and
listen to all possible error sources if you want a server that is
immune to unexpected errors crashing the process, which is a bit of a
daunting problem.
What if EventEmitters invoked a class-level callback for the default
behavior, e.g. "EventEmitter.onUnhandledError", that was defined by
default as:
EventEmitter.onUnhandledError = function(err) {throw err;};
Developers could override this behavior however. E.g. ....
EventEmitter.onUnhandledError = function(err) {sys.debug(err);};
... to prevent unhandled errors from killing the server unexpectedly.
Thoughts?
- rwk
Ryan Dahl
> My only concern would be in how hard/easy this makes writing a server
> that is fault tolerant. It implies that you have to identify and
> listen to all possible error sources if you want a server that is
> immune to unexpected errors crashing the process, which is a bit of a
> daunting problem.
>
> What if EventEmitters invoked a class-level callback for the default
> behavior, e.g. "EventEmitter.onUnhandledError", that was defined by
> default as:
>
> EventEmitter.onUnhandledError = function(err) {throw err;};
>
> Developers could override this behavior however. E.g. ....
>
> EventEmitter.onUnhandledError = function(err) {sys.debug(err);};
>
> ... to prevent unhandled errors from killing the server unexpectedly.
> Thoughts?
There is a process 'unhandledException' event.
It might be nice to have an environ variable which would make the
server not crash.
NODE_IGNORE_ERRORS=1 node myprogram.js
or something.
inimino
> Only throwing on emitter.emit("error", new Error(...)) seems more
> reasonable, and keeps the magic to a minimum. In that case, it's
> reasonable to assume that it was supposed to be caught and handled, or
> that it should crash the program.
I think it's far more magical to throw or not depending on the type
of the argument. An error event is an error event is an error event.
Robert Kieffer
> On Mon, Mar 29, 2010 at 3:30 PM, Robert Kieffer <bro...@gmail.com> wrote:
> > My only concern would be in how hard/easy this makes writing a server
> > that is fault tolerant. It implies that you have to identify and
> > listen to all possible error sources if you want a server that is
> > immune to unexpected errors crashing the process, which is a bit of a
> > daunting problem.
>
> > What if EventEmitters invoked a class-level callback for the default
> > behavior, e.g. "EventEmitter.onUnhandledError", that was defined by
> > default as:
>
> > EventEmitter.onUnhandledError = function(err) {throw err;};
>
> > Developers could override this behavior however. E.g. ....
>
> > EventEmitter.onUnhandledError = function(err) {sys.debug(err);};
>
> > ... to prevent unhandled errors from killing the server unexpectedly.
> > Thoughts?
>
> There is a process 'unhandledException' event.
'Wasn't aware of that. (in fact, just grep'ed for "unhandledException"
in node source and didn't find anything. But no matter.)
And, yeah, the process module is a better place for this kind of API.
However I'm not a fan of the emitter-listener pattern in this case,
because it makes overriding/replacing the default behavior more
difficult. I'd prefer to instead see support for a single
'process.onUnhandledException' callback that provided the default
behavior. The default could do what you're describing...
process.onUnhandledException = function(err) {
process.emit('unhandledException', err);
if (process.ENV.NODE_IGNORE_ERRORS) {
sys.log('Unhandled exception: ' + err.stack);
} else {
throw (err);
}
}
... but could easily be replaced for custom behavior, without having
to muck around in the process._events object to figure out how to
remove any existing listeners that might be throwing the event in ways
you don't want.
> It might be nice to have an environ variable which would make the
> server not crash.
>
> NODE_IGNORE_ERRORS=1 node myprogram.js
>
> or something.
While this is better than nothing, I'd sure like to get the best of
both worlds with something like the above if possible.
Ryan Dahl
> 'Wasn't aware of that. (in fact, just grep'ed for "unhandledException"
> in node source and didn't find anything. But no matter.)
Sorry, "uncaughtException"
> And, yeah, the process module is a better place for this kind of API.
> However I'm not a fan of the emitter-listener pattern in this case,
> because it makes overriding/replacing the default behavior more
> difficult. I'd prefer to instead see support for a single
> 'process.onUnhandledException' callback that provided the default
> behavior. The default could do what you're describing...
>
> process.onUnhandledException = function(err) {
> process.emit('unhandledException', err);
> if (process.ENV.NODE_IGNORE_ERRORS) {
> sys.log('Unhandled exception: ' + err.stack);
> } else {
> throw (err);
> }
> }
>
> ... but could easily be replaced for custom behavior, without having
> to muck around in the process._events object to figure out how to
> remove any existing listeners that might be throwing the event in ways
> you don't want.
Yeah, good point. I'll try it that way.