src: print arbitrary javascript exception value in node report

PR-URL: nodejs#38009 Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
sxa · Apr 26, 2021 · 55745a1 · 55745a1
1 parent 8a973fd
commit 55745a1
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 35 deletions.
diff --git 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
@@ -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<Object> error,
+                            Local<Value> error,
                             bool compact);
 static void PrintVersionInformation(JSONWriter* writer);
 static void PrintJavaScriptErrorStack(JSONWriter* writer,
                                       Isolate* isolate,
-                                      Local<Object> error,
+                                      Local<Value> error,
                                       const char* trigger);
 static void PrintJavaScriptErrorProperties(JSONWriter* writer,
                                            Isolate* isolate,
-                                           Local<Object> error);
+                                           Local<Value> 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<Object> error) {
+                              Local<Value> 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<Object> error,
+                   Local<Value> 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<Object> error,
+                            Local<Value> 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<Object> error) {
+                                           Local<Value> error) {
   writer->json_objectstart("errorProperties");
-  if (!error.IsEmpty()) {
+  if (!error.IsEmpty() && error->IsObject()) {
     TryCatch try_catch(isolate);
-    Local<Context> context = error->GetIsolate()->GetCurrentContext();
+    Local<Object> error_obj = error.As<Object>();
+    Local<Context> context = error_obj->GetIsolate()->GetCurrentContext();
     Local<Array> 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> value;
       Local<String> 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<std::string> ErrorToString(Isolate* isolate,
+                                        Local<Context> context,
+                                        Local<Value> error) {
+  if (error.IsEmpty()) {
+    return Nothing<std::string>();
+  }
+
+  MaybeLocal<String> maybe_str;
+  // `ToString` is not available to Symbols.
+  if (error->IsSymbol()) {
+    maybe_str = error.As<v8::Symbol>()->ToDetailString(context);
+  } else if (!error->IsObject()) {
+    maybe_str = error->ToString(context);
+  } else if (error->IsObject()) {
+    MaybeLocal<Value> stack = error.As<Object>()->Get(
+        context, node::FIXED_ONE_BYTE_STRING(isolate, "stack"));
+    if (!stack.IsEmpty() && stack.ToLocalChecked()->IsString()) {
+      maybe_str = stack.ToLocalChecked().As<String>();
+    }
+  }
+
+  Local<String> js_str;
+  if (!maybe_str.ToLocal(&js_str)) {
+    return Nothing<std::string>();
+  }
+  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<Object> error,
-                                 const char* trigger) {
-  Local<Value> stackstr;
-  std::string ss = "";
+                                      Isolate* isolate,
+                                      Local<Value> error,
+                                      const char* trigger) {
   TryCatch try_catch(isolate);
+  HandleScope scope(isolate);
+  Local<Context> 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
@@ -20,12 +20,12 @@ std::string TriggerNodeReport(v8::Isolate* isolate,
                               const char* message,
                               const char* trigger,
                               const std::string& name,
-                              v8::Local<v8::Object> error);
+                              v8::Local<v8::Value> error);
 void GetNodeReport(v8::Isolate* isolate,
                    node::Environment* env,
                    const char* message,
                    const char* trigger,
-                   v8::Local<v8::Object> error,
+                   v8::Local<v8::Value> 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
@@ -32,18 +32,18 @@ void WriteReport(const FunctionCallbackInfo<Value>& info) {
   Isolate* isolate = env->isolate();
   HandleScope scope(isolate);
   std::string filename;
-  Local<Object> error;
+  Local<Value> error;
 
   CHECK_EQ(info.Length(), 4);
   String::Utf8Value message(isolate, info[0].As<String>());
   String::Utf8Value trigger(isolate, info[1].As<String>());
 
   if (info[2]->IsString())
     filename = *String::Utf8Value(isolate, info[2]);
-  if (!info[3].IsEmpty() && info[3]->IsObject())
-    error = info[3].As<Object>();
+  if (!info[3].IsEmpty())
+    error = info[3];
   else
-    error = Local<Object>();
+    error = Local<Value>();
 
   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
@@ -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
@@ -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;