diff --git a/lib/internal/process/next_tick.js b/lib/internal/process/next_tick.js index 9481bebd224be6..e7fad84397c427 100644 --- a/lib/internal/process/next_tick.js +++ b/lib/internal/process/next_tick.js @@ -9,7 +9,7 @@ function setupNextTick() { const async_hooks = require('internal/async_hooks'); const promises = require('internal/process/promises'); const errors = require('internal/errors'); - const emitPendingUnhandledRejections = promises.setup(scheduleMicrotasks); + const { emitPromiseRejectionWarnings } = promises; const getDefaultTriggerAsyncId = async_hooks.getDefaultTriggerAsyncId; // Two arrays that share state between C++ and JS. const { async_hook_fields, async_id_fields } = async_wrap; @@ -30,6 +30,7 @@ function setupNextTick() { // *Must* match Environment::TickInfo::Fields in src/env.h. const kHasScheduled = 0; + const kHasPromiseRejections = 1; const nextTickQueue = { head: null, @@ -58,8 +59,6 @@ function setupNextTick() { } }; - var microtasksScheduled = false; - process.nextTick = nextTick; // Needs to be accessible from beyond this scope. process._tickCallback = _tickCallback; @@ -67,30 +66,6 @@ function setupNextTick() { // Set the nextTick() function for internal usage. exports.nextTick = internalNextTick; - const microTasksTickObject = { - callback: runMicrotasksCallback, - args: undefined, - [async_id_symbol]: 0, - [trigger_async_id_symbol]: 0 - }; - function scheduleMicrotasks() { - if (microtasksScheduled) - return; - - // For the moment all microtasks come from the void until the PromiseHook - // API is implemented. - nextTickQueue.push(microTasksTickObject); - microtasksScheduled = true; - } - - function runMicrotasksCallback() { - microtasksScheduled = false; - runMicrotasks(); - - if (nextTickQueue.head !== null || emitPendingUnhandledRejections()) - scheduleMicrotasks(); - } - function _tickCallback() { let tock; do { @@ -118,8 +93,8 @@ function setupNextTick() { emitAfter(asyncId); } runMicrotasks(); - emitPendingUnhandledRejections(); - } while (nextTickQueue.head !== null); + } while (nextTickQueue.head !== null || emitPromiseRejectionWarnings()); + tickInfo[kHasPromiseRejections] = 0; } class TickObject { diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 82f15f9f9df51e..a1c7331a0d98ba 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -2,123 +2,109 @@ const { safeToString } = process.binding('util'); -const promiseRejectEvent = process._promiseRejectEvent; -const hasBeenNotifiedProperty = new WeakMap(); -const promiseToGuidProperty = new WeakMap(); +const maybeUnhandledPromises = new WeakMap(); const pendingUnhandledRejections = []; -let lastPromiseId = 1; +const asyncHandledRejections = []; +let lastPromiseId = 0; -exports.setup = setupPromises; +module.exports = { + emitPromiseRejectionWarnings +}; -function getAsynchronousRejectionWarningObject(uid) { - return new Error('Promise rejection was handled ' + - `asynchronously (rejection id: ${uid})`); -} - -function setupPromises(scheduleMicrotasks) { - let deprecationWarned = false; +process._setupPromises(unhandledRejection, handledRejection); - process._setupPromises(function(event, promise, reason) { - if (event === promiseRejectEvent.unhandled) - unhandledRejection(promise, reason); - else if (event === promiseRejectEvent.handled) - rejectionHandled(promise); - else - require('assert').fail(null, null, 'unexpected PromiseRejectEvent'); +function unhandledRejection(promise, reason) { + maybeUnhandledPromises.set(promise, { + reason, + uid: ++lastPromiseId, + warned: false }); + pendingUnhandledRejections.push(promise); + return true; +} - function unhandledRejection(promise, reason) { - hasBeenNotifiedProperty.set(promise, false); - promiseToGuidProperty.set(promise, lastPromiseId++); - addPendingUnhandledRejection(promise, reason); +function handledRejection(promise) { + const promiseInfo = maybeUnhandledPromises.get(promise); + if (promiseInfo !== undefined) { + maybeUnhandledPromises.delete(promise); + if (promiseInfo.warned) { + const { uid } = promiseInfo; + // Generate the warning object early to get a good stack trace. + const warning = new Error('Promise rejection was handled ' + + `asynchronously (rejection id: ${uid})`); + warning.name = 'PromiseRejectionHandledWarning'; + warning.id = uid; + asyncHandledRejections.push({ promise, warning }); + return true; + } } + return false; +} - function rejectionHandled(promise) { - const hasBeenNotified = hasBeenNotifiedProperty.get(promise); - if (hasBeenNotified !== undefined) { - hasBeenNotifiedProperty.delete(promise); - const uid = promiseToGuidProperty.get(promise); - promiseToGuidProperty.delete(promise); - if (hasBeenNotified === true) { - let warning = null; - if (!process.listenerCount('rejectionHandled')) { - // Generate the warning object early to get a good stack trace. - warning = getAsynchronousRejectionWarningObject(uid); - } - process.nextTick(function() { - if (!process.emit('rejectionHandled', promise)) { - if (warning === null) - warning = getAsynchronousRejectionWarningObject(uid); - warning.name = 'PromiseRejectionHandledWarning'; - warning.id = uid; - process.emitWarning(warning); - } - }); - } - +const unhandledRejectionErrName = 'UnhandledPromiseRejectionWarning'; +function emitWarning(uid, reason) { + try { + if (reason instanceof Error) { + process.emitWarning(reason.stack, unhandledRejectionErrName); + } else { + process.emitWarning(safeToString(reason), unhandledRejectionErrName); } + } catch (e) { + // ignored } - function emitWarning(uid, reason) { - try { - if (reason instanceof Error) { - process.emitWarning(reason.stack, 'UnhandledPromiseRejectionWarning'); - } else { - process.emitWarning( - safeToString(reason), 'UnhandledPromiseRejectionWarning' - ); - } - } catch (e) { - // ignored + const warning = new Error( + 'Unhandled promise rejection. This error originated either by ' + + 'throwing inside of an async function without a catch block, ' + + 'or by rejecting a promise which was not handled with .catch(). ' + + `(rejection id: ${uid})` + ); + warning.name = unhandledRejectionErrName; + try { + if (reason instanceof Error) { + warning.stack = reason.stack; } + } catch (err) { + // ignored + } + process.emitWarning(warning); + emitDeprecationWarning(); +} - const warning = new Error( - 'Unhandled promise rejection. This error originated either by ' + - 'throwing inside of an async function without a catch block, ' + - 'or by rejecting a promise which was not handled with .catch(). ' + - `(rejection id: ${uid})` - ); - warning.name = 'UnhandledPromiseRejectionWarning'; - try { - if (reason instanceof Error) { - warning.stack = reason.stack; - } - } catch (err) { - // ignored - } - process.emitWarning(warning); - if (!deprecationWarned) { - deprecationWarned = true; - process.emitWarning( - 'Unhandled promise rejections are deprecated. In the future, ' + - 'promise rejections that are not handled will terminate the ' + - 'Node.js process with a non-zero exit code.', - 'DeprecationWarning', 'DEP0018'); - } +let deprecationWarned = false; +function emitDeprecationWarning() { + if (!deprecationWarned) { + deprecationWarned = true; + process.emitWarning( + 'Unhandled promise rejections are deprecated. In the future, ' + + 'promise rejections that are not handled will terminate the ' + + 'Node.js process with a non-zero exit code.', + 'DeprecationWarning', 'DEP0018'); } +} - function emitPendingUnhandledRejections() { - let hadListeners = false; - while (pendingUnhandledRejections.length > 0) { - const promise = pendingUnhandledRejections.shift(); - const reason = pendingUnhandledRejections.shift(); - if (hasBeenNotifiedProperty.get(promise) === false) { - hasBeenNotifiedProperty.set(promise, true); - const uid = promiseToGuidProperty.get(promise); - if (!process.emit('unhandledRejection', reason, promise)) { - emitWarning(uid, reason); - } else { - hadListeners = true; - } - } +function emitPromiseRejectionWarnings() { + while (asyncHandledRejections.length > 0) { + const { promise, warning } = asyncHandledRejections.shift(); + if (!process.emit('rejectionHandled', promise)) { + process.emitWarning(warning); } - return hadListeners; } - function addPendingUnhandledRejection(promise, reason) { - pendingUnhandledRejections.push(promise, reason); - scheduleMicrotasks(); + let hadListeners = false; + let len = pendingUnhandledRejections.length; + while (len--) { + const promise = pendingUnhandledRejections.shift(); + const promiseInfo = maybeUnhandledPromises.get(promise); + if (promiseInfo !== undefined) { + promiseInfo.warned = true; + const { reason, uid } = promiseInfo; + if (!process.emit('unhandledRejection', reason, promise)) { + emitWarning(uid, reason); + } else { + hadListeners = true; + } + } } - - return emitPendingUnhandledRejections; + return hadListeners || pendingUnhandledRejections.length !== 0; } diff --git a/src/env-inl.h b/src/env-inl.h index 37d1cf172ea14b..0a2adae2bb0ade 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -264,6 +264,14 @@ inline bool Environment::TickInfo::has_scheduled() const { return fields_[kHasScheduled] == 1; } +inline bool Environment::TickInfo::has_promise_rejections() const { + return fields_[kHasPromiseRejections] == 1; +} + +inline void Environment::TickInfo::promise_rejections_toggle_on() { + fields_[kHasPromiseRejections] = 1; +} + inline void Environment::AssignToContext(v8::Local context, const ContextInfo& info) { context->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex, this); diff --git a/src/env.h b/src/env.h index 638753db727136..d4577a67eac921 100644 --- a/src/env.h +++ b/src/env.h @@ -295,7 +295,8 @@ class ModuleWrap; V(performance_entry_callback, v8::Function) \ V(performance_entry_template, v8::Function) \ V(process_object, v8::Object) \ - V(promise_reject_function, v8::Function) \ + V(promise_reject_handled_function, v8::Function) \ + V(promise_reject_unhandled_function, v8::Function) \ V(promise_wrap_template, v8::ObjectTemplate) \ V(push_values_to_array_function, v8::Function) \ V(randombytes_constructor_template, v8::ObjectTemplate) \ @@ -484,6 +485,9 @@ class Environment { public: inline AliasedBuffer& fields(); inline bool has_scheduled() const; + inline bool has_promise_rejections() const; + + inline void promise_rejections_toggle_on(); private: friend class Environment; // So we can call the constructor. @@ -491,6 +495,7 @@ class Environment { enum Fields { kHasScheduled, + kHasPromiseRejections, kFieldsCount }; diff --git a/src/node.cc b/src/node.cc index 9e84dda74a9daa..95be389ce9456d 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1168,19 +1168,33 @@ void SetupNextTick(const FunctionCallbackInfo& args) { void PromiseRejectCallback(PromiseRejectMessage message) { Local promise = message.GetPromise(); Isolate* isolate = promise->GetIsolate(); - Local value = message.GetValue(); - Local event = Integer::New(isolate, message.GetEvent()); + v8::PromiseRejectEvent event = message.GetEvent(); Environment* env = Environment::GetCurrent(isolate); - Local callback = env->promise_reject_function(); + Local callback; + Local value; - if (value.IsEmpty()) + if (event == v8::kPromiseRejectWithNoHandler) { + callback = env->promise_reject_unhandled_function(); + value = message.GetValue(); + + if (value.IsEmpty()) + value = Undefined(isolate); + } else if (event == v8::kPromiseHandlerAddedAfterReject) { + callback = env->promise_reject_handled_function(); value = Undefined(isolate); + } else { + UNREACHABLE(); + } - Local args[] = { event, promise, value }; - Local process = env->process_object(); + Local args[] = { promise, value }; + MaybeLocal ret = callback->Call(env->context(), + Undefined(isolate), + arraysize(args), + args); - callback->Call(process, arraysize(args), args); + if (!ret.IsEmpty() && ret.ToLocalChecked()->IsTrue()) + env->tick_info()->promise_rejections_toggle_on(); } void SetupPromises(const FunctionCallbackInfo& args) { @@ -1188,9 +1202,11 @@ void SetupPromises(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); CHECK(args[0]->IsFunction()); + CHECK(args[1]->IsFunction()); isolate->SetPromiseRejectCallback(PromiseRejectCallback); - env->set_promise_reject_function(args[0].As()); + env->set_promise_reject_unhandled_function(args[0].As()); + env->set_promise_reject_handled_function(args[1].As()); env->process_object()->Delete( env->context(), @@ -1289,7 +1305,7 @@ void InternalCallbackScope::Close() { CHECK_EQ(env_->trigger_async_id(), 0); } - if (!tick_info->has_scheduled()) { + if (!tick_info->has_scheduled() && !tick_info->has_promise_rejections()) { return; } @@ -3260,20 +3276,6 @@ void SetupProcessObject(Environment* env, "napi", FIXED_ONE_BYTE_STRING(env->isolate(), node_napi_version)); - // process._promiseRejectEvent - Local promiseRejectEvent = Object::New(env->isolate()); - READONLY_DONT_ENUM_PROPERTY(process, - "_promiseRejectEvent", - promiseRejectEvent); - READONLY_PROPERTY(promiseRejectEvent, - "unhandled", - Integer::New(env->isolate(), - v8::kPromiseRejectWithNoHandler)); - READONLY_PROPERTY(promiseRejectEvent, - "handled", - Integer::New(env->isolate(), - v8::kPromiseHandlerAddedAfterReject)); - #if HAVE_OPENSSL // Stupid code to slice out the version string. { // NOLINT(whitespace/braces) diff --git a/test/message/unhandled_promise_trace_warnings.out b/test/message/unhandled_promise_trace_warnings.out index c9c7a5c8700b26..069e6cc678c0da 100644 --- a/test/message/unhandled_promise_trace_warnings.out +++ b/test/message/unhandled_promise_trace_warnings.out @@ -14,7 +14,6 @@ at * at * at * - at * (node:*) Error: This was rejected at * (*test*message*unhandled_promise_trace_warnings.js:*) at * @@ -34,9 +33,7 @@ at * at * (node:*) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1) - at getAsynchronousRejectionWarningObject (internal/process/promises.js:*) - at rejectionHandled (internal/process/promises.js:*) - at * + at handledRejection (internal/process/promises.js:*) at Promise.catch * at Immediate.setImmediate (*test*message*unhandled_promise_trace_warnings.js:*) at * diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js index ed5d485ac72e96..1ade061994c3bb 100644 --- a/test/parallel/test-promises-unhandled-rejections.js +++ b/test/parallel/test-promises-unhandled-rejections.js @@ -684,3 +684,16 @@ asyncTest('Throwing an error inside a rejectionHandled handler goes to' + } }, 1); }); + +asyncTest('Rejected promise inside unhandledRejection allows nextTick loop' + + ' to proceed first', function(done) { + clean(); + Promise.reject(0); + let didCall = false; + process.on('unhandledRejection', () => { + assert(!didCall); + didCall = true; + const promise = Promise.reject(0); + process.nextTick(() => promise.catch(() => done())); + }); +});