report: handle on-fatalerror better

--report-on-fatalerror was not honored properly, as there was no
way to check the value which was stored in the Environment pointer
which can be inaccessible under certain fatal error situations.
Move the flag out of Environment pointer so that this is doable.
Co-authored-by: Shobhit Chittora schittora@paypal.com
PR-URL: #32207Fixes: #31576
Refs: #29881
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

Author: HarshithaKP

src/node_errors.cc

@@ -406,7 +406,7 @@ void OnFatalError(const char* location, const char* message) {
 
  Isolate* isolate = Isolate::GetCurrent();
  Environment* env = Environment::GetCurrent(isolate);
-if (env == nullptr || env->isolate_data()->options()->report_on_fatalerror) {
+if (per_process::cli_options->report_on_fatalerror) {
 report::TriggerNodeReport(
  isolate, env, message, "FatalError", "", Local<String>());
  }

src/node_options.cc

@@ -590,10 +590,6 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
 "generate diagnostic report upon receiving signals",
  &PerIsolateOptions::report_on_signal,
 kAllowedInEnvironment);
-AddOption("--report-on-fatalerror",
-"generate diagnostic report on fatal (internal) errors",
- &PerIsolateOptions::report_on_fatalerror,
-kAllowedInEnvironment);
 AddOption("--report-signal",
 "causes diagnostic report to be produced on provided signal,"
 " unsupported in Windows. (default: SIGUSR2)",
@@ -667,6 +663,10 @@ PerProcessOptionsParser::PerProcessOptionsParser(
 AddOption("--v8-options",
 "print V8 command line options",
  &PerProcessOptions::print_v8_help);
+AddOption("--report-on-fatalerror",
+"generate diagnostic report on fatal (internal) errors",
+ &PerProcessOptions::report_on_fatalerror,
+kAllowedInEnvironment);
 
 #ifdef NODE_HAVE_I18N_SUPPORT
 AddOption("--icu-data-dir",

src/node_options.h

@@ -186,7 +186,6 @@ class PerIsolateOptions : public Options {
 bool no_node_snapshot = false;
 bool report_uncaught_exception = false;
 bool report_on_signal = false;
-bool report_on_fatalerror = false;
 bool report_compact = false;
  std::string report_signal = "SIGUSR2";
  std::string report_filename;
@@ -236,6 +235,7 @@ class PerProcessOptions : public Options {
  std::string use_largepages = "off";
 bool trace_sigint = false;
  std::vector<std::string> cmdline;
+bool report_on_fatalerror = false;
 
 inline PerIsolateOptions* get_per_isolate_options();
 voidCheckOptions(std::vector<std::string>* errors) override;

src/node_report_module.cc

@@ -131,13 +131,13 @@ static void SetSignal(const FunctionCallbackInfo<Value>& info) {
 staticvoidShouldReportOnFatalError(const FunctionCallbackInfo<Value>& info) {
  Environment* env = Environment::GetCurrent(info);
  info.GetReturnValue().Set(
-env->isolate_data()->options()->report_on_fatalerror);
+node::per_process::cli_options->report_on_fatalerror);
 }
 
 staticvoidSetReportOnFatalError(const FunctionCallbackInfo<Value>& info) {
  Environment* env = Environment::GetCurrent(info);
 CHECK(info[0]->IsBoolean());
-env->isolate_data()->options()->report_on_fatalerror = info[0]->IsTrue();
+node::per_process::cli_options->report_on_fatalerror = info[0]->IsTrue();
 }
 
 staticvoidShouldReportOnSignal(const FunctionCallbackInfo<Value>& info) {

test/report/test-report-fatal-error.js

@@ -1,6 +1,6 @@
 'use strict';
 
-constcommon=require('../common');
+require('../common');
 constassert=require('assert');
 // Testcase to produce report on fatal error (javascript heap OOM)
 if(process.argv[2]==='child'){
@@ -20,17 +20,26 @@ if (process.argv[2] === 'child') {
 consthelper=require('../common/report.js');
 consttmpdir=require('../common/tmpdir');
 tmpdir.refresh();
-constspawn=require('child_process').spawn;
-constargs=['--report-on-fatalerror',
-'--max-old-space-size=20',
-__filename,
-'child'];
-constchild=spawn(process.execPath,args,{cwd: tmpdir.path});
-child.on('exit',common.mustCall((code)=>{
-assert.notStrictEqual(code,0,'Process exited unexpectedly');
-constreports=helper.findReports(child.pid,tmpdir.path);
-assert.strictEqual(reports.length,1);
-constreport=reports[0];
-helper.validate(report);
-}));
+constspawnSync=require('child_process').spawnSync;
+letargs=['--report-on-fatalerror',
+'--max-old-space-size=20',
+__filename,
+'child'];
+
+letchild=spawnSync(process.execPath,args,{cwd: tmpdir.path});
+
+assert.notStrictEqual(child.status,0,'Process exited unexpectedly');
+letreports=helper.findReports(child.pid,tmpdir.path);
+assert.strictEqual(reports.length,1);
+constreport=reports[0];
+helper.validate(report);
+// Verify that reports are not created on fatal error by default.
+args=['--max-old-space-size=20',
+__filename,
+'child'];
+
+child=spawnSync(process.execPath,args,{cwd: tmpdir.path});
+assert.notStrictEqual(child.status,0,'Process exited unexpectedly');
+reports=helper.findReports(child.pid,tmpdir.path);
+assert.strictEqual(reports.length,0);
 }