Skip to content

Commit 56345a3

Browse files
bmecktargos
bmeck
authored and
targos
committed
async_hooks: fix AsyncLocalStorage in unhandledRejection cases
PR-URL: #41202 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vladimir de Turckheim <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
1 parent 367ab2a commit 56345a3

File tree

3 files changed

+187
-64
lines changed

3 files changed

+187
-64
lines changed

‎lib/internal/async_hooks.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,16 @@ function clearDefaultTriggerAsyncId() {
443443
async_id_fields[kDefaultTriggerAsyncId] = -1;
444444
}
445445

446-
446+
/**
447+
* Sets a default top level trigger ID to be used
448+
*
449+
* @template {Array<unknown>} T
450+
* @template {unknown} R
451+
* @param {number} triggerAsyncId
452+
* @param { (...T: args) => R } block
453+
* @param {T} args
454+
* @returns {R}
455+
*/
447456
function defaultTriggerAsyncIdScope(triggerAsyncId, block, ...args) {
448457
if (triggerAsyncId === undefined)
449458
return block.apply(null, args);

‎lib/internal/process/promises.js

+68-41
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@ const {
2727
const {
2828
pushAsyncContext,
2929
popAsyncContext,
30+
symbols: {
31+
async_id_symbol: kAsyncIdSymbol,
32+
trigger_async_id_symbol: kTriggerAsyncIdSymbol
33+
}
3034
} = require('internal/async_hooks');
31-
const async_hooks = require('async_hooks');
3235
const { isErrorStackTraceLimitWritable } = require('internal/errors');
3336

3437
// *Must* match Environment::TickInfo::Fields in src/env.h.
@@ -123,20 +126,11 @@ function resolveError(type, promise, reason) {
123126
}
124127

125128
function unhandledRejection(promise, reason) {
126-
const asyncId = async_hooks.executionAsyncId();
127-
const triggerAsyncId = async_hooks.triggerAsyncId();
128-
const resource = promise;
129-
130129
const emit = (reason, promise, promiseInfo) => {
131-
try {
132-
pushAsyncContext(asyncId, triggerAsyncId, resource);
133-
if (promiseInfo.domain) {
134-
return promiseInfo.domain.emit('error', reason);
135-
}
136-
return process.emit('unhandledRejection', reason, promise);
137-
} finally {
138-
popAsyncContext(asyncId);
130+
if (promiseInfo.domain) {
131+
return promiseInfo.domain.emit('error', reason);
139132
}
133+
return process.emit('unhandledRejection', reason, promise);
140134
};
141135

142136
maybeUnhandledPromises.set(promise, {
@@ -220,40 +214,73 @@ function processPromiseRejections() {
220214
promiseInfo.warned = true;
221215
const { reason, uid, emit } = promiseInfo;
222216

223-
switch (unhandledRejectionsMode) {
224-
case kStrictUnhandledRejections: {
225-
const err = reason instanceof Error ?
226-
reason : generateUnhandledRejectionError(reason);
227-
triggerUncaughtException(err, true /* fromPromise */);
228-
const handled = emit(reason, promise, promiseInfo);
229-
if (!handled) emitUnhandledRejectionWarning(uid, reason);
230-
break;
231-
}
232-
case kIgnoreUnhandledRejections: {
233-
emit(reason, promise, promiseInfo);
234-
break;
235-
}
236-
case kAlwaysWarnUnhandledRejections: {
237-
emit(reason, promise, promiseInfo);
238-
emitUnhandledRejectionWarning(uid, reason);
239-
break;
240-
}
241-
case kThrowUnhandledRejections: {
242-
const handled = emit(reason, promise, promiseInfo);
243-
if (!handled) {
217+
let needPop = true;
218+
const {
219+
[kAsyncIdSymbol]: promiseAsyncId,
220+
[kTriggerAsyncIdSymbol]: promiseTriggerAsyncId,
221+
} = promise;
222+
// We need to check if async_hooks are enabled
223+
// don't use enabledHooksExist as a Promise could
224+
// come from a vm.* context and not have an async id
225+
if (typeof promiseAsyncId !== 'undefined') {
226+
pushAsyncContext(
227+
promiseAsyncId,
228+
promiseTriggerAsyncId,
229+
promise
230+
);
231+
}
232+
try {
233+
switch (unhandledRejectionsMode) {
234+
case kStrictUnhandledRejections: {
244235
const err = reason instanceof Error ?
245236
reason : generateUnhandledRejectionError(reason);
237+
// This destroys the async stack, don't clear it after
246238
triggerUncaughtException(err, true /* fromPromise */);
239+
if (typeof promiseAsyncId !== 'undefined') {
240+
pushAsyncContext(
241+
promise[kAsyncIdSymbol],
242+
promise[kTriggerAsyncIdSymbol],
243+
promise
244+
);
245+
}
246+
const handled = emit(reason, promise, promiseInfo);
247+
if (!handled) emitUnhandledRejectionWarning(uid, reason);
248+
break;
247249
}
248-
break;
249-
}
250-
case kWarnWithErrorCodeUnhandledRejections: {
251-
const handled = emit(reason, promise, promiseInfo);
252-
if (!handled) {
250+
case kIgnoreUnhandledRejections: {
251+
emit(reason, promise, promiseInfo);
252+
break;
253+
}
254+
case kAlwaysWarnUnhandledRejections: {
255+
emit(reason, promise, promiseInfo);
253256
emitUnhandledRejectionWarning(uid, reason);
254-
process.exitCode = 1;
257+
break;
258+
}
259+
case kThrowUnhandledRejections: {
260+
const handled = emit(reason, promise, promiseInfo);
261+
if (!handled) {
262+
const err = reason instanceof Error ?
263+
reason : generateUnhandledRejectionError(reason);
264+
// This destroys the async stack, don't clear it after
265+
triggerUncaughtException(err, true /* fromPromise */);
266+
needPop = false;
267+
}
268+
break;
269+
}
270+
case kWarnWithErrorCodeUnhandledRejections: {
271+
const handled = emit(reason, promise, promiseInfo);
272+
if (!handled) {
273+
emitUnhandledRejectionWarning(uid, reason);
274+
process.exitCode = 1;
275+
}
276+
break;
277+
}
278+
}
279+
} finally {
280+
if (needPop) {
281+
if (typeof promiseAsyncId !== 'undefined') {
282+
popAsyncContext(promiseAsyncId);
255283
}
256-
break;
257284
}
258285
}
259286
maybeScheduledTicksOrMicrotasks = true;

‎test/async-hooks/test-async-local-storage-errors.js

+109-22
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,118 @@
11
'use strict';
2-
require('../common');
2+
const common = require('../common');
33
const assert = require('assert');
44
const { AsyncLocalStorage } = require('async_hooks');
5+
const vm = require('vm');
6+
7+
// err1 is emitted sync as a control - no events
8+
// err2 is emitted after a timeout - uncaughtExceptionMonitor
9+
// + uncaughtException
10+
// err3 is emitted after some awaits - unhandledRejection
11+
// err4 is emitted during handling err3 - uncaughtExceptionMonitor
12+
// err5 is emitted after err4 from a VM lacking hooks - unhandledRejection
13+
// + uncaughtException
514

6-
// case 2 using *AndReturn calls (dual behaviors)
715
const asyncLocalStorage = new AsyncLocalStorage();
16+
const callbackToken = { callbackToken: true };
17+
const awaitToken = { awaitToken: true };
818

919
let i = 0;
10-
process.setUncaughtExceptionCaptureCallback((err) => {
11-
++i;
12-
assert.strictEqual(err.message, 'err2');
13-
assert.strictEqual(asyncLocalStorage.getStore().get('hello'), 'node');
14-
});
15-
16-
try {
17-
asyncLocalStorage.run(new Map(), () => {
18-
const store = asyncLocalStorage.getStore();
19-
store.set('hello', 'node');
20-
setTimeout(() => {
21-
process.nextTick(() => {
22-
assert.strictEqual(i, 1);
23-
});
24-
throw new Error('err2');
25-
}, 0);
26-
throw new Error('err1');
20+
21+
// Redefining the uncaughtExceptionHandler is a bit odd, so we just do this
22+
// so we can track total invocations
23+
let underlyingExceptionHandler;
24+
const exceptionHandler = common.mustCall(function(...args) {
25+
return underlyingExceptionHandler.call(this, ...args);
26+
}, 2);
27+
process.setUncaughtExceptionCaptureCallback(exceptionHandler);
28+
29+
const exceptionMonitor = common.mustCall((err, origin) => {
30+
if (err.message === 'err2') {
31+
assert.strictEqual(origin, 'uncaughtException');
32+
assert.strictEqual(asyncLocalStorage.getStore(), callbackToken);
33+
} else if (err.message === 'err4') {
34+
assert.strictEqual(origin, 'unhandledRejection');
35+
assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
36+
} else {
37+
assert.fail('unknown error ' + err);
38+
}
39+
}, 2);
40+
process.on('uncaughtExceptionMonitor', exceptionMonitor);
41+
42+
function fireErr1() {
43+
underlyingExceptionHandler = common.mustCall(function(err) {
44+
++i;
45+
assert.strictEqual(err.message, 'err2');
46+
assert.strictEqual(asyncLocalStorage.getStore(), callbackToken);
47+
}, 1);
48+
try {
49+
asyncLocalStorage.run(callbackToken, () => {
50+
setTimeout(fireErr2, 0);
51+
throw new Error('err1');
52+
});
53+
} catch (e) {
54+
assert.strictEqual(e.message, 'err1');
55+
assert.strictEqual(asyncLocalStorage.getStore(), undefined);
56+
}
57+
}
58+
59+
function fireErr2() {
60+
process.nextTick(() => {
61+
assert.strictEqual(i, 1);
62+
fireErr3();
2763
});
28-
} catch (e) {
29-
assert.strictEqual(e.message, 'err1');
30-
assert.strictEqual(asyncLocalStorage.getStore(), undefined);
64+
throw new Error('err2');
65+
}
66+
67+
function fireErr3() {
68+
assert.strictEqual(asyncLocalStorage.getStore(), callbackToken);
69+
const rejectionHandler3 = common.mustCall((err) => {
70+
assert.strictEqual(err.message, 'err3');
71+
assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
72+
process.off('unhandledRejection', rejectionHandler3);
73+
74+
fireErr4();
75+
}, 1);
76+
process.on('unhandledRejection', rejectionHandler3);
77+
async function awaitTest() {
78+
await null;
79+
throw new Error('err3');
80+
}
81+
asyncLocalStorage.run(awaitToken, awaitTest);
82+
}
83+
84+
const uncaughtExceptionHandler4 = common.mustCall(
85+
function(err) {
86+
assert.strictEqual(err.message, 'err4');
87+
assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
88+
fireErr5();
89+
}, 1);
90+
function fireErr4() {
91+
assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
92+
underlyingExceptionHandler = uncaughtExceptionHandler4;
93+
// re-entrant check
94+
Promise.reject(new Error('err4'));
3195
}
96+
97+
function fireErr5() {
98+
assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
99+
underlyingExceptionHandler = () => {};
100+
const rejectionHandler5 = common.mustCall((err) => {
101+
assert.strictEqual(err.message, 'err5');
102+
assert.strictEqual(asyncLocalStorage.getStore(), awaitToken);
103+
process.off('unhandledRejection', rejectionHandler5);
104+
}, 1);
105+
process.on('unhandledRejection', rejectionHandler5);
106+
const makeOrphan = vm.compileFunction(`(${String(() => {
107+
async function main() {
108+
await null;
109+
Promise.resolve().then(() => {
110+
throw new Error('err5');
111+
});
112+
}
113+
main();
114+
})})()`);
115+
makeOrphan();
116+
}
117+
118+
fireErr1();

0 commit comments

Comments
 (0)