Skip to content

Commit decb848

Browse files
author
Shobhit Chittora
committed
src: removes fatal_error check from per-isolate + adds tests
1 parent 4cfbc7b commit decb848

File tree

4 files changed

+35
-30
lines changed

4 files changed

+35
-30
lines changed

src/node_options.cc

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,15 @@ void PerProcessOptions::CheckOptions(std::vector<std::string>* errors) {
7070
use_largepages != "silent") {
7171
errors->push_back("invalid value for --use-largepages");
7272
}
73+
74+
// #ifdef NODE_REPORT
75+
// if (report_on_fatalerror) {
76+
// errors->push_back(
77+
// "--report-on-fatalerror option is valid only when "
78+
// "--experimental-report is set");
79+
// }
80+
// #endif // NODE_REPORT
81+
7382
per_isolate->CheckOptions(errors);
7483
}
7584

@@ -100,12 +109,6 @@ void PerIsolateOptions::CheckOptions(std::vector<std::string>* errors) {
100109
"--experimental-report is set");
101110
}
102111

103-
if (report_on_fatalerror) {
104-
errors->push_back(
105-
"--report-on-fatalerror option is valid only when "
106-
"--experimental-report is set");
107-
}
108-
109112
if (report_on_signal) {
110113
errors->push_back("--report-on-signal option is valid only when "
111114
"--experimental-report is set");
@@ -622,10 +625,6 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
622625
"generate diagnostic report upon receiving signals",
623626
&PerIsolateOptions::report_on_signal,
624627
kAllowedInEnvironment);
625-
AddOption("--report-on-fatalerror",
626-
"generate diagnostic report on fatal (internal) errors",
627-
&PerIsolateOptions::report_on_fatalerror,
628-
kAllowedInEnvironment);
629628
AddOption("--report-signal",
630629
"causes diagnostic report to be produced on provided signal,"
631630
" unsupported in Windows. (default: SIGUSR2)",
@@ -699,12 +698,10 @@ PerProcessOptionsParser::PerProcessOptionsParser(
699698
&PerProcessOptions::print_v8_help);
700699

701700
#ifdef NODE_REPORT
702-
703701
AddOption("--report-on-fatalerror",
704702
"generate diagnostic report on fatal (internal) errors",
705703
&PerProcessOptions::report_on_fatalerror,
706-
kAllowedInEnvironment);
707-
704+
kAllowedInEnvironment);
708705
#endif // NODE_REPORT
709706

710707
#ifdef NODE_HAVE_I18N_SUPPORT

src/node_options.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,6 @@ class PerIsolateOptions : public Options {
190190
#ifdef NODE_REPORT
191191
bool report_uncaught_exception = false;
192192
bool report_on_signal = false;
193-
bool report_on_fatalerror = false;
194193
std::string report_signal;
195194
std::string report_filename;
196195
std::string report_directory;

src/node_report_module.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,13 @@ static void SetSignal(const FunctionCallbackInfo<Value>& info) {
119119
static void ShouldReportOnFatalError(const FunctionCallbackInfo<Value>& info) {
120120
Environment* env = Environment::GetCurrent(info);
121121
info.GetReturnValue().Set(
122-
env->isolate_data()->options()->report_on_fatalerror);
122+
node::per_process::cli_options->report_on_fatalerror);
123123
}
124124

125125
static void SetReportOnFatalError(const FunctionCallbackInfo<Value>& info) {
126126
Environment* env = Environment::GetCurrent(info);
127127
CHECK(info[0]->IsBoolean());
128-
env->isolate_data()->options()->report_on_fatalerror = info[0]->IsTrue();
128+
node::per_process::cli_options->report_on_fatalerror = info[0]->IsTrue();
129129
}
130130

131131
static void ShouldReportOnSignal(const FunctionCallbackInfo<Value>& info) {

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

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,27 @@ if (process.argv[2] === 'child') {
2121
const helper = require('../common/report.js');
2222
const tmpdir = require('../common/tmpdir');
2323
tmpdir.refresh();
24-
const spawn = require('child_process').spawn;
25-
const args = ['--experimental-report',
26-
'--report-on-fatalerror',
27-
'--max-old-space-size=20',
28-
__filename,
29-
'child'];
30-
const child = spawn(process.execPath, args, { cwd: tmpdir.path });
31-
child.on('exit', common.mustCall((code) => {
32-
assert.notStrictEqual(code, 0, 'Process exited unexpectedly');
33-
const reports = helper.findReports(child.pid, tmpdir.path);
34-
assert.strictEqual(reports.length, 1);
35-
const report = reports[0];
36-
helper.validate(report);
37-
}));
24+
const spawnSync = require('child_process').spawnSync;
25+
let args = ['--experimental-report',
26+
'--report-on-fatalerror',
27+
'--max-old-space-size=20',
28+
__filename,
29+
'child'];
30+
31+
let child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
32+
33+
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
34+
let reports = helper.findReports(child.pid, tmpdir.path);
35+
assert.strictEqual(reports.length, 1);
36+
const report = reports[0];
37+
helper.validate(report);
38+
39+
args = ['--max-old-space-size=20',
40+
__filename,
41+
'child'];
42+
43+
child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
44+
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
45+
reports = helper.findReports(child.pid, tmpdir.path);
46+
assert.strictEqual(reports.length, 0);
3847
}

0 commit comments

Comments
 (0)