diff --git a/README.md b/README.md index 378aa24..4e7b3f5 100644 --- a/README.md +++ b/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: diff --git a/src/module.cc b/src/module.cc index fb60a2b..5c2f76d 100644 --- a/src/module.cc +++ b/src/module.cc @@ -51,6 +51,8 @@ NAN_METHOD(TriggerReport) { Nan::HandleScope scope; v8::Isolate* isolate = info.GetIsolate(); char filename[NR_MAXNAME + 1] = ""; + MaybeLocal 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 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()); } 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()); } 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()); } 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()); } report_signal = 0; } diff --git a/src/node_report.cc b/src/node_report.cc index cb4827b..f6cc7eb 100644 --- a/src/node_report.cc +++ b/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 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 error); static void PrintStackFromStackTrace(std::ostream& out, Isolate* isolate, DumpEvent event); static void PrintStackFrame(std::ostream& out, Isolate* isolate, Local 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 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 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 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 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 error) { + if (error.IsEmpty() || !error.ToLocalChecked()->IsNativeError()) { + return; + } + + out << "\n================================================================================"; + out << "\n==== JavaScript Exception Details ==============================================\n\n"; + Local message = v8::Exception::CreateMessage(isolate, error.ToLocalChecked()); + Nan::Utf8String message_str(message->Get()); + + out << *message_str << "\n\n"; + + Local 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, LocalIsEval()) { diff --git a/src/node_report.h b/src/node_report.h index 567daa5..b597303 100644 --- a/src/node_report.h +++ b/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 error); +void GetNodeReport(Isolate* isolate, DumpEvent event, const char* message, const char* location, v8::MaybeLocal error, std::ostream& out); unsigned int ProcessNodeReportEvents(const char* args); unsigned int ProcessNodeReportSignal(const char* args); diff --git a/test/common.js b/test/common.js index 1c0a969..dbf24d8 100644 --- 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 new file mode 100644 index 0000000..0e7789f --- /dev/null +++ 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", + }); + }); +}