From 3fe1ceec5d92a6338fff1c795e6af467d4ebd33e Mon Sep 17 00:00:00 2001 From: himself65 Date: Wed, 26 Jun 2019 00:09:14 +0800 Subject: [PATCH] src: add errorProperties on process.report PR-URL: https://github.com/nodejs/node/pull/28426 Reviewed-By: Gireesh Punathil Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Rich Trott --- lib/internal/process/execution.js | 2 +- lib/internal/process/report.js | 4 +- src/node_errors.cc | 2 +- src/node_report.cc | 92 ++++++++++++++++++++------ src/node_report.h | 4 +- src/node_report_module.cc | 18 +++-- test/common/report.js | 37 +++++++++-- test/report/test-report-getreport.js | 7 ++ test/report/test-report-writereport.js | 9 ++- 9 files changed, 137 insertions(+), 38 deletions(-) diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index bbefcd47c74617..0d6b5cdf73cf23 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -157,7 +157,7 @@ function createOnGlobalUncaughtException() { report.writeReport(er ? er.message : 'Exception', 'Exception', null, - er ? er.stack : undefined); + er ? er : {}); } } catch {} // Ignore the exception. Diagnostic reporting is unavailable. } diff --git a/lib/internal/process/report.js b/lib/internal/process/report.js index 72a8b0c1ebe337..114e35ecf2bd06 100644 --- a/lib/internal/process/report.js +++ b/lib/internal/process/report.js @@ -25,7 +25,7 @@ const report = { throw new ERR_INVALID_ARG_TYPE('err', 'Object', err); } - return nr.writeReport('JavaScript API', 'API', file, err.stack); + return nr.writeReport('JavaScript API', 'API', file, err); }, getReport(err) { if (err === undefined) @@ -33,7 +33,7 @@ const report = { else if (err === null || typeof err !== 'object') throw new ERR_INVALID_ARG_TYPE('err', 'Object', err); - return JSONParse(nr.getReport(err.stack)); + return JSONParse(nr.getReport(err)); }, get directory() { return nr.getDirectory(); diff --git a/src/node_errors.cc b/src/node_errors.cc index 576db539358f35..6961e748c06ad9 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -418,7 +418,7 @@ void OnFatalError(const char* location, const char* message) { if (report_on_fatalerror) { report::TriggerNodeReport( - isolate, env, message, "FatalError", "", Local()); + isolate, env, message, "FatalError", "", Local()); } fflush(stderr); diff --git a/src/node_report.cc b/src/node_report.cc index 62b3d7abd1057b..c93e03afe63918 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -37,11 +37,18 @@ using node::Mutex; using node::NativeSymbolDebuggingContext; using node::TIME_TYPE; using node::worker::Worker; +using v8::Array; +using v8::Context; using v8::HeapSpaceStatistics; using v8::HeapStatistics; using v8::Isolate; using v8::Local; +using v8::Number; +using v8::Object; +using v8::StackTrace; using v8::String; +using v8::TryCatch; +using v8::Value; using v8::V8; namespace per_process = node::per_process; @@ -53,13 +60,16 @@ static void WriteNodeReport(Isolate* isolate, const char* trigger, const std::string& filename, std::ostream& out, - Local stackstr, + Local error, bool compact); static void PrintVersionInformation(JSONWriter* writer); -static void PrintJavaScriptStack(JSONWriter* writer, - Isolate* isolate, - Local stackstr, - const char* trigger); +static void PrintJavaScriptErrorStack(JSONWriter* writer, + Isolate* isolate, + Local error, + const char* trigger); +static void PrintJavaScriptErrorProperties(JSONWriter* writer, + Isolate* isolate, + Local error); static void PrintNativeStack(JSONWriter* writer); static void PrintResourceUsage(JSONWriter* writer); static void PrintGCStatistics(JSONWriter* writer, Isolate* isolate); @@ -76,7 +86,7 @@ std::string TriggerNodeReport(Isolate* isolate, const char* message, const char* trigger, const std::string& name, - Local stackstr) { + Local error) { std::string filename; // Determine the required report filename. In order of priority: @@ -142,7 +152,7 @@ std::string TriggerNodeReport(Isolate* isolate, compact = per_process::cli_options->report_compact; } WriteNodeReport(isolate, env, message, trigger, filename, *outstream, - stackstr, compact); + error, compact); // Do not close stdout/stderr, only close files we opened. if (outfile.is_open()) { @@ -161,9 +171,9 @@ void GetNodeReport(Isolate* isolate, Environment* env, const char* message, const char* trigger, - Local stackstr, + Local error, std::ostream& out) { - WriteNodeReport(isolate, env, message, trigger, "", out, stackstr, false); + WriteNodeReport(isolate, env, message, trigger, "", out, error, false); } // Internal function to coordinate and write the various @@ -174,7 +184,7 @@ static void WriteNodeReport(Isolate* isolate, const char* trigger, const std::string& filename, std::ostream& out, - Local stackstr, + Local error, bool compact) { // Obtain the current time and the pid. TIME_TYPE tm_struct; @@ -259,8 +269,13 @@ static void WriteNodeReport(Isolate* isolate, PrintVersionInformation(&writer); writer.json_objectend(); - // Report summary JavaScript stack backtrace - PrintJavaScriptStack(&writer, isolate, stackstr, trigger); + writer.json_objectstart("javascriptStack"); + // Report summary JavaScript error stack backtrace + PrintJavaScriptErrorStack(&writer, isolate, error, trigger); + + // Report summary JavaScript error properties backtrace + PrintJavaScriptErrorProperties(&writer, isolate, error); + writer.json_objectend(); // the end of 'javascriptStack' // Report native stack backtrace PrintNativeStack(&writer); @@ -301,7 +316,7 @@ static void WriteNodeReport(Isolate* isolate, env, "Worker thread subreport", trigger, - Local(), + Local(), os); Mutex::ScopedLock lock(workers_mutex); @@ -455,18 +470,56 @@ static void PrintNetworkInterfaceInfo(JSONWriter* writer) { } } +static void PrintJavaScriptErrorProperties(JSONWriter* writer, + Isolate* isolate, + Local error) { + writer->json_objectstart("errorProperties"); + if (!error.IsEmpty()) { + TryCatch try_catch(isolate); + Local context = error->GetIsolate()->GetCurrentContext(); + Local keys; + if (!error->GetOwnPropertyNames(context).ToLocal(&keys)) { + return writer->json_objectend(); // the end of 'errorProperties' + } + uint32_t keys_length = keys->Length(); + for (uint32_t i = 0; i < keys_length; i++) { + Local key; + if (!keys->Get(context, i).ToLocal(&key) || !key->IsString()) { + continue; + } + Local value; + Local value_string; + if (!error->Get(context, key).ToLocal(&value) || + !value->ToString(context).ToLocal(&value_string)) { + continue; + } + String::Utf8Value k(isolate, key); + if (!strcmp(*k, "stack") || !strcmp(*k, "message")) continue; + String::Utf8Value v(isolate, value_string); + writer->json_keyvalue(std::string(*k, k.length()), + std::string(*v, v.length())); + } + } + writer->json_objectend(); // the end of 'errorProperties' +} + // Report the JavaScript stack. -static void PrintJavaScriptStack(JSONWriter* writer, +static void PrintJavaScriptErrorStack(JSONWriter* writer, Isolate* isolate, - Local stackstr, + Local error, const char* trigger) { - writer->json_objectstart("javascriptStack"); - - std::string ss; + Local stackstr; + std::string ss = ""; + TryCatch try_catch(isolate); if ((!strcmp(trigger, "FatalError")) || (!strcmp(trigger, "Signal"))) { ss = "No stack.\nUnavailable.\n"; - } else { + } else if (!error.IsEmpty() && + error + ->Get(isolate->GetCurrentContext(), + node::FIXED_ONE_BYTE_STRING(isolate, + "stack")) + .ToLocal(&stackstr)) { String::Utf8Value sv(isolate, stackstr); ss = std::string(*sv, sv.length()); } @@ -490,7 +543,6 @@ static void PrintJavaScriptStack(JSONWriter* writer, } writer->json_arrayend(); } - writer->json_objectend(); } // Report a native stack backtrace diff --git a/src/node_report.h b/src/node_report.h index 98490784dd6e70..f4992f220221ed 100644 --- a/src/node_report.h +++ b/src/node_report.h @@ -20,12 +20,12 @@ std::string TriggerNodeReport(v8::Isolate* isolate, const char* message, const char* trigger, const std::string& name, - v8::Local stackstr); + v8::Local error); void GetNodeReport(v8::Isolate* isolate, node::Environment* env, const char* message, const char* trigger, - v8::Local stackstr, + v8::Local error, std::ostream& out); // Function declarations - utility functions in src/node_report_utils.cc diff --git a/src/node_report_module.cc b/src/node_report_module.cc index 5afc0cfe104fe6..d9dad28221a5cf 100644 --- a/src/node_report_module.cc +++ b/src/node_report_module.cc @@ -32,18 +32,21 @@ void WriteReport(const FunctionCallbackInfo& info) { Isolate* isolate = env->isolate(); HandleScope scope(isolate); std::string filename; - Local stackstr; + Local error; CHECK_EQ(info.Length(), 4); String::Utf8Value message(isolate, info[0].As()); String::Utf8Value trigger(isolate, info[1].As()); - stackstr = info[3].As(); if (info[2]->IsString()) filename = *String::Utf8Value(isolate, info[2]); + if (!info[3].IsEmpty() && info[3]->IsObject()) + error = info[3].As(); + else + error = Local(); filename = TriggerNodeReport( - isolate, env, *message, *trigger, filename, stackstr); + isolate, env, *message, *trigger, filename, error); // Return value is the report filename info.GetReturnValue().Set( String::NewFromUtf8(isolate, filename.c_str(), v8::NewStringType::kNormal) @@ -55,10 +58,17 @@ void GetReport(const FunctionCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); Isolate* isolate = env->isolate(); HandleScope scope(isolate); + Local error; std::ostringstream out; + CHECK_EQ(info.Length(), 1); + if (!info[0].IsEmpty() && info[0]->IsObject()) + error = info[0].As(); + else + error = Local(); + GetNodeReport( - isolate, env, "JavaScript API", __func__, info[0].As(), out); + isolate, env, "JavaScript API", __func__, error, out); // Return value is the contents of a report as a string. info.GetReturnValue().Set(String::NewFromUtf8(isolate, diff --git a/test/common/report.js b/test/common/report.js index 3b6b46eeec0f8d..f49493b443edea 100644 --- a/test/common/report.js +++ b/test/common/report.js @@ -24,16 +24,16 @@ function findReports(pid, dir) { return results; } -function validate(filepath) { +function validate(filepath, fields) { const report = fs.readFileSync(filepath, 'utf8'); if (process.report.compact) { const end = report.indexOf('\n'); assert.strictEqual(end, report.length - 1); } - validateContent(JSON.parse(report)); + validateContent(JSON.parse(report), fields); } -function validateContent(report) { +function validateContent(report, fields = []) { if (typeof report === 'string') { try { report = JSON.parse(report); @@ -43,7 +43,7 @@ function validateContent(report) { } } try { - _validateContent(report); + _validateContent(report, fields); } catch (err) { try { err.stack += util.format('\n------\nFailing Report:\n%O', report); @@ -52,7 +52,7 @@ function validateContent(report) { } } -function _validateContent(report) { +function _validateContent(report, fields = []) { const isWindows = process.platform === 'win32'; // Verify that all sections are present as own properties of the report. @@ -71,6 +71,26 @@ function _validateContent(report) { assert(typeof report[section] === 'object' && report[section] !== null); }); + fields.forEach((field) => { + function checkLoop(actual, rest, expect) { + actual = actual[rest.shift()]; + if (rest.length === 0 && actual !== undefined) { + assert.strictEqual(actual, expect); + } else { + assert(actual); + checkLoop(actual, rest, expect); + } + } + let actual, expect; + if (Array.isArray(field)) { + [actual, expect] = field; + } else { + actual = field; + expect = undefined; + } + checkLoop(report, actual.split('.'), expect); + }); + // Verify the format of the header section. const header = report.header; const headerFields = ['event', 'trigger', 'filename', 'dumpEventTime', @@ -144,7 +164,10 @@ function _validateContent(report) { assert.strictEqual(header.host, os.hostname()); // Verify the format of the javascriptStack section. - checkForUnknownFields(report.javascriptStack, ['message', 'stack']); + checkForUnknownFields(report.javascriptStack, + ['message', 'stack', 'errorProperties']); + assert.strictEqual(typeof report.javascriptStack.errorProperties, + 'object'); assert.strictEqual(typeof report.javascriptStack.message, 'string'); if (report.javascriptStack.stack !== undefined) { assert(Array.isArray(report.javascriptStack.stack)); @@ -262,7 +285,7 @@ function _validateContent(report) { // Verify the format of the workers section. assert(Array.isArray(report.workers)); - report.workers.forEach(_validateContent); + report.workers.forEach((worker) => _validateContent(worker)); } function checkForUnknownFields(actual, expected) { diff --git a/test/report/test-report-getreport.js b/test/report/test-report-getreport.js index 4c68b4ecbb5e48..9f1200002bbab4 100644 --- a/test/report/test-report-getreport.js +++ b/test/report/test-report-getreport.js @@ -23,6 +23,13 @@ const helper = require('../common/report'); assert.deepStrictEqual(helper.findReports(process.pid, process.cwd()), []); } +{ + const error = new Error(); + error.foo = 'goo'; + helper.validateContent(process.report.getReport(error), + [['javascriptStack.errorProperties.foo', 'goo']]); +} + // Test with an invalid error argument. [null, 1, Symbol(), function() {}, 'foo'].forEach((error) => { assert.throws(() => { diff --git a/test/report/test-report-writereport.js b/test/report/test-report-writereport.js index 50514c68373d15..971afd84c22281 100644 --- a/test/report/test-report-writereport.js +++ b/test/report/test-report-writereport.js @@ -15,7 +15,7 @@ process.report.directory = tmpdir.path; function validate() { const reports = helper.findReports(process.pid, tmpdir.path); assert.strictEqual(reports.length, 1); - helper.validate(reports[0]); + helper.validate(reports[0], arguments[0]); fs.unlinkSync(reports[0]); return reports[0]; } @@ -40,6 +40,13 @@ function validate() { validate(); } +{ + const error = new Error(); + error.foo = 'goo'; + process.report.writeReport(error); + validate([['javascriptStack.errorProperties.foo', 'goo']]); +} + { // Test with a file argument. const file = process.report.writeReport('custom-name-1.json');