Skip to content

Commit d8e0d9f

Browse files
legendecaslegendecas
committed
report: skip report if uncaught exception is handled
If the exception is handled by the userland process#uncaughtException, reports should not be generated repetitively as the process may continue to run.
1 parent ec44403 commit d8e0d9f

File tree

6 files changed

+97
-54
lines changed

6 files changed

+97
-54
lines changed

‎lib/internal/process/execution.js‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -151,34 +151,14 @@ function createOnGlobalUncaughtException() {
151151
// call that threw and was never cleared. So clear it now.
152152
clearDefaultTriggerAsyncId();
153153

154-
// If diagnostic reporting is enabled, call into its handler to see
155-
// whether it is interested in handling the situation.
156-
// Ignore if the error is scoped inside a domain.
157-
// use == in the checks as we want to allow for null and undefined
158-
if (er == null || er.domain == null) {
159-
try {
160-
const report = internalBinding('report');
161-
if (report != null && report.shouldReportOnUncaughtException()) {
162-
report.writeReport(
163-
typeof er?.message === 'string' ?
164-
er.message :
165-
'Exception',
166-
'Exception',
167-
null,
168-
er ?? {});
169-
}
170-
} catch {
171-
// Ignore the exception. Diagnostic reporting is unavailable.
172-
}
173-
}
174-
175154
const type = fromPromise ? 'unhandledRejection' : 'uncaughtException';
176155
process.emit('uncaughtExceptionMonitor', er, type);
177156
if (exceptionHandlerState.captureFn !== null) {
178157
exceptionHandlerState.captureFn(er);
179158
} else if (!process.emit('uncaughtException', er, type)) {
180159
// If someone handled it, then great. Otherwise, die in C++ land
181160
// since that means that we'll exit the process, emit the 'exit' event.
161+
182162
try {
183163
if (!process._exiting) {
184164
process._exiting = true;

‎src/node_errors.cc‎

Lines 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()) {

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

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,33 @@
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+
process.report.directory = process.argv[3];
11+
12+
throw new Error('test error');
13+
}
14+
15+
tmpdir.refresh();
16+
const child = childProcess.spawn(process.execPath, [
17+
'--report-uncaught-exception',
18+
'--report-compact',
19+
__filename,
20+
'child',
21+
tmpdir.path,
22+
]);
23+
child.on('exit', common.mustCall((code) => {
24+
assert.strictEqual(code, 1);
25+
const reports = helper.findReports(child.pid, tmpdir.path);
26+
assert.strictEqual(reports.length, 1);
27+
28+
helper.validate(reports[0], [
29+
['header.event', 'Exception'],
30+
['header.trigger', 'Exception'],
31+
['javascriptStack.message', 'Error: test error'],
32+
]);
33+
}));

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

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,33 @@
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+
process.report.directory = process.argv[3];
1011

11-
tmpdir.refresh();
12-
process.report.directory = tmpdir.path;
12+
// eslint-disable-next-line no-throw-literal
13+
throw 1;
14+
}
1315

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

1928
helper.validate(reports[0], [
2029
['header.event', 'Exception'],
21-
['javascriptStack.message', `${exception}`],
30+
['header.trigger', 'Exception'],
31+
['javascriptStack.message', '1'],
2232
]);
2333
}));
24-
25-
throw exception;

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

Lines changed: 16 additions & 9 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 = Symbol('foobar');
9+
if (process.argv[2] === 'child') {
10+
process.report.directory = process.argv[3];
1011

11-
tmpdir.refresh();
12-
process.report.directory = tmpdir.path;
12+
throw Symbol('foobar');
13+
}
1314

14-
process.on('uncaughtException', common.mustCall((err) => {
15-
assert.strictEqual(err, exception);
16-
const reports = helper.findReports(process.pid, tmpdir.path);
15+
tmpdir.refresh();
16+
const child = childProcess.spawn(process.execPath, [
17+
'--report-uncaught-exception',
18+
__filename,
19+
'child',
20+
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'],
29+
['header.trigger', 'Exception'],
2130
['javascriptStack.message', 'Symbol(foobar)'],
2231
]);
2332
}));
24-
25-
throw exception;

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +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');
8-
const error = new Error('test error');
98

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

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

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

0 commit comments

Comments
 (0)