report: fix missing section javascriptHeap on OOMError

src/node_errors.cc

@@ -513,6 +513,11 @@ void OOMErrorHandler(const char* location, bool is_heap_oom) {
   }
 
   if (report_on_fatalerror) {
+    // Trigger report with the isolate. Environment::GetCurrent may return
+    // nullptr here:
+    // - If the OOM is reported by a young generation space allocation,
+    //   Isolate::GetCurrentContext returns an empty handle.
+    // - Otherwise, Isolate::GetCurrentContext returns a non-empty handle.
     TriggerNodeReport(isolate, message, "OOMError", "", Local<Object>());
   }
 

src/node_report.cc

@@ -470,8 +470,12 @@ static void PrintJavaScriptStack(JSONWriter* writer,
   void* samples[MAX_FRAME_COUNT];
   isolate->GetStackSample(state, samples, MAX_FRAME_COUNT, &info);
 
+  constexpr StackTrace::StackTraceOptions stack_trace_options =
+      static_cast<StackTrace::StackTraceOptions>(
+          StackTrace::kDetailed |
+          StackTrace::kExposeFramesAcrossSecurityOrigins);
   Local<StackTrace> stack = StackTrace::CurrentStackTrace(
-      isolate, MAX_FRAME_COUNT, StackTrace::kDetailed);
+      isolate, MAX_FRAME_COUNT, stack_trace_options);
 
   if (stack->GetFrameCount() == 0) {
     PrintEmptyJavaScriptStack(writer);
@@ -784,21 +788,8 @@ static void PrintRelease(JSONWriter* writer) {
 
 }  // namespace report
 
-// External function to trigger a report, writing to file.
 std::string TriggerNodeReport(Isolate* isolate,
-                              const char* message,
-                              const char* trigger,
-                              const std::string& name,
-                              Local<Value> error) {
-  Environment* env = nullptr;
-  if (isolate != nullptr) {
-    env = Environment::GetCurrent(isolate);
-  }
-  return TriggerNodeReport(env, message, trigger, name, error);
-}
-
-// External function to trigger a report, writing to file.
-std::string TriggerNodeReport(Environment* env,
+                              Environment* env,
                               const char* message,
                               const char* trigger,
                               const std::string& name,
@@ -868,10 +859,6 @@ std::string TriggerNodeReport(Environment* env,
     compact = per_process::cli_options->report_compact;
   }
 
-  Isolate* isolate = nullptr;
-  if (env != nullptr) {
-    isolate = env->isolate();
-  }
   report::WriteNodeReport(
       isolate, env, message, trigger, filename, *outstream, error, compact);
 
@@ -887,6 +874,33 @@ std::string TriggerNodeReport(Environment* env,
   return filename;
 }
 
+// External function to trigger a report, writing to file.
+std::string TriggerNodeReport(Isolate* isolate,
+                              const char* message,
+                              const char* trigger,
+                              const std::string& name,
+                              Local<Value> error) {
+  Environment* env = nullptr;
+  if (isolate != nullptr) {
+    env = Environment::GetCurrent(isolate);
+  }
+  return TriggerNodeReport(isolate, env, message, trigger, name, error);
+}
+
+// External function to trigger a report, writing to file.
+std::string TriggerNodeReport(Environment* env,
+                              const char* message,
+                              const char* trigger,
+                              const std::string& name,
+                              Local<Value> error) {
+  return TriggerNodeReport(env != nullptr ? env->isolate() : nullptr,
+                           env,
+                           message,
+                           trigger,
+                           name,
+                           error);
+}
+
 // External function to trigger a report, writing to a supplied stream.
 void GetNodeReport(Isolate* isolate,
                    const char* message,

test/addons/report-api/binding.cc

@@ -1,6 +1,7 @@
 #include <node.h>
 #include <v8.h>
 
+using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::Isolate;
 using v8::Local;
@@ -43,11 +44,37 @@ void TriggerReportNoEnv(const FunctionCallbackInfo<Value>& args) {
                           Local<Value>());
 }
 
+void TriggerReportNoContext(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Context> context = isolate->GetCurrentContext();
+  context->Exit();
+
+  if (isolate->GetCurrentContext().IsEmpty()) {
+    node::TriggerNodeReport(
+        isolate, "FooMessage", "BarTrigger", std::string(), Local<Value>());
+  }
+
+  // Restore current context to avoid crashing in Context::Scope in
+  // SpinEventLoop.
+  context->Enter();
+}
+
+void TriggerReportNewContext(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Context> context = Context::New(isolate);
+  Context::Scope context_scope(context);
+
+  node::TriggerNodeReport(
+      isolate, "FooMessage", "BarTrigger", std::string(), Local<Value>());
+}
+
 void init(Local<Object> exports) {
   NODE_SET_METHOD(exports, "triggerReport", TriggerReport);
   NODE_SET_METHOD(exports, "triggerReportNoIsolate", TriggerReportNoIsolate);
   NODE_SET_METHOD(exports, "triggerReportEnv", TriggerReportEnv);
   NODE_SET_METHOD(exports, "triggerReportNoEnv", TriggerReportNoEnv);
+  NODE_SET_METHOD(exports, "triggerReportNoContext", TriggerReportNoContext);
+  NODE_SET_METHOD(exports, "triggerReportNewContext", TriggerReportNewContext);
 }
 
 NODE_MODULE(NODE_GYP_MODULE_NAME, init)

test/addons/report-api/test.js

@@ -9,7 +9,7 @@ const tmpdir = require('../../common/tmpdir');
 const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);
 const addon = require(binding);
 
-function myAddonMain(method, hasJavaScriptFrames) {
+function myAddonMain(method, { hasIsolate, hasEnv }) {
   tmpdir.refresh();
   process.report.directory = tmpdir.path;
 
@@ -19,26 +19,35 @@ function myAddonMain(method, hasJavaScriptFrames) {
   assert.strictEqual(reports.length, 1);
 
   const report = reports[0];
-  helper.validate(report);
+  helper.validate(report, [
+    ['header.event', 'FooMessage'],
+    ['header.trigger', 'BarTrigger'],
+  ]);
 
   const content = require(report);
-  assert.strictEqual(content.header.event, 'FooMessage');
-  assert.strictEqual(content.header.trigger, 'BarTrigger');
 
   // Check that the javascript stack is present.
-  if (hasJavaScriptFrames) {
+  if (hasIsolate) {
     assert.strictEqual(content.javascriptStack.stack.findIndex((frame) => frame.match('myAddonMain')), 0);
   } else {
     assert.strictEqual(content.javascriptStack, undefined);
   }
+
+  if (hasEnv) {
+    assert.strictEqual(content.header.threadId, 0);
+  } else {
+    assert.strictEqual(content.header.threadId, null);
+  }
 }
 
 const methods = [
-  ['triggerReport', true],
-  ['triggerReportNoIsolate', false],
-  ['triggerReportEnv', true],
-  ['triggerReportNoEnv', false],
+  ['triggerReport', true, true],
+  ['triggerReportNoIsolate', false, false],
+  ['triggerReportEnv', true, true],
+  ['triggerReportNoEnv', false, false],
+  ['triggerReportNoContext', true, false],
+  ['triggerReportNewContext', true, false],
 ];
-for (const [method, hasJavaScriptFrames] of methods) {
-  myAddonMain(method, hasJavaScriptFrames);
+for (const [method, hasIsolate, hasEnv] of methods) {
+  myAddonMain(method, { hasIsolate, hasEnv });
 }

test/report/test-report-fatalerror-oomerror-compact.js

@@ -12,9 +12,13 @@ const fixtures = require('../common/fixtures');
 
 // Common args that will cause an out-of-memory error for child process.
 const ARGS = [
-  '--max-old-space-size=20',
+  '--max-heap-size=20',
   fixtures.path('report-oom'),
 ];
+const REPORT_FIELDS = [
+  ['header.trigger', 'OOMError'],
+  ['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
+];
 
 {
   // Verify that --report-compact is respected when set.
@@ -27,8 +31,8 @@ const ARGS = [
   assert.strictEqual(reports.length, 1);
 
   const report = reports[0];
-  helper.validate(report);
-  assert.strictEqual(require(report).header.threadId, null);
+  helper.validate(report, REPORT_FIELDS);
+
   // Subtract 1 because "xx\n".split("\n") => [ 'xx', '' ].
   const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1;
   assert.strictEqual(lines, 1);

test/report/test-report-fatalerror-oomerror-directory.js

@@ -12,9 +12,13 @@ const fixtures = require('../common/fixtures');
 
 // Common args that will cause an out-of-memory error for child process.
 const ARGS = [
-  '--max-old-space-size=20',
+  '--max-heap-size=20',
   fixtures.path('report-oom'),
 ];
+const REPORT_FIELDS = [
+  ['header.trigger', 'OOMError'],
+  ['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
+];
 
 {
   // Verify that --report-directory is respected when set.
@@ -29,8 +33,8 @@ const ARGS = [
   assert.strictEqual(reports.length, 1);
 
   const report = reports[0];
-  helper.validate(report);
-  assert.strictEqual(require(report).header.threadId, null);
+  helper.validate(report, REPORT_FIELDS);
+
   const lines = fs.readFileSync(report, 'utf8').split('\n').length - 1;
   assert(lines > 10);
 }

test/report/test-report-fatalerror-oomerror-filename.js

@@ -11,9 +11,13 @@ const fixtures = require('../common/fixtures');
 
 // Common args that will cause an out-of-memory error for child process.
 const ARGS = [
-  '--max-old-space-size=20',
+  '--max-heap-size=20',
   fixtures.path('report-oom'),
 ];
+const REPORT_FIELDS = [
+  ['header.trigger', 'OOMError'],
+  ['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
+];
 
 {
   // Verify that --report-compact is respected when set.
@@ -34,7 +38,5 @@ const ARGS = [
   const lines = child.stderr.split('\n');
   // Skip over unavoidable free-form output and gc log from V8.
   const report = lines.find((i) => i.startsWith('{'));
-  const json = JSON.parse(report);
-
-  assert.strictEqual(json.header.threadId, null);
+  helper.validateContent(report, REPORT_FIELDS);
 }

test/report/test-report-fatalerror-oomerror-not-set.js

@@ -11,7 +11,7 @@ const fixtures = require('../common/fixtures');
 
 // Common args that will cause an out-of-memory error for child process.
 const ARGS = [
-  '--max-old-space-size=20',
+  '--max-heap-size=20',
   fixtures.path('report-oom'),
 ];
 

test/report/test-report-fatalerror-oomerror-set.js

@@ -11,9 +11,13 @@ const fixtures = require('../common/fixtures');
 
 // Common args that will cause an out-of-memory error for child process.
 const ARGS = [
-  '--max-old-space-size=20',
+  '--max-heap-size=20',
   fixtures.path('report-oom'),
 ];
+const REPORT_FIELDS = [
+  ['header.trigger', 'OOMError'],
+  ['javascriptHeap.memoryLimit', 20 * 1024 * 1024 /* 20MB */],
+];
 
 {
   // Verify that --report-on-fatalerror is respected when set.
@@ -26,12 +30,5 @@ const ARGS = [
   assert.strictEqual(reports.length, 1);
 
   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(content.header.threadId, null);
-  assert.strictEqual(content.header.trigger, 'OOMError');
+  helper.validate(report, REPORT_FIELDS);
 }