Skip to content

Commit d861324

Browse files
legendecasFlarna
authored andcommitted
lib: properly process JavaScript exceptions on async_hooks fatal error
JavaScript exceptions could be arbitrary values. PR-URL: #38106 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 6cb314b commit d861324

File tree

2 files changed

+63
-2
lines changed

2 files changed

+63
-2
lines changed

Diff for: lib/internal/async_hooks.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ const {
100100
const { async_id_symbol,
101101
trigger_async_id_symbol } = internalBinding('symbols');
102102

103+
// Lazy load of internal/util/inspect;
104+
let inspect;
105+
103106
// Used in AsyncHook and AsyncResource.
104107
const init_symbol = Symbol('init');
105108
const before_symbol = Symbol('before');
@@ -156,12 +159,17 @@ function executionAsyncResource() {
156159
return lookupPublicResource(resource);
157160
}
158161

162+
function inspectExceptionValue(e) {
163+
inspect ??= require('internal/util/inspect').inspect;
164+
return { message: inspect(e) };
165+
}
166+
159167
// Used to fatally abort the process if a callback throws.
160168
function fatalError(e) {
161-
if (typeof e.stack === 'string') {
169+
if (typeof e?.stack === 'string') {
162170
process._rawDebug(e.stack);
163171
} else {
164-
const o = { message: e };
172+
const o = inspectExceptionValue(e);
165173
ErrorCaptureStackTrace(o, fatalError);
166174
process._rawDebug(o.stack);
167175
}

Diff for: test/parallel/test-async-hooks-fatal-error.js

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
const childProcess = require('child_process');
5+
const os = require('os');
6+
7+
if (process.argv[2] === 'child') {
8+
child(process.argv[3], process.argv[4]);
9+
} else {
10+
main();
11+
}
12+
13+
function child(type, valueType) {
14+
const { createHook } = require('async_hooks');
15+
const fs = require('fs');
16+
17+
createHook({
18+
[type]() {
19+
if (valueType === 'symbol') {
20+
throw Symbol('foo');
21+
}
22+
// eslint-disable-next-line no-throw-literal
23+
throw null;
24+
}
25+
}).enable();
26+
27+
// Trigger `promiseResolve`.
28+
new Promise((resolve) => resolve())
29+
// Trigger `after`/`destroy`.
30+
.then(() => fs.promises.readFile(__filename, 'utf8'))
31+
// Make process exit with code 0 if no error caught.
32+
.then(() => process.exit(0));
33+
}
34+
35+
function main() {
36+
const types = [ 'init', 'before', 'after', 'destroy', 'promiseResolve' ];
37+
const valueTypes = [
38+
[ 'null', 'Error: null' ],
39+
[ 'symbol', 'Error: Symbol(foo)' ],
40+
];
41+
for (const type of types) {
42+
for (const [valueType, expect] of valueTypes) {
43+
const cp = childProcess.spawnSync(
44+
process.execPath,
45+
[ __filename, 'child', type, valueType ],
46+
{
47+
encoding: 'utf8',
48+
});
49+
assert.strictEqual(cp.status, 1, type);
50+
assert.strictEqual(cp.stderr.trim().split(os.EOL)[0], expect, type);
51+
}
52+
}
53+
}

0 commit comments

Comments
 (0)