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 b4c933d

Browse filesBrowse files
apapirovskiaddaleax
apapirovski
authored and
addaleax
committed
promises: refactor rejection handling
Remove the unnecessary microTasksTickObject for scheduling microtasks and instead use TickInfo to keep track of whether promise rejections exist that need to be emitted. Consequently allow the microtasks to execute on average fewer times, in more predictable manner than previously. Simplify unhandled & handled rejection tracking to do more in C++ to avoid needing to expose additional info in JS. When new unhandledRejections are emitted within an unhandledRejection handler, allow the event loop to proceed first instead. This means that if the end-user code handles all promise rejections on nextTick, rejections within unhandledRejection now won't spiral into an infinite loop. PR-URL: #18207 Fixes: #17913 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 1be5e33 commit b4c933d
Copy full SHA for b4c933d

File tree

Expand file treeCollapse file tree

7 files changed

+143
-157
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+143
-157
lines changed
Open diff view settings
Collapse file

‎lib/internal/process/next_tick.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/next_tick.js
+4-29Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ function setupNextTick() {
99
const async_hooks = require('internal/async_hooks');
1010
const promises = require('internal/process/promises');
1111
const errors = require('internal/errors');
12-
const emitPendingUnhandledRejections = promises.setup(scheduleMicrotasks);
12+
const { emitPromiseRejectionWarnings } = promises;
1313
const getDefaultTriggerAsyncId = async_hooks.getDefaultTriggerAsyncId;
1414
// Two arrays that share state between C++ and JS.
1515
const { async_hook_fields, async_id_fields } = async_wrap;
@@ -30,6 +30,7 @@ function setupNextTick() {
3030

3131
// *Must* match Environment::TickInfo::Fields in src/env.h.
3232
const kHasScheduled = 0;
33+
const kHasPromiseRejections = 1;
3334

3435
const nextTickQueue = {
3536
head: null,
@@ -58,39 +59,13 @@ function setupNextTick() {
5859
}
5960
};
6061

61-
var microtasksScheduled = false;
62-
6362
process.nextTick = nextTick;
6463
// Needs to be accessible from beyond this scope.
6564
process._tickCallback = _tickCallback;
6665

6766
// Set the nextTick() function for internal usage.
6867
exports.nextTick = internalNextTick;
6968

70-
const microTasksTickObject = {
71-
callback: runMicrotasksCallback,
72-
args: undefined,
73-
[async_id_symbol]: 0,
74-
[trigger_async_id_symbol]: 0
75-
};
76-
function scheduleMicrotasks() {
77-
if (microtasksScheduled)
78-
return;
79-
80-
// For the moment all microtasks come from the void until the PromiseHook
81-
// API is implemented.
82-
nextTickQueue.push(microTasksTickObject);
83-
microtasksScheduled = true;
84-
}
85-
86-
function runMicrotasksCallback() {
87-
microtasksScheduled = false;
88-
runMicrotasks();
89-
90-
if (nextTickQueue.head !== null || emitPendingUnhandledRejections())
91-
scheduleMicrotasks();
92-
}
93-
9469
function _tickCallback() {
9570
let tock;
9671
do {
@@ -118,8 +93,8 @@ function setupNextTick() {
11893
emitAfter(asyncId);
11994
}
12095
runMicrotasks();
121-
emitPendingUnhandledRejections();
122-
} while (nextTickQueue.head !== null);
96+
} while (nextTickQueue.head !== null || emitPromiseRejectionWarnings());
97+
tickInfo[kHasPromiseRejections] = 0;
12398
}
12499

125100
class TickObject {
Collapse file

‎lib/internal/process/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/promises.js
+86-100Lines changed: 86 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -2,123 +2,109 @@
22

33
const { safeToString } = process.binding('util');
44

5-
const promiseRejectEvent = process._promiseRejectEvent;
6-
const hasBeenNotifiedProperty = new WeakMap();
7-
const promiseToGuidProperty = new WeakMap();
5+
const maybeUnhandledPromises = new WeakMap();
86
const pendingUnhandledRejections = [];
9-
let lastPromiseId = 1;
7+
const asyncHandledRejections = [];
8+
let lastPromiseId = 0;
109

11-
exports.setup = setupPromises;
10+
module.exports = {
11+
emitPromiseRejectionWarnings
12+
};
1213

13-
function getAsynchronousRejectionWarningObject(uid) {
14-
return new Error('Promise rejection was handled ' +
15-
`asynchronously (rejection id: ${uid})`);
16-
}
17-
18-
function setupPromises(scheduleMicrotasks) {
19-
let deprecationWarned = false;
14+
process._setupPromises(unhandledRejection, handledRejection);
2015

21-
process._setupPromises(function(event, promise, reason) {
22-
if (event === promiseRejectEvent.unhandled)
23-
unhandledRejection(promise, reason);
24-
else if (event === promiseRejectEvent.handled)
25-
rejectionHandled(promise);
26-
else
27-
require('assert').fail(null, null, 'unexpected PromiseRejectEvent');
16+
function unhandledRejection(promise, reason) {
17+
maybeUnhandledPromises.set(promise, {
18+
reason,
19+
uid: ++lastPromiseId,
20+
warned: false
2821
});
22+
pendingUnhandledRejections.push(promise);
23+
return true;
24+
}
2925

30-
function unhandledRejection(promise, reason) {
31-
hasBeenNotifiedProperty.set(promise, false);
32-
promiseToGuidProperty.set(promise, lastPromiseId++);
33-
addPendingUnhandledRejection(promise, reason);
26+
function handledRejection(promise) {
27+
const promiseInfo = maybeUnhandledPromises.get(promise);
28+
if (promiseInfo !== undefined) {
29+
maybeUnhandledPromises.delete(promise);
30+
if (promiseInfo.warned) {
31+
const { uid } = promiseInfo;
32+
// Generate the warning object early to get a good stack trace.
33+
const warning = new Error('Promise rejection was handled ' +
34+
`asynchronously (rejection id: ${uid})`);
35+
warning.name = 'PromiseRejectionHandledWarning';
36+
warning.id = uid;
37+
asyncHandledRejections.push({ promise, warning });
38+
return true;
39+
}
3440
}
41+
return false;
42+
}
3543

36-
function rejectionHandled(promise) {
37-
const hasBeenNotified = hasBeenNotifiedProperty.get(promise);
38-
if (hasBeenNotified !== undefined) {
39-
hasBeenNotifiedProperty.delete(promise);
40-
const uid = promiseToGuidProperty.get(promise);
41-
promiseToGuidProperty.delete(promise);
42-
if (hasBeenNotified === true) {
43-
let warning = null;
44-
if (!process.listenerCount('rejectionHandled')) {
45-
// Generate the warning object early to get a good stack trace.
46-
warning = getAsynchronousRejectionWarningObject(uid);
47-
}
48-
process.nextTick(function() {
49-
if (!process.emit('rejectionHandled', promise)) {
50-
if (warning === null)
51-
warning = getAsynchronousRejectionWarningObject(uid);
52-
warning.name = 'PromiseRejectionHandledWarning';
53-
warning.id = uid;
54-
process.emitWarning(warning);
55-
}
56-
});
57-
}
58-
44+
const unhandledRejectionErrName = 'UnhandledPromiseRejectionWarning';
45+
function emitWarning(uid, reason) {
46+
try {
47+
if (reason instanceof Error) {
48+
process.emitWarning(reason.stack, unhandledRejectionErrName);
49+
} else {
50+
process.emitWarning(safeToString(reason), unhandledRejectionErrName);
5951
}
52+
} catch (e) {
53+
// ignored
6054
}
6155

62-
function emitWarning(uid, reason) {
63-
try {
64-
if (reason instanceof Error) {
65-
process.emitWarning(reason.stack, 'UnhandledPromiseRejectionWarning');
66-
} else {
67-
process.emitWarning(
68-
safeToString(reason), 'UnhandledPromiseRejectionWarning'
69-
);
70-
}
71-
} catch (e) {
72-
// ignored
56+
const warning = new Error(
57+
'Unhandled promise rejection. This error originated either by ' +
58+
'throwing inside of an async function without a catch block, ' +
59+
'or by rejecting a promise which was not handled with .catch(). ' +
60+
`(rejection id: ${uid})`
61+
);
62+
warning.name = unhandledRejectionErrName;
63+
try {
64+
if (reason instanceof Error) {
65+
warning.stack = reason.stack;
7366
}
67+
} catch (err) {
68+
// ignored
69+
}
70+
process.emitWarning(warning);
71+
emitDeprecationWarning();
72+
}
7473

75-
const warning = new Error(
76-
'Unhandled promise rejection. This error originated either by ' +
77-
'throwing inside of an async function without a catch block, ' +
78-
'or by rejecting a promise which was not handled with .catch(). ' +
79-
`(rejection id: ${uid})`
80-
);
81-
warning.name = 'UnhandledPromiseRejectionWarning';
82-
try {
83-
if (reason instanceof Error) {
84-
warning.stack = reason.stack;
85-
}
86-
} catch (err) {
87-
// ignored
88-
}
89-
process.emitWarning(warning);
90-
if (!deprecationWarned) {
91-
deprecationWarned = true;
92-
process.emitWarning(
93-
'Unhandled promise rejections are deprecated. In the future, ' +
94-
'promise rejections that are not handled will terminate the ' +
95-
'Node.js process with a non-zero exit code.',
96-
'DeprecationWarning', 'DEP0018');
97-
}
74+
let deprecationWarned = false;
75+
function emitDeprecationWarning() {
76+
if (!deprecationWarned) {
77+
deprecationWarned = true;
78+
process.emitWarning(
79+
'Unhandled promise rejections are deprecated. In the future, ' +
80+
'promise rejections that are not handled will terminate the ' +
81+
'Node.js process with a non-zero exit code.',
82+
'DeprecationWarning', 'DEP0018');
9883
}
84+
}
9985

100-
function emitPendingUnhandledRejections() {
101-
let hadListeners = false;
102-
while (pendingUnhandledRejections.length > 0) {
103-
const promise = pendingUnhandledRejections.shift();
104-
const reason = pendingUnhandledRejections.shift();
105-
if (hasBeenNotifiedProperty.get(promise) === false) {
106-
hasBeenNotifiedProperty.set(promise, true);
107-
const uid = promiseToGuidProperty.get(promise);
108-
if (!process.emit('unhandledRejection', reason, promise)) {
109-
emitWarning(uid, reason);
110-
} else {
111-
hadListeners = true;
112-
}
113-
}
86+
function emitPromiseRejectionWarnings() {
87+
while (asyncHandledRejections.length > 0) {
88+
const { promise, warning } = asyncHandledRejections.shift();
89+
if (!process.emit('rejectionHandled', promise)) {
90+
process.emitWarning(warning);
11491
}
115-
return hadListeners;
11692
}
11793

118-
function addPendingUnhandledRejection(promise, reason) {
119-
pendingUnhandledRejections.push(promise, reason);
120-
scheduleMicrotasks();
94+
let hadListeners = false;
95+
let len = pendingUnhandledRejections.length;
96+
while (len--) {
97+
const promise = pendingUnhandledRejections.shift();
98+
const promiseInfo = maybeUnhandledPromises.get(promise);
99+
if (promiseInfo !== undefined) {
100+
promiseInfo.warned = true;
101+
const { reason, uid } = promiseInfo;
102+
if (!process.emit('unhandledRejection', reason, promise)) {
103+
emitWarning(uid, reason);
104+
} else {
105+
hadListeners = true;
106+
}
107+
}
121108
}
122-
123-
return emitPendingUnhandledRejections;
109+
return hadListeners || pendingUnhandledRejections.length !== 0;
124110
}
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,14 @@ inline bool Environment::TickInfo::has_scheduled() const {
264264
return fields_[kHasScheduled] == 1;
265265
}
266266

267+
inline bool Environment::TickInfo::has_promise_rejections() const {
268+
return fields_[kHasPromiseRejections] == 1;
269+
}
270+
271+
inline void Environment::TickInfo::promise_rejections_toggle_on() {
272+
fields_[kHasPromiseRejections] = 1;
273+
}
274+
267275
inline void Environment::AssignToContext(v8::Local<v8::Context> context,
268276
const ContextInfo& info) {
269277
context->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex, this);
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,8 @@ class ModuleWrap;
295295
V(performance_entry_callback, v8::Function) \
296296
V(performance_entry_template, v8::Function) \
297297
V(process_object, v8::Object) \
298-
V(promise_reject_function, v8::Function) \
298+
V(promise_reject_handled_function, v8::Function) \
299+
V(promise_reject_unhandled_function, v8::Function) \
299300
V(promise_wrap_template, v8::ObjectTemplate) \
300301
V(push_values_to_array_function, v8::Function) \
301302
V(randombytes_constructor_template, v8::ObjectTemplate) \
@@ -484,13 +485,17 @@ class Environment {
484485
public:
485486
inline AliasedBuffer<uint8_t, v8::Uint8Array>& fields();
486487
inline bool has_scheduled() const;
488+
inline bool has_promise_rejections() const;
489+
490+
inline void promise_rejections_toggle_on();
487491

488492
private:
489493
friend class Environment; // So we can call the constructor.
490494
inline explicit TickInfo(v8::Isolate* isolate);
491495

492496
enum Fields {
493497
kHasScheduled,
498+
kHasPromiseRejections,
494499
kFieldsCount
495500
};
496501

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.