Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit d0f1ff5

Browse filesBrowse files
Sajal Khandelwaldanielleadams
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 <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 1fc8307 commit d0f1ff5
Copy full SHA for d0f1ff5

File tree

Expand file treeCollapse file tree

5 files changed

+130
-11
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+130
-11
lines changed
Open diff view settings
Collapse file

‎lib/internal/async_hooks.js‎

Copy file name to clipboardExpand all lines: lib/internal/async_hooks.js
+2Lines 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: {
Collapse file

‎lib/internal/process/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/promises.js
+26-8Lines 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 ?
Collapse file

‎src/node_task_queue.cc‎

Copy file name to clipboardExpand all lines: src/node_task_queue.cc
+74-3Lines changed: 74 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,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);
Collapse file
+27Lines changed: 27 additions & 0 deletions
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+
}));
Collapse file

‎test/parallel/test-bootstrap-modules.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-bootstrap-modules.js
+1Lines changed: 1 addition & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.