Skip to content

Commit 8699a07

Browse files
author
Sajal Khandelwal
committed
async_hooks: execute unhandledRejection cb in Promise execution context
This commit now executes `process.on('unhandledRejection')` in the async execution context of the concerned `Promise`.
1 parent 907d6b6 commit 8699a07

File tree

4 files changed

+132
-11
lines changed

4 files changed

+132
-11
lines changed

lib/internal/async_hooks.js

Lines changed: 2 additions & 0 deletions
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

Lines changed: 26 additions & 8 deletions
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

Lines changed: 68 additions & 3 deletions
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,55 @@ 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;
28+
using v8::Nothing;
2529
using v8::Object;
2630
using v8::Promise;
2731
using v8::PromiseRejectEvent;
2832
using v8::PromiseRejectMessage;
2933
using v8::Value;
3034

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

77115
Local<Value> args[] = { type, promise, value };
78116

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.
117+
double async_id = AsyncWrap::kInvalidAsyncId;
118+
double trigger_async_id = AsyncWrap::kInvalidAsyncId;
119+
GetAssignedPromiseAsyncId(env, promise, env->async_id_symbol())
120+
.To(&async_id);
121+
GetAssignedPromiseAsyncId(env, promise, env->trigger_async_id_symbol())
122+
.To(&trigger_async_id);
123+
124+
GetAssignedPromiseWrapAsyncId(env, promise, env->async_id_symbol())
125+
.To(&async_id);
126+
GetAssignedPromiseWrapAsyncId(env, promise, env->trigger_async_id_symbol())
127+
.To(&trigger_async_id);
128+
129+
if (async_id != AsyncWrap::kInvalidAsyncId &&
130+
trigger_async_id != AsyncWrap::kInvalidAsyncId) {
131+
env->async_hooks()->push_async_context(
132+
async_id, trigger_async_id, promise);
133+
}
134+
82135
TryCatchScope try_catch(env);
83136
USE(callback->Call(
84137
env->context(), Undefined(isolate), arraysize(args), args));
138+
139+
if (async_id != AsyncWrap::kInvalidAsyncId &&
140+
trigger_async_id != AsyncWrap::kInvalidAsyncId &&
141+
env->execution_async_id() == async_id) {
142+
// This condition might not be true if async_hooks was enabled during
143+
// the promise callback execution.
144+
env->async_hooks()->pop_async_context(async_id);
145+
}
146+
147+
// V8 does not expect this callback to have a scheduled exceptions once it
148+
// returns, so we print them out in a best effort to do something about it
149+
// without failing silently and without crashing the process.
85150
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
86151
fprintf(stderr, "Exception in PromiseRejectCallback:\n");
87152
PrintCaughtException(isolate, env->context(), try_catch);
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
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+
23+
// Promise.resolve()
24+
// .then(() => {
25+
// throw new Error('Throwing on purpose');
26+
// });
27+
28+
Promise.reject();
29+
30+
process.on('unhandledRejection', common.mustCall(() => {
31+
// assert.strictEqual(promiseAsyncIds.length, 2);
32+
// assert.strictEqual(async_hooks.executionAsyncId(), promiseAsyncIds[1]);
33+
34+
assert.strictEqual(promiseAsyncIds.length, 1);
35+
assert.strictEqual(async_hooks.executionAsyncId(), promiseAsyncIds[0]);
36+
}));

0 commit comments

Comments
 (0)