Repository navigation
illegal hardware exception #6
Description
Activity
Its fairly reproduceable. I ran with gdb, too:
Node.js report completed Uncaught #<UserException> FROM Server.my_listener (/home/sam/w/core/nodereport/demo/exception.js:15:5) emitTwo (events.js:106:13) Server.emit (events.js:191:7) HTTPParser.parserOnIncoming [as onIncoming] (_http_server.js:546:12) HTTPParser.parserOnHeadersComplete (_http_common.js:99:23) Thread 1 "node" received signal SIGILL, Illegal instruction. 0x000000000132de89 in v8::base::OS::Abort() () (gdb) bt #0 0x000000000132de89 in v8::base::OS::Abort() () #1 0x0000000000d4e30a in v8::internal::Isolate::Throw(v8::internal::Object*, v8::internal::MessageLocation*) () #2 0x0000000000eaf423 in v8::internal::Runtime_Throw(int, v8::internal::Object**, v8::internal::Isolate*) ()Yes, nodereport sets --abort_on_uncaught_exception, and uses
isolate->SetAbortOnUncaughtExceptionCallback()to intercept the exception. So the behaviour you are seeing should be the same as when running node with --abort_on_uncaught_exception. On Linux thev8::base::OS::Abort()code intentionally aborts and triggers a core dump using an illegal instruction. The nodereport setCoreDump(yes/no) API should let you switch to node behaviour without --abort_on_uncaught_exception. Maybe that should be the default.I think normal node behaviour should be the default, yes. If people want abort on uncaught exception, they can set that flag on the command line.
But... it sounds like the mechanism you are using won't allow that. Have you considered using
process.once('uncaughtException', ..(in c++) to catch uncaught exceptions? That would allow you to re-raise them, I think, and for whatever the "normal" thing is to happen (normal means uncaught exceptions usually print stack traces and exit with1, unless--abort_on_uncaught_exceptionis set in which case it aborts, or unless some user has unwisely added an event listener foruncaughtException, in which case they are responsible for handling it).I'm pretty surprised node aborts with an illegal instruction and not
abort(3).This still needs more work:
-
on node 4 and 6, with node-report enabled, the uncaught exception prints a stack trace but node continues rather than exiting with rc=1
-
the demos need a refresh to reflect the node-report defaults change, plus improvement, e.g buttons to trigger the required events rather than browser refresh
-
We replace Node.js'
ShouldAbortOnUncaughtExceptioncallback with our own. Should we duplicate the logic?There is a complicated story behind ShouldAbortOnUncaughtException() in node.cc, see:
nodejs/node#3036
nodejs/node#3654The logic was added to fix behaviour when domains are used. It prevents the --abort-on-uncaught-exception option from triggering an abort when domains are used to catch errors. In practice, domains are deprecated, and the call to TopDomainHasErrorHandler(env) has a simple check, which means that the logic is a no-op now:
if (!env->using_domains()) return false;So I've simply omitted that code in the node-report callback
@sam-github latest node-report v2.1.2 has the fix to the exception handler, and the improved demos, hopefully we can close this now
Is this expected?
System info:
I got this when I was trying to verify whether nodereport after reporting an uncaught exception, would let the exception continue to propagate and cause node to exit (as it should).