Allow Error object to be passed to node-report

Add an optional parameter to triggerReport and getReport so an
Error object can be passed. If it is passed the error message
and stack trace it contains will be added to the node-report
output in a new "JavaScript Exception Details" section.

This makes node-report more useful in custom error handlers as
it will include the stack trace of the original error as well
as the stack trace where the error was handled.

PR-URL: #82
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Richard Chamberlain <[email protected]>

Author: hhellyer

README.md

@@ -69,6 +69,26 @@ can be specified as a parameter on the `triggerReport()` call.
 nodereport.triggerReport("myReportName");
 ```
 
+Both `triggerReport()` and `getReport()` can take an optional `Error` object
+as a parameter. If an `Error` object is provided, the message and stack trace
+from the object will be included in the report in the `JavaScript Exception
+Details` section.
+When using node-report to handle errors in a callback or an exception handler
+this allows the report to include the location of the original error as well
+as where it was handled.
+If both a filename and `Error` object are passed to `triggerReport()` the
+`Error` object should be the second parameter.
+
+```js
+try {
+  process.chdir('/foo/foo');
+} catch (err) {
+  nodereport.triggerReport(err);
+}
+  ...
+});
+```
+
 ## Configuration
 
 Additional configuration is available using the following APIs:

src/module.cc

@@ -51,6 +51,8 @@ NAN_METHOD(TriggerReport) {
   Nan::HandleScope scope;
   v8::Isolate* isolate = info.GetIsolate();
   char filename[NR_MAXNAME + 1] = "";
+  MaybeLocal<Value> error;
+  int err_index = 0;
 
   if (info[0]->IsString()) {
     // Filename parameter supplied
@@ -60,9 +62,16 @@ NAN_METHOD(TriggerReport) {
     } else {
       Nan::ThrowError("node-report: filename parameter is too long");
     }
+    err_index++;
   }
+
+  // We need to pass the JavaScript object so we can query it for a stack trace.
+  if (info[err_index]->IsNativeError()) {
+    error = info[err_index];
+  }
+
   if (nodereport_events & NR_APICALL) {
-    TriggerNodeReport(isolate, kJavaScript, "JavaScript API", __func__, filename);
+    TriggerNodeReport(isolate, kJavaScript, "JavaScript API", __func__, filename, error);
     // Return value is the report filename
     info.GetReturnValue().Set(Nan::New(filename).ToLocalChecked());
   }
@@ -77,7 +86,12 @@ NAN_METHOD(GetReport) {
   v8::Isolate* isolate = info.GetIsolate();
   std::ostringstream out;
 
-  GetNodeReport(isolate, kJavaScript, "JavaScript API", __func__, out);
+  MaybeLocal<Value> error;
+  if (info[0]->IsNativeError()) {
+    error = info[0];
+  }
+
+  GetNodeReport(isolate, kJavaScript, "JavaScript API", __func__, error, out);
   // Return value is the contents of a report as a string.
   info.GetReturnValue().Set(Nan::New(out.str()).ToLocalChecked());
 }
@@ -156,7 +170,7 @@ static void OnFatalError(const char* location, const char* message) {
   }
   // Trigger report if requested
   if (nodereport_events & NR_FATALERROR) {
-    TriggerNodeReport(Isolate::GetCurrent(), kFatalError, message, location, nullptr);
+    TriggerNodeReport(Isolate::GetCurrent(), kFatalError, message, location, nullptr, MaybeLocal<Value>());
   }
   fflush(stderr);
   raise(SIGABRT);
@@ -165,7 +179,7 @@ static void OnFatalError(const char* location, const char* message) {
 bool OnUncaughtException(v8::Isolate* isolate) {
   // Trigger report if requested
   if (nodereport_events & NR_EXCEPTION) {
-    TriggerNodeReport(isolate, kException, "exception", __func__, nullptr);
+    TriggerNodeReport(isolate, kException, "exception", __func__, nullptr, MaybeLocal<Value>());
   }
   if ((commandline_string.find("abort-on-uncaught-exception") != std::string::npos) ||
       (commandline_string.find("abort_on_uncaught_exception") != std::string::npos)) {
@@ -231,7 +245,7 @@ static void SignalDumpInterruptCallback(Isolate* isolate, void* data) {
         fprintf(stdout, "node-report: SignalDumpInterruptCallback triggering report\n");
       }
       TriggerNodeReport(isolate, kSignal_JS,
-                        node::signo_string(report_signal), __func__, nullptr);
+                        node::signo_string(report_signal), __func__, nullptr, MaybeLocal<Value>());
     }
     report_signal = 0;
   }
@@ -246,7 +260,7 @@ static void SignalDumpAsyncCallback(uv_async_t* handle) {
         fprintf(stdout, "node-report: SignalDumpAsyncCallback triggering NodeReport\n");
       }
       TriggerNodeReport(Isolate::GetCurrent(), kSignal_UV,
-                        node::signo_string(report_signal), __func__, nullptr);
+                        node::signo_string(report_signal), __func__, nullptr, MaybeLocal<Value>());
     }
     report_signal = 0;
   }

src/node_report.cc

@@ -82,10 +82,11 @@ typedef struct tm TIME_TYPE;
 #endif
 
 // Internal/static function declarations
-static void WriteNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, char* filename, std::ostream &out, TIME_TYPE* time);
+static void WriteNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, char* filename, std::ostream &out, MaybeLocal<Value> error, TIME_TYPE* time);
 static void PrintCommandLine(std::ostream& out);
 static void PrintVersionInformation(std::ostream& out);
 static void PrintJavaScriptStack(std::ostream& out, Isolate* isolate, DumpEvent event, const char* location);
+static void PrintJavaScriptErrorStack(std::ostream& out, Isolate* isolate, MaybeLocal<Value> error);
 static void PrintStackFromStackTrace(std::ostream& out, Isolate* isolate, DumpEvent event);
 static void PrintStackFrame(std::ostream& out, Isolate* isolate, Local<StackFrame> frame, int index, void* pc);
 static void PrintNativeStack(std::ostream& out);
@@ -378,8 +379,9 @@ void SetCommandLine() {
  *   const char* message
  *   const char* location
  *   char* name - in/out - returns the report filename
+ *   MaybeLocal<Value> error - JavaScript Error object.
  ******************************************************************************/
-void TriggerNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, char* name) {
+void TriggerNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, char* name, MaybeLocal<Value> error) {
   // Recursion check for report in progress, bail out
   if (report_active) return;
   report_active = true;
@@ -460,7 +462,7 @@ void TriggerNodeReport(Isolate* isolate, DumpEvent event, const char* message, c
   // Pass our stream about by reference, not by copying it.
   std::ostream &out = outfile.is_open() ? outfile : *outstream;
 
-  WriteNodeReport(isolate, event, message, location, filename, out, &tm_struct);
+  WriteNodeReport(isolate, event, message, location, filename, out, error, &tm_struct);
 
   // Do not close stdout/stderr, only close files we opened.
   if(outfile.is_open()) {
@@ -474,7 +476,7 @@ void TriggerNodeReport(Isolate* isolate, DumpEvent event, const char* message, c
 
 }
 
-void GetNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, std::ostream& out) {
+void GetNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, MaybeLocal<Value> error, std::ostream& out) {
   // Obtain the current time and the pid (platform dependent)
   TIME_TYPE tm_struct;
 #ifdef _WIN32
@@ -484,7 +486,7 @@ void GetNodeReport(Isolate* isolate, DumpEvent event, const char* message, const
   gettimeofday(&time_val, nullptr);
   localtime_r(&time_val.tv_sec, &tm_struct);
 #endif
-  WriteNodeReport(isolate, event, message, location, nullptr, out, &tm_struct);
+  WriteNodeReport(isolate, event, message, location, nullptr, out, error, &tm_struct);
 }
 
 static void walkHandle(uv_handle_t* h, void* arg) {
@@ -531,7 +533,7 @@ static void walkHandle(uv_handle_t* h, void* arg) {
   *out << buf;
 }
 
-static void WriteNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, char* filename, std::ostream &out, TIME_TYPE* tm_struct) {
+static void WriteNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, char* filename, std::ostream &out, MaybeLocal<Value> error, TIME_TYPE* tm_struct) {
 
 #ifdef _WIN32
   DWORD pid = GetCurrentProcessId();
@@ -589,6 +591,11 @@ static void WriteNodeReport(Isolate* isolate, DumpEvent event, const char* messa
   PrintNativeStack(out);
   out << std::flush;
 
+  // Print the stack trace and message from the Error object.
+  // (If one was provided.)
+  PrintJavaScriptErrorStack(out, isolate, error);
+  out << std::flush;
+
   // Print V8 Heap and Garbage Collector information
   PrintGCStatistics(out, isolate);
   out << std::flush;
@@ -791,6 +798,29 @@ static void PrintJavaScriptStack(std::ostream& out, Isolate* isolate, DumpEvent
 #endif
 }
 
+static void PrintJavaScriptErrorStack(std::ostream& out, Isolate* isolate, MaybeLocal<Value> error) {
+  if (error.IsEmpty() || !error.ToLocalChecked()->IsNativeError()) {
+    return;
+  }
+
+  out << "\n================================================================================";
+  out << "\n==== JavaScript Exception Details ==============================================\n\n";
+  Local<Message> message = v8::Exception::CreateMessage(isolate, error.ToLocalChecked());
+  Nan::Utf8String message_str(message->Get());
+
+  out << *message_str << "\n\n";
+
+  Local<StackTrace> stack = v8::Exception::GetStackTrace(error.ToLocalChecked());
+  if (stack.IsEmpty()) {
+     out << "No stack trace available from Exception::GetStackTrace()\n";
+     return;
+  }
+  // Print the stack trace, samples are not available as the exception isn't from the current stack.
+  for (int i = 0; i < stack->GetFrameCount(); i++) {
+    PrintStackFrame(out, isolate, stack->GetFrame(i), i, nullptr);
+  }
+}
+
 /*******************************************************************************
  * Function to print stack using GetStackSample() and StackTrace::StackTrace()
  *
@@ -842,13 +872,14 @@ static void PrintStackFrame(std::ostream& out, Isolate* isolate, Local<StackFram
   char buf[64];
 
   // First print the frame index and the instruction address
+  if (pc != nullptr) {
 #ifdef _WIN32
-  snprintf( buf, sizeof(buf), "%2d: [pc=0x%p] ", i, pc);
-  out << buf;
+    snprintf( buf, sizeof(buf), "%2d: [pc=0x%p] ", i, pc);
 #else
-  snprintf( buf, sizeof(buf), "%2d: [pc=%p] ", i, pc);
-  out << buf;
+    snprintf( buf, sizeof(buf), "%2d: [pc=%p] ", i, pc);
 #endif
+    out << buf;
+  }
 
   // Now print the JavaScript function name and source information
   if (frame->IsEval()) {

src/node_report.h

@@ -19,6 +19,7 @@ using v8::String;
 using v8::Value;
 using v8::StackTrace;
 using v8::StackFrame;
+using v8::MaybeLocal;
 
 // Bit-flags for node-report trigger options
 #define NR_EXCEPTION  0x01
@@ -32,8 +33,8 @@ using v8::StackFrame;
 
 enum DumpEvent {kException, kFatalError, kSignal_JS, kSignal_UV, kJavaScript};
 
-void TriggerNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, char* name);
-void GetNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, std::ostream& out);
+void TriggerNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, char* name, v8::MaybeLocal<v8::Value> error);
+void GetNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, v8::MaybeLocal<v8::Value> error, std::ostream& out);
 
 unsigned int ProcessNodeReportEvents(const char* args);
 unsigned int ProcessNodeReportSignal(const char* args);

test/common.js

@@ -58,11 +58,18 @@ exports.validateContent = function validateContent(data, t, options) {
   const expectedVersions = options ?
                            options.expectedVersions || nodeComponents :
                            nodeComponents;
-  var plan = REPORT_SECTIONS.length + nodeComponents.length + 5;
+  const expectedException = options.expectedException;
+  if (options.expectedException) {
+    REPORT_SECTIONS.push('JavaScript Exception Details');
+  }
+
+  let plan = REPORT_SECTIONS.length + nodeComponents.length + 5;
   if (options.commandline) plan++;
+  if (options.expectedException) plan++;
   const glibcRE = /\(glibc:\s([\d.]+)/;
   const nodeReportSection = getSection(reportContents, 'Node Report');
   const sysInfoSection = getSection(reportContents, 'System Information');
+  const exceptionSection = getSection(reportContents, 'JavaScript Exception Details');
   const libcPath = getLibcPath(sysInfoSection);
   const libcVersion = getLibcVersion(libcPath);
   if (glibcRE.test(nodeReportSection) && libcVersion) plan++;
@@ -84,6 +91,10 @@ exports.validateContent = function validateContent(data, t, options) {
             new RegExp('Node.js version: ' + process.version),
             'Node Report header section contains expected Node.js version');
   }
+  if (options && options.expectedException) {
+	  t.match(exceptionSection, new RegExp('Uncaught Error: ' + options.expectedException),
+      'Node Report JavaScript Exception contains expected message');
+  }
   if (options && options.commandline) {
     if (this.isWindows()) {
       // On Windows we need to strip double quotes from the command line in

test/test-api-pass-error.js

@@ -0,0 +1,29 @@
+'use strict';
+
+// Testcase for passing an error object to the API call.
+
+if (process.argv[2] === 'child') {
+  const nodereport = require('../');
+  try {
+    throw new Error("Testing error handling");
+  } catch (err) {
+    nodereport.triggerReport(err);
+  }
+} else {
+  const common = require('./common.js');
+  const spawn = require('child_process').spawn;
+  const tap = require('tap');
+
+  const child = spawn(process.execPath, [__filename, 'child']);
+  child.on('exit', (code) => {
+    tap.plan(3);
+    tap.equal(code, 0, 'Process exited cleanly');
+    const reports = common.findReports(child.pid);
+    tap.equal(reports.length, 1, 'Found reports ' + reports);
+    const report = reports[0];
+    common.validate(tap, report, {pid: child.pid,
+      commandline: child.spawnargs.join(' '),
+      expectedException: "Testing error handling",
+    });
+  });
+}