diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 038e71502bdb1a..105f5718800bda 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -304,13 +304,13 @@ function setupProcessFatal() { - process._fatalException = function(er) { + process._fatalException = function(er, fromPromise) { var caught; if (process.domain && process.domain._errorHandler) caught = process.domain._errorHandler(er) || caught; - if (!caught) + if (!caught && !fromPromise) caught = process.emit('uncaughtException', er); // If someone handled it, then great. otherwise, die in C++ land diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 0e382d11d5523b..fc5b922b29d086 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -2,17 +2,10 @@ const promiseRejectEvent = process._promiseRejectEvent; const hasBeenNotifiedProperty = new WeakMap(); -const promiseToGuidProperty = new WeakMap(); const pendingUnhandledRejections = []; -let lastPromiseId = 1; exports.setup = setupPromises; -function getAsynchronousRejectionWarningObject(uid) { - return new Error('Promise rejection was handled ' + - `asynchronously (rejection id: ${uid})`); -} - function setupPromises(scheduleMicrotasks) { process._setupPromises(function(event, promise, reason) { if (event === promiseRejectEvent.unhandled) @@ -25,7 +18,6 @@ function setupPromises(scheduleMicrotasks) { function unhandledRejection(promise, reason) { hasBeenNotifiedProperty.set(promise, false); - promiseToGuidProperty.set(promise, lastPromiseId++); addPendingUnhandledRejection(promise, reason); } @@ -33,47 +25,15 @@ function setupPromises(scheduleMicrotasks) { 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); - } + process.emit('rejectionHandled', promise); }); } } } - function emitWarning(uid, reason) { - const warning = new Error('Unhandled promise rejection ' + - `(rejection id: ${uid}): ${reason}`); - warning.name = 'UnhandledPromiseRejectionWarning'; - warning.id = uid; - if (reason instanceof Error) { - warning.stack = reason.stack; - } - 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'); - } - } - var deprecationWarned = false; function emitPendingUnhandledRejections() { let hadListeners = false; while (pendingUnhandledRejections.length > 0) { @@ -81,9 +41,8 @@ function setupPromises(scheduleMicrotasks) { 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); + process.promiseFatal(promise); } else { hadListeners = true; } diff --git a/src/node.cc b/src/node.cc index ed46dd3cb3a2f7..336bac2660e79f 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1286,6 +1286,21 @@ void SetupPromises(const FunctionCallbackInfo& args) { FIXED_ONE_BYTE_STRING(args.GetIsolate(), "_setupPromises")).FromJust(); } +void PromiseFatal(const FunctionCallbackInfo& args) { + CHECK(args[0]->IsPromise()); + + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + + Local promise = args[0].As(); + + CHECK(promise->State() == Promise::PromiseState::kRejected); + Local err = promise->Result(); + Local message = Exception::CreateMessage(isolate, err); + + InternalFatalException(isolate, err, message, true); +} + } // anonymous namespace @@ -1719,10 +1734,9 @@ void AppendExceptionLine(Environment* env, arrow_str).FromMaybe(false)); } - -static void ReportException(Environment* env, - Local er, - Local message) { +void ReportException(Environment* env, + Local er, + Local message) { HandleScope scope(env->isolate()); AppendExceptionLine(env, er, message, FATAL_ERROR); @@ -2615,6 +2629,14 @@ NO_RETURN void FatalError(const char* location, const char* message) { void FatalException(Isolate* isolate, Local error, Local message) { + InternalFatalException(isolate, error, message, false); +} + + +void InternalFatalException(Isolate* isolate, + Local error, + Local message, + bool from_promise) { HandleScope scope(isolate); Environment* env = Environment::GetCurrent(isolate); @@ -2637,9 +2659,12 @@ void FatalException(Isolate* isolate, // Do not call FatalException when _fatalException handler throws fatal_try_catch.SetVerbose(false); + Local argv[2] = { error, + Boolean::New(env->isolate(), from_promise) }; + // this will return true if the JS layer handled it, false otherwise Local caught = - fatal_exception_function->Call(process_object, 1, &error); + fatal_exception_function->Call(process_object, 2, argv); if (fatal_try_catch.HasCaught()) { // the fatal exception function threw, so we must exit @@ -3495,6 +3520,8 @@ void SetupProcessObject(Environment* env, env->SetMethod(process, "_setupPromises", SetupPromises); env->SetMethod(process, "_setupDomainUse", SetupDomainUse); + env->SetMethod(process, "promiseFatal", PromiseFatal); + // pre-set _events object for faster emit checks Local events_obj = Object::New(env->isolate()); CHECK(events_obj->SetPrototype(env->context(), diff --git a/src/node_internals.h b/src/node_internals.h index e07cb9d6d39a41..be61fe315225ed 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -168,6 +168,11 @@ void AppendExceptionLine(Environment* env, v8::Local message, enum ErrorHandlingMode mode); +void InternalFatalException(v8::Isolate* isolate, + v8::Local error, + v8::Local message, + bool from_promise); + NO_RETURN void FatalError(const char* location, const char* message); void ProcessEmitWarning(Environment* env, const char* fmt, ...); diff --git a/test/message/promise_fast_handled_reject.js b/test/message/promise_fast_handled_reject.js new file mode 100644 index 00000000000000..09c316d1201423 --- /dev/null +++ b/test/message/promise_fast_handled_reject.js @@ -0,0 +1,26 @@ +'use strict'; +const common = require('../common'); + +const p1 = new Promise((res, rej) => { + consol.log('One'); // eslint-disable-line no-undef +}); + +const p2 = new Promise((res, rej) => { // eslint-disable-line no-unused-vars + consol.log('Two'); // eslint-disable-line no-undef +}); + +const p3 = new Promise((res, rej) => { + consol.log('Three'); // eslint-disable-line no-undef +}); + +new Promise((res, rej) => { + setTimeout(common.mustCall(() => { + p1.catch(() => {}); + p3.catch(() => {}); + }), 1); +}); + +process.on('uncaughtException', (err) => + common.fail('Should not trigger uncaught exception')); + +process.on('exit', () => process._rawDebug('exit event emitted')); diff --git a/test/message/promise_fast_handled_reject.out b/test/message/promise_fast_handled_reject.out new file mode 100644 index 00000000000000..f1dcc5218b3cf4 --- /dev/null +++ b/test/message/promise_fast_handled_reject.out @@ -0,0 +1,16 @@ +exit event emitted +*test*message*promise_fast_handled_reject.js:* + consol.log('One'); // eslint-disable-line no-undef + ^ + +ReferenceError: consol is not defined + at Promise (*test*message*promise_fast_handled_reject.js:*:*) + at Promise () + at Object. (*test*message*promise_fast_handled_reject.js:*:*) + at Module._compile (module.js:*:*) + at Object.Module._extensions..js (module.js:*:*) + at Module.load (module.js:*:*) + at tryModuleLoad (module.js:*:*) + at Function.Module._load (module.js:*:*) + at Function.Module.runMain (module.js:*:*) + at startup (bootstrap_node.js:*:*) diff --git a/test/message/promise_fast_reject.js b/test/message/promise_fast_reject.js new file mode 100644 index 00000000000000..cafc2a27b353d5 --- /dev/null +++ b/test/message/promise_fast_reject.js @@ -0,0 +1,18 @@ +'use strict'; + +// We should always have the stacktrace of the oldest rejection. + +const common = require('../common'); + +new Promise(function(res, rej) { + consol.log('One'); // eslint-disable-line no-undef +}); + +new Promise(function(res, rej) { + consol.log('Two'); // eslint-disable-line no-undef +}); + +process.on('uncaughtException', (err) => + common.fail('Should not trigger uncaught exception')); + +process.on('exit', () => process._rawDebug('exit event emitted')); diff --git a/test/message/promise_fast_reject.out b/test/message/promise_fast_reject.out new file mode 100644 index 00000000000000..90fe2c93efa6dc --- /dev/null +++ b/test/message/promise_fast_reject.out @@ -0,0 +1,16 @@ +exit event emitted +*test*message*promise_fast_reject.js:* + consol.log('One'); // eslint-disable-line no-undef + ^ + +ReferenceError: consol is not defined + at *test*message*promise_fast_reject.js:*:* + at Promise () + at Object. (*test*message*promise_fast_reject.js:*:*) + at Module._compile (module.js:*:*) + at Object.Module._extensions..js (module.js:*:*) + at Module.load (module.js:*:*) + at tryModuleLoad (module.js:*:*) + at Function.Module._load (module.js:*:*) + at Function.Module.runMain (module.js:*:*) + at startup (bootstrap_node.js:*:*) diff --git a/test/message/promise_reject.js b/test/message/promise_reject.js new file mode 100644 index 00000000000000..a70abd4c4b9873 --- /dev/null +++ b/test/message/promise_reject.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../common'); + + +Promise.reject(new Error('oops')); + +setImmediate(() => { + common.fail('Should not reach Immediate'); +}); + +process.on('beforeExit', () => + common.fail('beforeExit should not be reached')); + +process.on('uncaughtException', (err) => { + common.fail('Should not trigger uncaught exception'); +}); + +process.on('exit', () => process._rawDebug('exit event emitted')); diff --git a/test/message/promise_reject.out b/test/message/promise_reject.out new file mode 100644 index 00000000000000..6c720da0c1a9cd --- /dev/null +++ b/test/message/promise_reject.out @@ -0,0 +1,15 @@ +exit event emitted +*test*message*promise_reject.js:* +Promise.reject(new Error('oops')); + ^ + +Error: oops + at *test*message*promise_reject.js:*:* + at Module._compile (module.js:*:*) + at Object.Module._extensions..js (module.js:*:*) + at Module.load (module.js:*:*) + at tryModuleLoad (module.js:*:*) + at Function.Module._load (module.js:*:*) + at Function.Module.runMain (module.js:*:*) + at startup (bootstrap_node.js:*:*) + at bootstrap_node.js:*:* diff --git a/test/message/unhandled_promise_trace_warnings.js b/test/message/unhandled_promise_trace_warnings.js deleted file mode 100644 index 8aad957fb5cf49..00000000000000 --- a/test/message/unhandled_promise_trace_warnings.js +++ /dev/null @@ -1,5 +0,0 @@ -// Flags: --trace-warnings -'use strict'; -const common = require('../common'); -const p = Promise.reject(new Error('This was rejected')); -setImmediate(() => p.catch(common.noop)); diff --git a/test/message/unhandled_promise_trace_warnings.out b/test/message/unhandled_promise_trace_warnings.out deleted file mode 100644 index 4372efdeedb1ae..00000000000000 --- a/test/message/unhandled_promise_trace_warnings.out +++ /dev/null @@ -1,29 +0,0 @@ -(node:*) Error: This was rejected - at * (*test*message*unhandled_promise_trace_warnings.js:*) - at * - at * - at * - at * - at * - at * - at * - at * -(node:*) [DEP0018] DeprecationWarning: 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. - at * - at * - at * - at * - at * - at * - 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 Promise.then * - at Promise.catch * - at Immediate.setImmediate (*test*message*unhandled_promise_trace_warnings.js:*) - at * - at * - at * diff --git a/test/parallel/test-promises-handled-reject.js b/test/parallel/test-promises-handled-reject.js new file mode 100644 index 00000000000000..1015a2e45fc78d --- /dev/null +++ b/test/parallel/test-promises-handled-reject.js @@ -0,0 +1,19 @@ +'use strict'; +const common = require('../common'); + +// Flags: --expose-gc + +const p = new Promise((res, rej) => { + consol.log('oops'); // eslint-disable-line no-undef +}); + +// Manually call GC due to possible memory contraints with attempting to +// trigger it "naturally". +process.nextTick(common.mustCall(() => { + p.catch(() => {}); + /* eslint-disable no-undef */ + gc(); + gc(); + gc(); + /* eslint-enable no-undef */ +}, 1)); diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js index fb2404a99016b8..92ead009145ff8 100644 --- a/test/parallel/test-promises-unhandled-rejections.js +++ b/test/parallel/test-promises-unhandled-rejections.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const domain = require('domain'); @@ -673,6 +673,8 @@ asyncTest('Throwing an error inside a rejectionHandled handler goes to' + tearDownException(); done(); }); + // Prevent fatal unhandled error. + process.on('unhandledRejection', common.noop); process.on('rejectionHandled', function() { throw e2; }); diff --git a/test/parallel/test-promises-warning-on-unhandled-rejection.js b/test/parallel/test-promises-warning-on-unhandled-rejection.js deleted file mode 100644 index f3c7a8771e9f55..00000000000000 --- a/test/parallel/test-promises-warning-on-unhandled-rejection.js +++ /dev/null @@ -1,29 +0,0 @@ -// Flags: --no-warnings -'use strict'; - -// Test that warnings are emitted when a Promise experiences an uncaught -// rejection, and then again if the rejection is handled later on. - -const common = require('../common'); -const assert = require('assert'); - -let b = 0; - -process.on('warning', common.mustCall((warning) => { - switch (b++) { - case 0: - assert.strictEqual(warning.name, 'UnhandledPromiseRejectionWarning'); - assert(/Unhandled promise rejection/.test(warning.message)); - break; - case 1: - assert.strictEqual(warning.name, 'DeprecationWarning'); - break; - case 2: - assert.strictEqual(warning.name, 'PromiseRejectionHandledWarning'); - assert(/Promise rejection was handled asynchronously/ - .test(warning.message)); - } -}, 3)); - -const p = Promise.reject('This was rejected'); -setImmediate(common.mustCall(() => p.catch(common.noop)));