report: print javascript stack on fatal error

Try to print JavaScript stack on fatal error. OOMError needs to be distinguished from fatal error since no new handle can be created at that time. PR-URL: #44242 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
nodejs · Oct 3, 2022 · 53364fc · 53364fc
1 parent f3fd4e1
commit 53364fc
Show file tree

Hide file tree

Showing 8 changed files with 196 additions and 8 deletions.
diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -237,6 +237,7 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
   auto* fatal_error_cb = s.fatal_error_callback ?
       s.fatal_error_callback : OnFatalError;
   isolate->SetFatalErrorHandler(fatal_error_cb);
+  isolate->SetOOMErrorHandler(OOMErrorHandler);
 
   if ((s.flags & SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK) == 0) {
     auto* prepare_stack_trace_cb = s.prepare_stack_trace_callback ?

diff --git a/src/node_errors.cc b/src/node_errors.cc
@@ -459,6 +459,36 @@ void OnFatalError(const char* location, const char* message) {
   ABORT();
 }
 
+void OOMErrorHandler(const char* location, bool is_heap_oom) {
+  const char* message =
+      is_heap_oom ? "Allocation failed - JavaScript heap out of memory"
+                  : "Allocation failed - process out of memory";
+  if (location) {
+    FPrintF(stderr, "FATAL ERROR: %s %s\n", location, message);
+  } else {
+    FPrintF(stderr, "FATAL ERROR: %s\n", message);
+  }
+
+  Isolate* isolate = Isolate::TryGetCurrent();
+  Environment* env = nullptr;
+  if (isolate != nullptr) {
+    env = Environment::GetCurrent(isolate);
+  }
+  bool report_on_fatalerror;
+  {
+    Mutex::ScopedLock lock(node::per_process::cli_options_mutex);
+    report_on_fatalerror = per_process::cli_options->report_on_fatalerror;
+  }
+
+  if (report_on_fatalerror) {
+    report::TriggerNodeReport(
+        isolate, env, message, "OOMError", "", Local<Object>());
+  }
+
+  fflush(stderr);
+  ABORT();
+}
+
 v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings(
     v8::Local<v8::Context> context,
     v8::Local<v8::Value> source,

diff --git a/src/node_errors.h b/src/node_errors.h
@@ -21,6 +21,7 @@ void AppendExceptionLine(Environment* env,
 
 [[noreturn]] void FatalError(const char* location, const char* message);
 void OnFatalError(const char* location, const char* message);
+void OOMErrorHandler(const char* location, bool is_heap_oom);
 
 // Helpers to construct errors similar to the ones provided by
 // lib/internal/errors.js.

diff --git a/src/node_report.cc b/src/node_report.cc
@@ -26,6 +26,7 @@
 constexpr int NODE_REPORT_VERSION = 2;
 constexpr int NANOS_PER_SEC = 1000 * 1000 * 1000;
 constexpr double SEC_PER_MICROS = 1e-6;
+constexpr int MAX_FRAME_COUNT = 10;
 
 namespace node {
 namespace report {
@@ -43,6 +44,10 @@ using v8::Maybe;
 using v8::MaybeLocal;
 using v8::Nothing;
 using v8::Object;
+using v8::RegisterState;
+using v8::SampleInfo;
+using v8::StackFrame;
+using v8::StackTrace;
 using v8::String;
 using v8::TryCatch;
 using v8::V8;
@@ -62,6 +67,9 @@ static void PrintJavaScriptErrorStack(JSONWriter* writer,
                                       Isolate* isolate,
                                       Local<Value> error,
                                       const char* trigger);
+static void PrintJavaScriptStack(JSONWriter* writer,
+                                 Isolate* isolate,
+                                 const char* trigger);
 static void PrintJavaScriptErrorProperties(JSONWriter* writer,
                                            Isolate* isolate,
                                            Local<Value> error);
@@ -269,8 +277,6 @@ static void WriteNodeReport(Isolate* isolate,
     // 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 V8 Heap and Garbage Collector information
@@ -317,7 +323,7 @@ static void WriteNodeReport(Isolate* isolate,
                       env,
                       "Worker thread subreport",
                       trigger,
-                      Local<Object>(),
+                      Local<Value>(),
                       os);
 
         Mutex::ScopedLock lock(workers_mutex);
@@ -534,19 +540,80 @@ static Maybe<std::string> ErrorToString(Isolate* isolate,
   return Just<>(std::string(*sv, sv.length()));
 }
 
+static void PrintEmptyJavaScriptStack(JSONWriter* writer) {
+  writer->json_keyvalue("message", "No stack.");
+  writer->json_arraystart("stack");
+  writer->json_element("Unavailable.");
+  writer->json_arrayend();
+
+  writer->json_objectstart("errorProperties");
+  writer->json_objectend();
+}
+
+// Do our best to report the JavaScript stack without calling into JavaScript.
+static void PrintJavaScriptStack(JSONWriter* writer,
+                                 Isolate* isolate,
+                                 const char* trigger) {
+  // Can not capture the stacktrace when the isolate is in a OOM state.
+  if (!strcmp(trigger, "OOMError")) {
+    PrintEmptyJavaScriptStack(writer);
+    return;
+  }
+
+  HandleScope scope(isolate);
+  RegisterState state;
+  state.pc = nullptr;
+  state.fp = &state;
+  state.sp = &state;
+
+  // in-out params
+  SampleInfo info;
+  void* samples[MAX_FRAME_COUNT];
+  isolate->GetStackSample(state, samples, MAX_FRAME_COUNT, &info);
+
+  Local<StackTrace> stack = StackTrace::CurrentStackTrace(
+      isolate, MAX_FRAME_COUNT, StackTrace::kDetailed);
+
+  if (stack->GetFrameCount() == 0) {
+    PrintEmptyJavaScriptStack(writer);
+    return;
+  }
+
+  writer->json_keyvalue("message", trigger);
+  writer->json_arraystart("stack");
+  for (int i = 0; i < stack->GetFrameCount(); i++) {
+    Local<StackFrame> frame = stack->GetFrame(isolate, i);
+
+    Utf8Value function_name(isolate, frame->GetFunctionName());
+    Utf8Value script_name(isolate, frame->GetScriptName());
+    const int line_number = frame->GetLineNumber();
+    const int column = frame->GetColumn();
+
+    std::string stack_line = SPrintF(
+        "at %s (%s:%d:%d)", *function_name, *script_name, line_number, column);
+    writer->json_element(stack_line);
+  }
+  writer->json_arrayend();
+  writer->json_objectstart("errorProperties");
+  writer->json_objectend();
+}
+
 // Report the JavaScript stack.
 static void PrintJavaScriptErrorStack(JSONWriter* writer,
                                       Isolate* isolate,
                                       Local<Value> error,
                                       const char* trigger) {
+  if (error.IsEmpty()) {
+    return PrintJavaScriptStack(writer, isolate, trigger);
+  }
+
   TryCatch try_catch(isolate);
   HandleScope scope(isolate);
   Local<Context> context = isolate->GetCurrentContext();
   std::string ss = "";
-  if ((!strcmp(trigger, "FatalError")) ||
-      (!strcmp(trigger, "Signal")) ||
-      (!ErrorToString(isolate, context, error).To(&ss))) {
-    ss = "No stack.\nUnavailable.\n";
+  if (!ErrorToString(isolate, context, error).To(&ss)) {
+    PrintEmptyJavaScriptStack(writer);
+    return;
   }
 
   int line = ss.find('\n');
@@ -569,6 +636,9 @@ static void PrintJavaScriptErrorStack(JSONWriter* writer,
     }
     writer->json_arrayend();
   }
+
+  // Report summary JavaScript error properties backtrace
+  PrintJavaScriptErrorProperties(writer, isolate, error);
 }
 
 // Report a native stack backtrace

diff --git a/test/addons/report-fatalerror/binding.cc b/test/addons/report-fatalerror/binding.cc
@@ -0,0 +1,23 @@
+#include <node.h>
+#include <v8.h>
+
+using v8::FunctionCallbackInfo;
+using v8::Isolate;
+using v8::Local;
+using v8::MaybeLocal;
+using v8::Object;
+using v8::Value;
+
+void TriggerFatalError(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+
+  // Trigger a v8 ApiCheck failure.
+  MaybeLocal<Value> value;
+  value.ToLocalChecked();
+}
+
+void init(Local<Object> exports) {
+  NODE_SET_METHOD(exports, "triggerFatalError", TriggerFatalError);
+}
+
+NODE_MODULE(NODE_GYP_MODULE_NAME, init)
diff --git a/test/addons/report-fatalerror/binding.gyp b/test/addons/report-fatalerror/binding.gyp
@@ -0,0 +1,9 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'sources': [ 'binding.cc' ],
+      'includes': ['../common.gypi'],
+    }
+  ]
+}
diff --git a/test/addons/report-fatalerror/test.js b/test/addons/report-fatalerror/test.js
@@ -0,0 +1,52 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const path = require('path');
+const spawnSync = require('child_process').spawnSync;
+const helper = require('../../common/report.js');
+const tmpdir = require('../../common/tmpdir');
+
+const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);
+
+if (process.argv[2] === 'child') {
+  (function childMain() {
+    const addon = require(binding);
+    addon.triggerFatalError();
+  })();
+  return;
+}
+
+const ARGS = [
+  __filename,
+  'child',
+];
+
+{
+  // Verify that --report-on-fatalerror is respected when set.
+  tmpdir.refresh();
+  const args = ['--report-on-fatalerror', ...ARGS];
+  const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
+  assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
+
+  const reports = helper.findReports(child.pid, tmpdir.path);
+  assert.strictEqual(reports.length, 1);
+
+  const report = reports[0];
+  helper.validate(report);
+
+  const content = require(report);
+  assert.strictEqual(content.header.trigger, 'FatalError');
+
+  // Check that the javascript stack is present.
+  assert.strictEqual(content.javascriptStack.stack.findIndex((frame) => frame.match('childMain')), 0);
+}
+
+{
+  // Verify that --report-on-fatalerror is respected when not set.
+  const args = ARGS;
+  const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
+  assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
+  const reports = helper.findReports(child.pid, tmpdir.path);
+  assert.strictEqual(reports.length, 0);
+}
diff --git a/test/report/test-report-fatal-error.js → ...report/test-report-fatalerror-oomerror.js b/test/report/test-report-fatal-error.js → ...report/test-report-fatalerror-oomerror.js
@@ -44,10 +44,12 @@ const ARGS = [
   const report = reports[0];
   helper.validate(report);
 
+  const content = require(report);
   // Errors occur in a context where env is not available, so thread ID is
   // unknown. Assert this, to verify that the underlying env-less situation is
   // actually reached.
-  assert.strictEqual(require(report).header.threadId, null);
+  assert.strictEqual(content.header.threadId, null);
+  assert.strictEqual(content.header.trigger, 'OOMError');
 }
 
 {