From be300cf6f5c62aad88be17f41825e56c75d7fcbc Mon Sep 17 00:00:00 2001 From: Dan Fabulich Date: Tue, 19 May 2020 17:41:01 -0700 Subject: [PATCH] process: add unhandled-rejection throw and warn-with-error-code This PR defines two new modes for the --unhandled-rejections flag. The first mode is called "throw". The "throw" mode first emits unhandledRejection. If this hook is not set, the "throw" mode will raise the unhandled rejection as an uncaught exception. The second mode is called "warn-with-error-code". The "warn-with-error-code" mode first emits unhandledRejection. If this hook is not set, the "warn-with-error-code" mode will trigger a warning and set the process's exit code to 1. The PR doesn't change the default behavior for unhandled rejections. That will come in a separate PR. Refs: https://github.com/nodejs/node/pull/33021 PR-URL: https://github.com/nodejs/node/pull/33475 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Anna Henningsen --- doc/api/cli.md | 6 +- lib/internal/process/promises.js | 52 +++++++++++++++--- src/node_options.cc | 2 + .../promise_unhandled_warn_with_error.js | 10 ++++ .../promise_unhandled_warn_with_error.out | 10 ++++ .../test-promise-unhandled-throw-handler.js | 38 +++++++++++++ test/parallel/test-promise-unhandled-throw.js | 55 +++++++++++++++++++ 7 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 test/message/promise_unhandled_warn_with_error.js create mode 100644 test/message/promise_unhandled_warn_with_error.out create mode 100644 test/parallel/test-promise-unhandled-throw-handler.js create mode 100644 test/parallel/test-promise-unhandled-throw.js diff --git a/doc/api/cli.md b/doc/api/cli.md index d31fa43fb6a6e7..cee1a9622bf2ce 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -952,11 +952,15 @@ for the very first unhandled rejection in case no [`unhandledRejection`][] hook is used. Using this flag allows to change what should happen when an unhandled rejection -occurs. One of three modes can be chosen: +occurs. One of the following modes can be chosen: +* `throw`: Emit [`unhandledRejection`][]. If this hook is not set, raise the + unhandled rejection as an uncaught exception. * `strict`: Raise the unhandled rejection as an uncaught exception. * `warn`: Always trigger a warning, no matter if the [`unhandledRejection`][] hook is set or not but do not print the deprecation warning. +* `warn-with-error-code`: Emit [`unhandledRejection`][]. If this hook is not + set, trigger a warning, and set the process exit code to 1. * `none`: Silence all warnings. ### `--use-bundled-ca`, `--use-openssl-ca` diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 1a0ed06f213df8..f2145d425c620f 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -30,22 +30,37 @@ const pendingUnhandledRejections = []; const asyncHandledRejections = []; let lastPromiseId = 0; -// --unhandled-rejection=none: +// --unhandled-rejections=none: // Emit 'unhandledRejection', but do not emit any warning. const kIgnoreUnhandledRejections = 0; -// --unhandled-rejection=warn: + +// --unhandled-rejections=warn: // Emit 'unhandledRejection', then emit 'UnhandledPromiseRejectionWarning'. const kAlwaysWarnUnhandledRejections = 1; -// --unhandled-rejection=strict: + +// --unhandled-rejections=strict: // Emit 'uncaughtException'. If it's not handled, print the error to stderr // and exit the process. // Otherwise, emit 'unhandledRejection'. If 'unhandledRejection' is not // handled, emit 'UnhandledPromiseRejectionWarning'. -const kThrowUnhandledRejections = 2; -// --unhandled-rejection is unset: -// Emit 'unhandledRejection', if it's handled, emit +const kStrictUnhandledRejections = 2; + +// --unhandled-rejections=throw: +// Emit 'unhandledRejection', if it's unhandled, emit +// 'uncaughtException'. If it's not handled, print the error to stderr +// and exit the process. +const kThrowUnhandledRejections = 3; + +// --unhandled-rejections=warn-with-error-code: +// Emit 'unhandledRejection', if it's unhandled, emit +// 'UnhandledPromiseRejectionWarning', then set process exit code to 1. + +const kWarnWithErrorCodeUnhandledRejections = 4; + +// --unhandled-rejections is unset: +// Emit 'unhandledRejection', if it's unhandled, emit // 'UnhandledPromiseRejectionWarning', then emit deprecation warning. -const kDefaultUnhandledRejections = 3; +const kDefaultUnhandledRejections = 5; let unhandledRejectionsMode; @@ -65,7 +80,11 @@ function getUnhandledRejectionsMode() { case 'warn': return kAlwaysWarnUnhandledRejections; case 'strict': + return kStrictUnhandledRejections; + case 'throw': return kThrowUnhandledRejections; + case 'warn-with-error-code': + return kWarnWithErrorCodeUnhandledRejections; default: return kDefaultUnhandledRejections; } @@ -188,7 +207,7 @@ function processPromiseRejections() { promiseInfo.warned = true; const { reason, uid } = promiseInfo; switch (unhandledRejectionsMode) { - case kThrowUnhandledRejections: { + case kStrictUnhandledRejections: { const err = reason instanceof Error ? reason : generateUnhandledRejectionError(reason); triggerUncaughtException(err, true /* fromPromise */); @@ -205,6 +224,23 @@ function processPromiseRejections() { emitUnhandledRejectionWarning(uid, reason); break; } + case kThrowUnhandledRejections: { + const handled = process.emit('unhandledRejection', reason, promise); + if (!handled) { + const err = reason instanceof Error ? + reason : generateUnhandledRejectionError(reason); + triggerUncaughtException(err, true /* fromPromise */); + } + break; + } + case kWarnWithErrorCodeUnhandledRejections: { + const handled = process.emit('unhandledRejection', reason, promise); + if (!handled) { + emitUnhandledRejectionWarning(uid, reason); + process.exitCode = 1; + } + break; + } case kDefaultUnhandledRejections: { const handled = process.emit('unhandledRejection', reason, promise); if (!handled) { diff --git a/src/node_options.cc b/src/node_options.cc index f4d6e75acd9d07..309a7bd505a1e8 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -118,6 +118,8 @@ void EnvironmentOptions::CheckOptions(std::vector* errors) { } if (!unhandled_rejections.empty() && + unhandled_rejections != "warn-with-error-code" && + unhandled_rejections != "throw" && unhandled_rejections != "strict" && unhandled_rejections != "warn" && unhandled_rejections != "none") { diff --git a/test/message/promise_unhandled_warn_with_error.js b/test/message/promise_unhandled_warn_with_error.js new file mode 100644 index 00000000000000..0f22e8deeba653 --- /dev/null +++ b/test/message/promise_unhandled_warn_with_error.js @@ -0,0 +1,10 @@ +// Flags: --unhandled-rejections=warn-with-error-code +'use strict'; + +const common = require('../common'); +const assert = require('assert'); + +common.disableCrashOnUnhandledRejection(); + +Promise.reject(new Error('alas')); +process.on('exit', assert.strictEqual.bind(null, 1)); diff --git a/test/message/promise_unhandled_warn_with_error.out b/test/message/promise_unhandled_warn_with_error.out new file mode 100644 index 00000000000000..b539adb2d1e769 --- /dev/null +++ b/test/message/promise_unhandled_warn_with_error.out @@ -0,0 +1,10 @@ +*UnhandledPromiseRejectionWarning: Error: alas + at *promise_unhandled_warn_with_error.js:*:* + at * + at * + at * + at * + at * + at * +(Use `node --trace-warnings ...` to show where the warning was created) +*UnhandledPromiseRejectionWarning: 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(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1) \ No newline at end of file diff --git a/test/parallel/test-promise-unhandled-throw-handler.js b/test/parallel/test-promise-unhandled-throw-handler.js new file mode 100644 index 00000000000000..be441c2d34c6bd --- /dev/null +++ b/test/parallel/test-promise-unhandled-throw-handler.js @@ -0,0 +1,38 @@ +// Flags: --unhandled-rejections=throw +'use strict'; + +const common = require('../common'); +const Countdown = require('../common/countdown'); +const assert = require('assert'); + +common.disableCrashOnUnhandledRejection(); + +// Verify that the unhandledRejection handler prevents triggering +// uncaught exceptions + +const err1 = new Error('One'); + +const errors = [err1, null]; + +const ref = new Promise(() => { + throw err1; +}); +// Explicitly reject `null`. +Promise.reject(null); + +process.on('warning', common.mustNotCall('warning')); +process.on('rejectionHandled', common.mustNotCall('rejectionHandled')); +process.on('exit', assert.strictEqual.bind(null, 0)); +process.on('uncaughtException', common.mustNotCall('uncaughtException')); + +const timer = setTimeout(() => console.log(ref), 1000); + +const counter = new Countdown(2, () => { + clearTimeout(timer); +}); + +process.on('unhandledRejection', common.mustCall((err) => { + counter.dec(); + const knownError = errors.shift(); + assert.deepStrictEqual(err, knownError); +}, 2)); diff --git a/test/parallel/test-promise-unhandled-throw.js b/test/parallel/test-promise-unhandled-throw.js new file mode 100644 index 00000000000000..30c6b87135b49b --- /dev/null +++ b/test/parallel/test-promise-unhandled-throw.js @@ -0,0 +1,55 @@ +// Flags: --unhandled-rejections=throw +'use strict'; + +const common = require('../common'); +const Countdown = require('../common/countdown'); +const assert = require('assert'); + +common.disableCrashOnUnhandledRejection(); + +// Verify that unhandled rejections always trigger uncaught exceptions instead +// of triggering unhandled rejections. + +const err1 = new Error('One'); +const err2 = new Error( + '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(). The promise rejected with the' + + ' reason "null".' +); +err2.code = 'ERR_UNHANDLED_REJECTION'; +Object.defineProperty(err2, 'name', { + value: 'UnhandledPromiseRejection', + writable: true, + configurable: true +}); + +const errors = [err1, err2]; +const identical = [true, false]; + +const ref = new Promise(() => { + throw err1; +}); +// Explicitly reject `null`. +Promise.reject(null); + +process.on('warning', common.mustNotCall('warning')); +// If we add an unhandledRejection handler, the exception won't be thrown +// process.on('unhandledRejection', common.mustCall(2)); +process.on('rejectionHandled', common.mustNotCall('rejectionHandled')); +process.on('exit', assert.strictEqual.bind(null, 0)); + +const timer = setTimeout(() => console.log(ref), 1000); + +const counter = new Countdown(2, () => { + clearTimeout(timer); +}); + +process.on('uncaughtException', common.mustCall((err, origin) => { + counter.dec(); + assert.strictEqual(origin, 'unhandledRejection', err); + const knownError = errors.shift(); + assert.deepStrictEqual(err, knownError); + // Check if the errors are reference equal. + assert(identical.shift() ? err === knownError : err !== knownError); +}, 2));