Add a test case for passing an Error object to triggerReport

Update the API docs for getReport and triggerReport. Fix crash when error object does not include a stack trace.
nodejs · hhellyer · Mar 29, 2017 · Apr 21, 2017 · Apr 25, 2017 · Apr 25, 2017
commit c73968a8d58b508a5ca099049bf65d3245127870
diff --git a/README.md b/README.md
@@ -69,6 +69,18 @@ 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
+fs.stat('/usr/local/fake/fake.txt', (err, stats) => {
+  if(err) {
+    nodereport.triggerReport(err);
+  }
+  ...
+});
+```
+
 ## Configuration
 
 Additional configuration is available using the following APIs:

diff --git a/src/node_report.cc b/src/node_report.cc
@@ -810,6 +810,10 @@ static void PrintJavaScriptErrorStack(std::ostream& out, Isolate* isolate, v8::M
   out << *message_str << "\n";
 
   Local<StackTrace> stack = v8::Exception::GetStackTrace(error.ToLocalChecked());
+  if (stack.IsEmpty()) {
+     out << "\nNo stack trace available from Exception::GetStackTrace()\n";
+     return;
+  }
   // Print the stack trace, adding in the pc values from GetStackSample() if available
   for (int i = 0; i < stack->GetFrameCount(); i++) {
     PrintStackFrame(out, isolate, stack->GetFrame(i), i, nullptr);

diff --git a/test/common.js b/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

diff --git a/test/test-api-pass-error.js b/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",
+    });
+  });
+}