Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 28e298f

Browse filesBrowse files
HarshithaKPMylesBorins
HarshithaKP
authored and
MylesBorins
committed
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: #32207 Fixes: #31576 Refs: #29881 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
1 parent b7a8878 commit 28e298f
Copy full SHA for 28e298f

File tree

Expand file treeCollapse file tree

5 files changed

+31
-22
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+31
-22
lines changed
Open diff view settings
Collapse file

‎src/node_errors.cc‎

Copy file name to clipboardExpand all lines: src/node_errors.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ void OnFatalError(const char* location, const char* message) {
406406

407407
Isolate* isolate = Isolate::GetCurrent();
408408
Environment* env = Environment::GetCurrent(isolate);
409-
if (env == nullptr || env->isolate_data()->options()->report_on_fatalerror) {
409+
if (per_process::cli_options->report_on_fatalerror) {
410410
report::TriggerNodeReport(
411411
isolate, env, message, "FatalError", "", Local<String>());
412412
}
Collapse file

‎src/node_options.cc‎

Copy file name to clipboardExpand all lines: src/node_options.cc
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -590,10 +590,6 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
590590
"generate diagnostic report upon receiving signals",
591591
&PerIsolateOptions::report_on_signal,
592592
kAllowedInEnvironment);
593-
AddOption("--report-on-fatalerror",
594-
"generate diagnostic report on fatal (internal) errors",
595-
&PerIsolateOptions::report_on_fatalerror,
596-
kAllowedInEnvironment);
597593
AddOption("--report-signal",
598594
"causes diagnostic report to be produced on provided signal,"
599595
" unsupported in Windows. (default: SIGUSR2)",
@@ -667,6 +663,10 @@ PerProcessOptionsParser::PerProcessOptionsParser(
667663
AddOption("--v8-options",
668664
"print V8 command line options",
669665
&PerProcessOptions::print_v8_help);
666+
AddOption("--report-on-fatalerror",
667+
"generate diagnostic report on fatal (internal) errors",
668+
&PerProcessOptions::report_on_fatalerror,
669+
kAllowedInEnvironment);
670670

671671
#ifdef NODE_HAVE_I18N_SUPPORT
672672
AddOption("--icu-data-dir",
Collapse file

‎src/node_options.h‎

Copy file name to clipboardExpand all lines: src/node_options.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,6 @@ class PerIsolateOptions : public Options {
186186
bool no_node_snapshot = false;
187187
bool report_uncaught_exception = false;
188188
bool report_on_signal = false;
189-
bool report_on_fatalerror = false;
190189
bool report_compact = false;
191190
std::string report_signal = "SIGUSR2";
192191
std::string report_filename;
@@ -236,6 +235,7 @@ class PerProcessOptions : public Options {
236235
std::string use_largepages = "off";
237236
bool trace_sigint = false;
238237
std::vector<std::string> cmdline;
238+
bool report_on_fatalerror = false;
239239

240240
inline PerIsolateOptions* get_per_isolate_options();
241241
void CheckOptions(std::vector<std::string>* errors) override;
Collapse file

‎src/node_report_module.cc‎

Copy file name to clipboardExpand all lines: src/node_report_module.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,13 @@ static void SetSignal(const FunctionCallbackInfo<Value>& info) {
131131
static void ShouldReportOnFatalError(const FunctionCallbackInfo<Value>& info) {
132132
Environment* env = Environment::GetCurrent(info);
133133
info.GetReturnValue().Set(
134-
env->isolate_data()->options()->report_on_fatalerror);
134+
node::per_process::cli_options->report_on_fatalerror);
135135
}
136136

137137
static void SetReportOnFatalError(const FunctionCallbackInfo<Value>& info) {
138138
Environment* env = Environment::GetCurrent(info);
139139
CHECK(info[0]->IsBoolean());
140-
env->isolate_data()->options()->report_on_fatalerror = info[0]->IsTrue();
140+
node::per_process::cli_options->report_on_fatalerror = info[0]->IsTrue();
141141
}
142142

143143
static void ShouldReportOnSignal(const FunctionCallbackInfo<Value>& info) {
Collapse file

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

Copy file name to clipboard
+23-14Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
const common = require('../common');
3+
require('../common');
44
const assert = require('assert');
55
// Testcase to produce report on fatal error (javascript heap OOM)
66
if (process.argv[2] === 'child') {
@@ -20,17 +20,26 @@ if (process.argv[2] === 'child') {
2020
const helper = require('../common/report.js');
2121
const tmpdir = require('../common/tmpdir');
2222
tmpdir.refresh();
23-
const spawn = require('child_process').spawn;
24-
const args = ['--report-on-fatalerror',
25-
'--max-old-space-size=20',
26-
__filename,
27-
'child'];
28-
const child = spawn(process.execPath, args, { cwd: tmpdir.path });
29-
child.on('exit', common.mustCall((code) => {
30-
assert.notStrictEqual(code, 0, 'Process exited unexpectedly');
31-
const reports = helper.findReports(child.pid, tmpdir.path);
32-
assert.strictEqual(reports.length, 1);
33-
const report = reports[0];
34-
helper.validate(report);
35-
}));
23+
const spawnSync = require('child_process').spawnSync;
24+
let args = ['--report-on-fatalerror',
25+
'--max-old-space-size=20',
26+
__filename,
27+
'child'];
28+
29+
let child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
30+
31+
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
32+
let reports = helper.findReports(child.pid, tmpdir.path);
33+
assert.strictEqual(reports.length, 1);
34+
const report = reports[0];
35+
helper.validate(report);
36+
// Verify that reports are not created on fatal error by default.
37+
args = ['--max-old-space-size=20',
38+
__filename,
39+
'child'];
40+
41+
child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
42+
assert.notStrictEqual(child.status, 0, 'Process exited unexpectedly');
43+
reports = helper.findReports(child.pid, tmpdir.path);
44+
assert.strictEqual(reports.length, 0);
3645
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.