Need to capture exception message in node-report

[rnchamberlain]

Currently, node-report uses the isolate->SetAbortOnUncaughtExceptionCallback() API to intercept and trigger a report on an uncaught exception. We can get the JS stack OK, but not the contents of the exception, in particular the exception type and message. We really need to be able to include extra information about the exception in the report header.

Also we could then support filtering on exception types (eg trigger a report on syntax errors only).

[hhellyer]

This is a little bit tricky, there doesn't seem to be a nice way to get the exception inside the function passed to the SetAbortOnUncaughtExceptionCallback. However it is provide in process.on('uncaughtException,...) and process.on('unhandledRejection',...).

I’ve had a quick look at enhancing triggerReport and getReport so an js exception can be passed in and the error message and stack trace can be written along with the current stack trace. It doesn’t exactly hit this requirement but it does have a number of useful qualities:

• We could move our uncaught exception handling to process.on('uncaughtException,…), if we wanted to.
• We could provide some basic promise support by using process.on('unhandledRejection’,…). (This doesn’t fully address @sam-github’s requirement in issue can something useful be reported about unhandled rejections? #75 but it’s a start.)
• It will make node-report more useful for Node.js users who want to embed node-report in their own error handling. (If you want to use node report when a promise is rejected then you probably want the stack trace from the error passed to reject not the stack trace from the rejection code.)

The obvious downside is that it’s a behaviour change. In particular —abort_on_uncaught_exception causes the process.on(‘uncaughtException’,…) callback not to be invoked. (We could possibly use the native hook if —abort_on_uncaught_exception is set and the process.on callback otherwise.)

I’ll produce a prototype that implements support for passing an error to node-report. We should probably discuss whether it’s better to be handling exceptions in native code or JavaScript via process.on() under this issue. (This doesn’t impact other use cases such as when node report is invoked via signal handlers as there won’t be error objects in those cases. For the same reason it should be safe to inspect error objects when they are passed as it can only happen in user error cases where V8 itself is still functioning.)

[rnchamberlain]

Ideally I think we need a new native hook for uncaught exception - so we are independent of —abort_on_uncaught_exception. With the exception object and stack trace as parameters.

[hhellyer]

I've pushed up a change that lets users pass an Error object to triggerReport or getReport: https://github.com/hhellyer/node-report/tree/pass_exception_object

I think this is a generally useful change as it allows someone using node-report in their own error handlers to pass an error object and see the message and stack trace in node-report rather than just the stack for where they handled the error. If other people agree I'd like to raise a pull request just for that. (It will be useful for anyone who requires node-report/api.) I'll add documentation updates and tests when I create the pull request.

There is a separate question over whether and how we should change to this for our default error handling.

• For uncaught exceptions there isn't (as far as I can see) a way to get the exception in the native callback from v8 so we could replace that with a hook to process.on(‘uncaughtException’,…) - if --abort-on-uncaught-exception isn't set. (If it is set the process.on hook never gets called so we'd need to leave things as they are for the moment.)

• For unhandled rejections we could use this to hook process.on('unhandledRejection’,…) however I'm not sure if it would be a good idea to produce a node-report for unhandled rejections. I don't know if there's lots of code out there that ignores it's rejections - one badly behaved package could cause a lot of node-reports to be generated. I don't think unhandled rejections cause Node.js to terminate unlike uncaught exceptions.

[hhellyer]

Sample output from new section:

================================================================================
==== JavaScript Exception Details ==============================================

Uncaught TypeError: foo.bar is not a function
/Users/hhellyer/work/consumability/node/testscripts/uncaughtexception.js:12:5
Module._compile (module.js:571:32)
Module._extensions..js (module.js:580:10)
Module.load (module.js:488:32)
tryModuleLoad (module.js:447:12)
Module._load (module.js:439:3)
Module.runMain (module.js:605:10)
run (bootstrap_node.js:418:7)
startup (bootstrap_node.js:139:9)
bootstrap_node.js:533:3

[hhellyer]

I've raised PR #82 to simply allow a user to pass an Error object when calling node-report.