From 55745a1817152629ad492e9b8dd1e3bca49686b7 Mon Sep 17 00:00:00 2001 From: legendecas Date: Thu, 1 Apr 2021 01:31:34 +0800 Subject: [PATCH] src: print arbitrary javascript exception value in node report PR-URL: https://github.com/nodejs/node/pull/38009 Reviewed-By: Michael Dawson Reviewed-By: James M Snell Reviewed-By: Gireesh Punathil --- lib/internal/process/execution.js | 11 ++- src/node_report.cc | 80 +++++++++++++------ src/node_report.h | 4 +- src/node_report_module.cc | 8 +- ...st-report-uncaught-exception-primitives.js | 25 ++++++ .../test-report-uncaught-exception-symbols.js | 25 ++++++ 6 files changed, 118 insertions(+), 35 deletions(-) create mode 100644 test/report/test-report-uncaught-exception-primitives.js create mode 100644 test/report/test-report-uncaught-exception-symbols.js diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index e370770643ca6f..e2d9898012d2d7 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -152,10 +152,13 @@ function createOnGlobalUncaughtException() { try { const report = internalBinding('report'); if (report != null && report.shouldReportOnUncaughtException()) { - report.writeReport(er ? er.message : 'Exception', - 'Exception', - null, - er ? er : {}); + report.writeReport( + typeof er?.message === 'string' ? + er.message : + 'Exception', + 'Exception', + null, + er ? er : {}); } } catch {} // Ignore the exception. Diagnostic reporting is unavailable. } diff --git a/src/node_report.cc b/src/node_report.cc index 13f87b1e52e5d6..0144d22c17d13b 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -39,10 +39,15 @@ using node::TIME_TYPE; using node::worker::Worker; using v8::Array; using v8::Context; +using v8::HandleScope; using v8::HeapSpaceStatistics; using v8::HeapStatistics; using v8::Isolate; +using v8::Just; using v8::Local; +using v8::Maybe; +using v8::MaybeLocal; +using v8::Nothing; using v8::Object; using v8::String; using v8::TryCatch; @@ -58,16 +63,16 @@ static void WriteNodeReport(Isolate* isolate, const char* trigger, const std::string& filename, std::ostream& out, - Local error, + Local error, bool compact); static void PrintVersionInformation(JSONWriter* writer); static void PrintJavaScriptErrorStack(JSONWriter* writer, Isolate* isolate, - Local error, + Local error, const char* trigger); static void PrintJavaScriptErrorProperties(JSONWriter* writer, Isolate* isolate, - Local error); + Local error); static void PrintNativeStack(JSONWriter* writer); static void PrintResourceUsage(JSONWriter* writer); static void PrintGCStatistics(JSONWriter* writer, Isolate* isolate); @@ -84,7 +89,7 @@ std::string TriggerNodeReport(Isolate* isolate, const char* message, const char* trigger, const std::string& name, - Local error) { + Local error) { std::string filename; // Determine the required report filename. In order of priority: @@ -169,7 +174,7 @@ void GetNodeReport(Isolate* isolate, Environment* env, const char* message, const char* trigger, - Local error, + Local error, std::ostream& out) { WriteNodeReport(isolate, env, message, trigger, "", out, error, false); } @@ -182,7 +187,7 @@ static void WriteNodeReport(Isolate* isolate, const char* trigger, const std::string& filename, std::ostream& out, - Local error, + Local error, bool compact) { // Obtain the current time and the pid. TIME_TYPE tm_struct; @@ -474,13 +479,14 @@ static void PrintNetworkInterfaceInfo(JSONWriter* writer) { static void PrintJavaScriptErrorProperties(JSONWriter* writer, Isolate* isolate, - Local error) { + Local error) { writer->json_objectstart("errorProperties"); - if (!error.IsEmpty()) { + if (!error.IsEmpty() && error->IsObject()) { TryCatch try_catch(isolate); - Local context = error->GetIsolate()->GetCurrentContext(); + Local error_obj = error.As(); + Local context = error_obj->GetIsolate()->GetCurrentContext(); Local keys; - if (!error->GetOwnPropertyNames(context).ToLocal(&keys)) { + if (!error_obj->GetOwnPropertyNames(context).ToLocal(&keys)) { return writer->json_objectend(); // the end of 'errorProperties' } uint32_t keys_length = keys->Length(); @@ -491,7 +497,7 @@ static void PrintJavaScriptErrorProperties(JSONWriter* writer, } Local value; Local value_string; - if (!error->Get(context, key).ToLocal(&value) || + if (!error_obj->Get(context, key).ToLocal(&value) || !value->ToString(context).ToLocal(&value_string)) { continue; } @@ -505,26 +511,50 @@ static void PrintJavaScriptErrorProperties(JSONWriter* writer, writer->json_objectend(); // the end of 'errorProperties' } +static Maybe ErrorToString(Isolate* isolate, + Local context, + Local error) { + if (error.IsEmpty()) { + return Nothing(); + } + + MaybeLocal maybe_str; + // `ToString` is not available to Symbols. + if (error->IsSymbol()) { + maybe_str = error.As()->ToDetailString(context); + } else if (!error->IsObject()) { + maybe_str = error->ToString(context); + } else if (error->IsObject()) { + MaybeLocal stack = error.As()->Get( + context, node::FIXED_ONE_BYTE_STRING(isolate, "stack")); + if (!stack.IsEmpty() && stack.ToLocalChecked()->IsString()) { + maybe_str = stack.ToLocalChecked().As(); + } + } + + Local js_str; + if (!maybe_str.ToLocal(&js_str)) { + return Nothing(); + } + String::Utf8Value sv(isolate, js_str); + return Just<>(std::string(*sv, sv.length())); +} + // Report the JavaScript stack. static void PrintJavaScriptErrorStack(JSONWriter* writer, - Isolate* isolate, - Local error, - const char* trigger) { - Local stackstr; - std::string ss = ""; + Isolate* isolate, + Local error, + const char* trigger) { TryCatch try_catch(isolate); + HandleScope scope(isolate); + Local context = isolate->GetCurrentContext(); + std::string ss = ""; if ((!strcmp(trigger, "FatalError")) || - (!strcmp(trigger, "Signal"))) { + (!strcmp(trigger, "Signal")) || + (!ErrorToString(isolate, context, error).To(&ss))) { ss = "No stack.\nUnavailable.\n"; - } 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()); } + int line = ss.find('\n'); if (line == -1) { writer->json_keyvalue("message", ss); diff --git a/src/node_report.h b/src/node_report.h index f4992f220221ed..a8292eb2dd477d 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 error); + v8::Local error); void GetNodeReport(v8::Isolate* isolate, node::Environment* env, const char* message, const char* trigger, - v8::Local error, + 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 97c6bea3ad5ec5..190755a85b2369 100644 --- a/src/node_report_module.cc +++ b/src/node_report_module.cc @@ -32,7 +32,7 @@ void WriteReport(const FunctionCallbackInfo& info) { Isolate* isolate = env->isolate(); HandleScope scope(isolate); std::string filename; - Local error; + Local error; CHECK_EQ(info.Length(), 4); String::Utf8Value message(isolate, info[0].As()); @@ -40,10 +40,10 @@ void WriteReport(const FunctionCallbackInfo& info) { if (info[2]->IsString()) filename = *String::Utf8Value(isolate, info[2]); - if (!info[3].IsEmpty() && info[3]->IsObject()) - error = info[3].As(); + if (!info[3].IsEmpty()) + error = info[3]; else - error = Local(); + error = Local(); filename = TriggerNodeReport( isolate, env, *message, *trigger, filename, error); diff --git a/test/report/test-report-uncaught-exception-primitives.js b/test/report/test-report-uncaught-exception-primitives.js new file mode 100644 index 00000000000000..75a05f335cf2e2 --- /dev/null +++ b/test/report/test-report-uncaught-exception-primitives.js @@ -0,0 +1,25 @@ +// Flags: --report-uncaught-exception +'use strict'; +// Test producing a report on uncaught exception. +const common = require('../common'); +const assert = require('assert'); +const helper = require('../common/report'); +const tmpdir = require('../common/tmpdir'); + +const exception = 1; + +tmpdir.refresh(); +process.report.directory = tmpdir.path; + +process.on('uncaughtException', common.mustCall((err) => { + assert.strictEqual(err, exception); + const reports = helper.findReports(process.pid, tmpdir.path); + assert.strictEqual(reports.length, 1); + console.log(reports[0]); + helper.validate(reports[0], [ + ['header.event', 'Exception'], + ['javascriptStack.message', `${exception}`], + ]); +})); + +throw exception; diff --git a/test/report/test-report-uncaught-exception-symbols.js b/test/report/test-report-uncaught-exception-symbols.js new file mode 100644 index 00000000000000..5997d0e0898ac0 --- /dev/null +++ b/test/report/test-report-uncaught-exception-symbols.js @@ -0,0 +1,25 @@ +// Flags: --report-uncaught-exception +'use strict'; +// Test producing a report on uncaught exception. +const common = require('../common'); +const assert = require('assert'); +const helper = require('../common/report'); +const tmpdir = require('../common/tmpdir'); + +const exception = Symbol('foobar'); + +tmpdir.refresh(); +process.report.directory = tmpdir.path; + +process.on('uncaughtException', common.mustCall((err) => { + assert.strictEqual(err, exception); + const reports = helper.findReports(process.pid, tmpdir.path); + assert.strictEqual(reports.length, 1); + console.log(reports[0]); + helper.validate(reports[0], [ + ['header.event', 'Exception'], + ['javascriptStack.message', 'Symbol(foobar)'], + ]); +})); + +throw exception;