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 c842ab3

Browse filesBrowse files
legendecasruyadorno
authored andcommitted
report: skip report if uncaught exception is handled
If the exception is handled by the userland process#uncaughtException handler, reports should not be generated repetitively as the process may continue to run. PR-URL: #44208 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent a02492f commit c842ab3
Copy full SHA for c842ab3
Expand file treeCollapse file tree

9 files changed

+122
-60
lines changed
Open diff view settings
Collapse file

‎doc/api/cli.md‎

Copy file name to clipboardExpand all lines: doc/api/cli.md
+6-3Lines changed: 6 additions & 3 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -1048,6 +1048,9 @@ Default signal is `SIGUSR2`.
10481048
<!-- YAML
10491049
added: v11.8.0
10501050
changes:
1051+
- version: REPLACEME
1052+
pr-url: https://github.com/nodejs/node/pull/44208
1053+
description: Report is not generated if the uncaught exception is handled.
10511054
- version:
10521055
- v13.12.0
10531056
- v12.17.0
@@ -1059,9 +1062,9 @@ changes:
10591062
`--report-uncaught-exception`.
10601063
-->
10611064

1062-
Enables report to be generated on uncaught exceptions. Useful when inspecting
1063-
the JavaScript stack in conjunction with native stack and other runtime
1064-
environment data.
1065+
Enables report to be generated when the process exits due to an uncaught
1066+
exception. Useful when inspecting the JavaScript stack in conjunction with
1067+
native stack and other runtime environment data.
10651068

10661069
### `--secure-heap=n`
10671070

Collapse file

