Skip to content

Commit d0f1ff5

Browse files
Sajal Khandelwaldanielleadams
Sajal Khandelwal
authored andcommitted
async_hooks: set unhandledRejection async context
This commit now executes `process.on('unhandledRejection')` in the async execution context of the concerned `Promise`. PR-URL: #37281 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent 1fc8307 commit d0f1ff5

File tree

5 files changed

+130
-11
lines changed

5 files changed

+130
-11
lines changed

lib/internal/async_hooks.js

+2
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,8 @@ module.exports = {
573573
emitBefore: emitBeforeScript,
574574
emitAfter: emitAfterScript,
575575
emitDestroy: emitDestroyScript,
576+
pushAsyncContext,
577+
popAsyncContext,
576578
registerDestroyHook,
577579
useDomainTrampoline,
578580
nativeHooks: {

lib/internal/process/promises.js

+26-8
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ const {
2424
triggerUncaughtException
2525
} = internalBinding('errors');
2626

27+
const {
28+
pushAsyncContext,
29+
popAsyncContext,
30+
} = require('internal/async_hooks');
31+
const async_hooks = require('async_hooks');
32+
2733
// *Must* match Environment::TickInfo::Fields in src/env.h.
2834
const kHasRejectionToWarn = 1;
2935

@@ -116,11 +122,28 @@ function resolveError(type, promise, reason) {
116122
}
117123

118124
function unhandledRejection(promise, reason) {
125+
const asyncId = async_hooks.executionAsyncId();
126+
const triggerAsyncId = async_hooks.triggerAsyncId();
127+
const resource = promise;
128+
129+
const emit = (reason, promise, promiseInfo) => {
130+
try {
131+
pushAsyncContext(asyncId, triggerAsyncId, resource);
132+
if (promiseInfo.domain) {
133+
return promiseInfo.domain.emit('error', reason);
134+
}
135+
return process.emit('unhandledRejection', reason, promise);
136+
} finally {
137+
popAsyncContext(asyncId);
138+
}
139+
};
140+
119141
maybeUnhandledPromises.set(promise, {
120142
reason,
121143
uid: ++lastPromiseId,
122144
warned: false,
123-
domain: process.domain
145+
domain: process.domain,
146+
emit
124147
});
125148
// This causes the promise to be referenced at least for one tick.
126149
ArrayPrototypePush(pendingUnhandledRejections, promise);
@@ -194,13 +217,8 @@ function processPromiseRejections() {
194217
continue;
195218
}
196219
promiseInfo.warned = true;
197-
const { reason, uid } = promiseInfo;
198-
function emit(reason, promise, promiseInfo) {
199-
if (promiseInfo.domain) {
200-
return promiseInfo.domain.emit('error', reason);
201-
}
202-
return process.emit('unhandledRejection', reason, promise);
203-
}
220+
const { reason, uid, emit } = promiseInfo;
221+
204222
switch (unhandledRejectionsMode) {
205223
case kStrictUnhandledRejections: {
206224
const err = reason instanceof Error ?

src/node_task_queue.cc

+74-3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include "async_wrap.h"
12
#include "env-inl.h"
23
#include "node.h"
34
#include "node_errors.h"
@@ -16,18 +17,54 @@ using v8::Context;
1617
using v8::Function;
1718
using v8::FunctionCallbackInfo;
1819
using v8::Isolate;
20+
using v8::Just;
1921
using v8::kPromiseHandlerAddedAfterReject;
2022
using v8::kPromiseRejectAfterResolved;
2123
using v8::kPromiseRejectWithNoHandler;
2224
using v8::kPromiseResolveAfterResolved;
2325
using v8::Local;
26+
using v8::Maybe;
2427
using v8::Number;
2528
using v8::Object;
2629
using v8::Promise;
2730
using v8::PromiseRejectEvent;
2831
using v8::PromiseRejectMessage;
2932
using v8::Value;
3033

34+
static Maybe<double> GetAssignedPromiseAsyncId(Environment* env,
35+
Local<Promise> promise,
36+
Local<Value> id_symbol) {
37+
Local<Value> maybe_async_id;
38+
if (!promise->Get(env->context(), id_symbol).ToLocal(&maybe_async_id)) {
39+
return v8::Just(AsyncWrap::kInvalidAsyncId);
40+
}
41+
return maybe_async_id->IsNumber()
42+
? maybe_async_id->NumberValue(env->context())
43+
: v8::Just(AsyncWrap::kInvalidAsyncId);
44+
}
45+
46+
static Maybe<double> GetAssignedPromiseWrapAsyncId(Environment* env,
47+
Local<Promise> promise,
48+
Local<Value> id_symbol) {
49+
// This check is imperfect. If the internal field is set, it should
50+
// be an object. If it's not, we just ignore it. Ideally v8 would
51+
// have had GetInternalField returning a MaybeLocal but this works
52+
// for now.
53+
Local<Value> promiseWrap = promise->GetInternalField(0);
54+
if (promiseWrap->IsObject()) {
55+
Local<Value> maybe_async_id;
56+
if (!promiseWrap.As<Object>()->Get(env->context(), id_symbol)
57+
.ToLocal(&maybe_async_id)) {
58+
return v8::Just(AsyncWrap::kInvalidAsyncId);
59+
}
60+
return maybe_async_id->IsNumber()
61+
? maybe_async_id->NumberValue(env->context())
62+
: v8::Just(AsyncWrap::kInvalidAsyncId);
63+
} else {
64+
return v8::Just(AsyncWrap::kInvalidAsyncId);
65+
}
66+
}
67+
3168
void PromiseRejectCallback(PromiseRejectMessage message) {
3269
static std::atomic<uint64_t> unhandledRejections{0};
3370
static std::atomic<uint64_t> rejectionsHandledAfter{0};
@@ -76,12 +113,46 @@ void PromiseRejectCallback(PromiseRejectMessage message) {
76113

77114
Local<Value> args[] = { type, promise, value };
78115

79-
// V8 does not expect this callback to have a scheduled exceptions once it
80-
// returns, so we print them out in a best effort to do something about it
81-
// without failing silently and without crashing the process.
116+
double async_id = AsyncWrap::kInvalidAsyncId;
117+
double trigger_async_id = AsyncWrap::kInvalidAsyncId;
82118
TryCatchScope try_catch(env);
119+
120+
if (!GetAssignedPromiseAsyncId(env, promise, env->async_id_symbol())
121+
.To(&async_id)) return;
122+
if (!GetAssignedPromiseAsyncId(env, promise, env->trigger_async_id_symbol())
123+
.To(&trigger_async_id)) return;
124+
125+
if (async_id == AsyncWrap::kInvalidAsyncId &&
126+
trigger_async_id == AsyncWrap::kInvalidAsyncId) {
127+
// That means that promise might be a PromiseWrap, so we'll
128+
// check there as well.
129+
if (!GetAssignedPromiseWrapAsyncId(env, promise, env->async_id_symbol())
130+
.To(&async_id)) return;
131+
if (!GetAssignedPromiseWrapAsyncId(
132+
env, promise, env->trigger_async_id_symbol())
133+
.To(&trigger_async_id)) return;
134+
}
135+
136+
if (async_id != AsyncWrap::kInvalidAsyncId &&
137+
trigger_async_id != AsyncWrap::kInvalidAsyncId) {
138+
env->async_hooks()->push_async_context(
139+
async_id, trigger_async_id, promise);
140+
}
141+
83142
USE(callback->Call(
84143
env->context(), Undefined(isolate), arraysize(args), args));
144+
145+
if (async_id != AsyncWrap::kInvalidAsyncId &&
146+
trigger_async_id != AsyncWrap::kInvalidAsyncId &&
147+
env->execution_async_id() == async_id) {
148+
// This condition might not be true if async_hooks was enabled during
149+
// the promise callback execution.
150+
env->async_hooks()->pop_async_context(async_id);
151+
}
152+
153+
// V8 does not expect this callback to have a scheduled exceptions once it
154+
// returns, so we print them out in a best effort to do something about it
155+
// without failing silently and without crashing the process.
85156
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
86157
fprintf(stderr, "Exception in PromiseRejectCallback:\n");
87158
PrintCaughtException(isolate, env->context(), try_catch);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
const assert = require('assert');
6+
const initHooks = require('./init-hooks');
7+
const async_hooks = require('async_hooks');
8+
9+
if (!common.isMainThread)
10+
common.skip('Worker bootstrapping works differently -> different async IDs');
11+
12+
const promiseAsyncIds = [];
13+
const hooks = initHooks({
14+
oninit(asyncId, type) {
15+
if (type === 'PROMISE') {
16+
promiseAsyncIds.push(asyncId);
17+
}
18+
}
19+
});
20+
21+
hooks.enable();
22+
Promise.reject();
23+
24+
process.on('unhandledRejection', common.mustCall(() => {
25+
assert.strictEqual(promiseAsyncIds.length, 1);
26+
assert.strictEqual(async_hooks.executionAsyncId(), promiseAsyncIds[0]);
27+
}));

test/parallel/test-bootstrap-modules.js

+1
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ const expectedModules = new Set([
102102
'NativeModule internal/worker/io',
103103
'NativeModule internal/worker/js_transferable',
104104
'NativeModule internal/blob',
105+
'NativeModule async_hooks',
105106
'NativeModule path',
106107
'NativeModule stream',
107108
'NativeModule timers',

0 commit comments

Comments
 (0)