FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

report: skip report if uncaught exception is handled · nodejs/node@ff34d48 · GitHub

/ node Public

Commit ff34d48

Browse files
legendecas
authored and
juanarbol
committed
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 003ab59 commit ff34d48

9 files changed

Lines changed: 122 additions & 60 deletions

‎doc/api/cli.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,6 +1021,9 @@ Default signal is `SIGUSR2`.
10211021
<!-- YAML
10221022
added: v11.8.0
10231023
changes:
1024+
- version: REPLACEME
1025+
pr-url: https://github.com/nodejs/node/pull/44208
1026+
description: Report is not generated if the uncaught exception is handled.
10241027
- version:
10251028
- v13.12.0
10261029
- v12.17.0
@@ -1032,9 +1035,9 @@ changes:
10321035
`--report-uncaught-exception`.
10331036
-->
10341037

1035-
Enables report to be generated on uncaught exceptions. Useful when inspecting
1036-
the JavaScript stack in conjunction with native stack and other runtime
1037-
environment data.
1038+
Enables report to be generated when the process exits due to an uncaught
1039+
exception. Useful when inspecting the JavaScript stack in conjunction with
1040+
native stack and other runtime environment data.
10381041

10391042
### `--secure-heap=n`
10401043

‎lib/internal/process/execution.js‎

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

‎src/node_errors.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,7 @@ static void ReportFatalException(Environment* env,
354354
}
355355

356356
node::Utf8Value trace(env->isolate(), stack_trace);
357+
std::string report_message = "Exception";
357358

358359
// range errors have a trace member set to undefined
359360
if (trace.length() > 0 && !stack_trace->IsUndefined()) {
@@ -386,6 +387,8 @@ static void ReportFatalException(Environment* env,
386387
} else {
387388
node::Utf8Value name_string(env->isolate(), name.ToLocalChecked());
388389
node::Utf8Value message_string(env->isolate(), message.ToLocalChecked());
390+
// Update the report message if it is an object has message property.
391+
report_message = message_string.ToString();
389392

390393
if (arrow.IsEmpty() || !arrow->IsString() || decorated) {
391394
FPrintF(stderr, "%s: %s\n", name_string, message_string);
@@ -407,6 +410,11 @@ static void ReportFatalException(Environment* env,
407410
}
408411
}
409412

413+
if (env->isolate_data()->options()->report_uncaught_exception) {
414+
report::TriggerNodeReport(
415+
isolate, env, report_message.c_str(), "Exception", "", error);
416+
}
417+
410418
if (env->options()->trace_uncaught) {
411419
Local<StackTrace> trace = message->GetStackTrace();
412420
if (!trace.IsEmpty()) {

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

Lines 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+
}));

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

Lines 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;

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

Lines 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);

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

Lines 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;

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

Lines 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;

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

Lines 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)
Back | FazBrowse Home | New Git URL