‎lib/internal/process/execution.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/execution.js
-21Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -139,27 +139,6 @@ function createOnGlobalUncaughtException() {
139139
// call that threw and was never cleared. So clear it now.
140140
clearDefaultTriggerAsyncId();
141141

142-
// If diagnostic reporting is enabled, call into its handler to see
143-
// whether it is interested in handling the situation.
144-
// Ignore if the error is scoped inside a domain.
145-
// use == in the checks as we want to allow for null and undefined
146-
if (er == null || er.domain == null) {
147-
try {
148-
const report = internalBinding('report');
149-
if (report != null && report.shouldReportOnUncaughtException()) {
150-
report.writeReport(
151-
typeof er?.message === 'string' ?
152-
er.message :
153-
'Exception',
154-
'Exception',
155-
null,
156-
er ?? {});
157-
}
158-
} catch {
159-
// Ignore the exception. Diagnostic reporting is unavailable.
160-
}
161-
}
162-
163142
const type = fromPromise ? 'unhandledRejection' : 'uncaughtException';
164143
process.emit('uncaughtExceptionMonitor', er, type);
165144
if (exceptionHandlerState.captureFn !== null) {
Collapse file

‎src/node_errors.cc‎

Copy file name to clipboardExpand all lines: src/node_errors.cc
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,7 @@ static void ReportFatalException(Environment* env,
392392
}
393393

394394
node::Utf8Value trace(env->isolate(), stack_trace);
395+
std::string report_message = "Exception";
395396

396397
// range errors have a trace member set to undefined
397398
if (trace.length() > 0 && !stack_trace->IsUndefined()) {
@@ -424,6 +425,8 @@ static void ReportFatalException(Environment* env,
424425
} else {
425426
node::Utf8Value name_string(env->isolate(), name.ToLocalChecked());
426427
node::Utf8Value message_string(env->isolate(), message.ToLocalChecked());
428+
// Update the report message if it is an object has message property.
429+
report_message = message_string.ToString();
427430

428431
if (arrow.IsEmpty() || !arrow->IsString() || decorated) {
429432
FPrintF(stderr, "%s: %s\n", name_string, message_string);
@@ -445,6 +448,11 @@ static void ReportFatalException(Environment* env,
445448
}
446449
}
447450

451+
if (env->isolate_data()->options()->report_uncaught_exception) {
452+
report::TriggerNodeReport(
453+
isolate, env, report_message.c_str(), "Exception", "", error);
454+
}
455+
448456
if (env->options()->trace_uncaught) {
449457
Local<StackTrace> trace = message->GetStackTrace();
450458
if (!trace.IsEmpty()) {
Collapse file
+31-4Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,32 @@
1-
// Flags: --experimental-report --report-uncaught-exception --report-compact
21
'use strict';
3-
// Test producing a compact report on uncaught exception.
4-
require('../common');
5-
require('./test-report-uncaught-exception.js');
2+
// Test producing a report on uncaught exception.
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const childProcess = require('child_process');
6+
const helper = require('../common/report');
7+
const tmpdir = require('../common/tmpdir');
8+
9+
if (process.argv[2] === 'child') {
10+
throw new Error('test error');
11+
}
12+
13+
tmpdir.refresh();
14+
const child = childProcess.spawn(process.execPath, [
15+
'--report-uncaught-exception',
16+
'--report-compact',
17+
__filename,
18+
'child',
19+
], {
20+
cwd: tmpdir.path
21+
});
22+
child.on('exit', common.mustCall((code) => {
23+
assert.strictEqual(code, 1);
24+
const reports = helper.findReports(child.pid, tmpdir.path);
25+
assert.strictEqual(reports.length, 1);
26+
27+
helper.validate(reports[0], [
28+
['header.event', 'Exception'],
29+
['header.trigger', 'Exception'],
30+
['javascriptStack.message', 'Error: test error'],
31+
]);
32+
}));
Collapse file
+23Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Flags: --report-uncaught-exception
2+
'use strict';
3+
// Test report is suppressed on uncaught exception hook.
4+
const common = require('../common');
5+
const assert = require('assert');
6+
const helper = require('../common/report');
7+
const tmpdir = require('../common/tmpdir');
8+
const error = new Error('test error');
9+
10+
tmpdir.refresh();
11+
process.report.directory = tmpdir.path;
12+
13+
// Make sure the uncaughtException listener is called.
14+
process.on('uncaughtException', common.mustCall());
15+
16+
process.on('exit', (code) => {
17+
assert.strictEqual(code, 0);
18+
// Make sure no reports are generated.
19+
const reports = helper.findReports(process.pid, tmpdir.path);
20+
assert.strictEqual(reports.length, 0);
21+
});
22+
23+
throw error;
Collapse file

‎test/report/test-report-uncaught-exception-override.js‎

Copy file name to clipboardExpand all lines: test/report/test-report-uncaught-exception-override.js
+1-3Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ process.report.directory = tmpdir.path;
1212

1313
// First, install an uncaught exception hook.
1414
process.setUncaughtExceptionCaptureCallback(common.mustCall());
15-
16-
// Make sure this is ignored due to the above override.
17-
process.on('uncaughtException', common.mustNotCall());
15+
// Do not install process uncaughtException handler.
1816

1917
process.on('exit', (code) => {
2018
assert.strictEqual(code, 0);
Collapse file

‎test/report/test-report-uncaught-exception-primitives.js‎

Copy file name to clipboardExpand all lines: test/report/test-report-uncaught-exception-primitives.js
+17-10Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,32 @@
1-
// Flags: --report-uncaught-exception
21
'use strict';
32
// Test producing a report on uncaught exception.
43
const common = require('../common');
54
const assert = require('assert');
5+
const childProcess = require('child_process');
66
const helper = require('../common/report');
77
const tmpdir = require('../common/tmpdir');
88

9-
const exception = 1;
9+
if (process.argv[2] === 'child') {
10+
// eslint-disable-next-line no-throw-literal
11+
throw 1;
12+
}
1013

1114
tmpdir.refresh();
12-
process.report.directory = tmpdir.path;
13-
14-
process.on('uncaughtException', common.mustCall((err) => {
15-
assert.strictEqual(err, exception);
16-
const reports = helper.findReports(process.pid, tmpdir.path);
15+
const child = childProcess.spawn(process.execPath, [
16+
'--report-uncaught-exception',
17+
__filename,
18+
'child',
19+
], {
20+
cwd: tmpdir.path,
21+
});
22+
child.on('exit', common.mustCall((code) => {
23+
assert.strictEqual(code, 1);
24+
const reports = helper.findReports(child.pid, tmpdir.path);
1725
assert.strictEqual(reports.length, 1);
1826

1927
helper.validate(reports[0], [
2028
['header.event', 'Exception'],
21-
['javascriptStack.message', `${exception}`],
29+
['header.trigger', 'Exception'],
30+
['javascriptStack.message', '1'],
2231
]);
2332
}));
24-
25-
throw exception;
Collapse file

‎test/report/test-report-uncaught-exception-symbols.js‎

Copy file name to clipboardExpand all lines: test/report/test-report-uncaught-exception-symbols.js
+15-9Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,31 @@
1-
// Flags: --report-uncaught-exception
21
'use strict';
32
// Test producing a report on uncaught exception.
43
const common = require('../common');
54
const assert = require('assert');
5+
const childProcess = require('child_process');
66
const helper = require('../common/report');
77
const tmpdir = require('../common/tmpdir');
88

9-
const exception = Symbol('foobar');
9+
if (process.argv[2] === 'child') {
10+
throw Symbol('foobar');
11+
}
1012

1113
tmpdir.refresh();
12-
process.report.directory = tmpdir.path;
13-
14-
process.on('uncaughtException', common.mustCall((err) => {
15-
assert.strictEqual(err, exception);
16-
const reports = helper.findReports(process.pid, tmpdir.path);
14+
const child = childProcess.spawn(process.execPath, [
15+
'--report-uncaught-exception',
16+
__filename,
17+
'child',
18+
], {
19+
cwd: tmpdir.path,
20+
});
21+
child.on('exit', common.mustCall((code) => {
22+
assert.strictEqual(code, 1);
23+
const reports = helper.findReports(child.pid, tmpdir.path);
1724
assert.strictEqual(reports.length, 1);
1825

1926
helper.validate(reports[0], [
2027
['header.event', 'Exception'],
28+
['header.trigger', 'Exception'],
2129
['javascriptStack.message', 'Symbol(foobar)'],
2230
]);
2331
}));
24-
25-
throw exception;
Collapse file
+21-10Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,31 @@
1-
// Flags: --report-uncaught-exception
21
'use strict';
32
// Test producing a report on uncaught exception.
43
const common = require('../common');
54
const assert = require('assert');
5+
const childProcess = require('child_process');
66
const helper = require('../common/report');
77
const tmpdir = require('../common/tmpdir');
8-
const error = new Error('test error');
98

10-
tmpdir.refresh();
11-
process.report.directory = tmpdir.path;
9+
if (process.argv[2] === 'child') {
10+
throw new Error('test error');
11+
}
1212

13-
process.on('uncaughtException', common.mustCall((err) => {
14-
assert.strictEqual(err, error);
15-
const reports = helper.findReports(process.pid, tmpdir.path);
13+
tmpdir.refresh();
14+
const child = childProcess.spawn(process.execPath, [
15+
'--report-uncaught-exception',
16+
__filename,
17+
'child',
18+
], {
19+
cwd: tmpdir.path,
20+
});
21+
child.on('exit', common.mustCall((code) => {
22+
assert.strictEqual(code, 1);
23+
const reports = helper.findReports(child.pid, tmpdir.path);
1624
assert.strictEqual(reports.length, 1);
17-
helper.validate(reports[0]);
18-
}));
1925

20-
throw error;
26+
helper.validate(reports[0], [
27+
['header.event', 'Exception'],
28+
['header.trigger', 'Exception'],
29+
['javascriptStack.message', 'Error: test error'],
30+
]);
31+
}));

0 commit comments

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