Skip to content

Commit

Permalink
process: Don't warn/throw on unhandled rejections when hook is set
Browse files Browse the repository at this point in the history
--unhandled-rejections has three explicit modes (strict, warn, none)
plus one implicit "default" mode, which logs an additional deprecation
warning (DEP0018).

Prior to this commit, the default mode was subtly different from warn
mode. If the unhandledRejections hook is set, default mode suppresses
all warnings. In warn mode, unhandledRejections would always fire a
warning, regardless of whether the hook was set.

In addition, prior to this commit, strict mode would always throw an
exception, regardless of whether the hook was set.

In this commit, all modes honor the unhandledRejections hook. If the
user has set the hook, then the user has taken full responsibility over
the behavior of unhandled rejections. In that case, no additional
warnings or thrown exceptions will be fired, even in warn mode or
strict mode.

This commit is a stepping stone towards resolving DEP0018. After this
commit, any code that includes an unhandledRejection hook will behave
unchanged when we change the default mode.

Refs: nodejs#26599
  • Loading branch information
dfabulich committed Apr 23, 2020
1 parent 91ca221 commit d5409f4
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 45 deletions.
18 changes: 13 additions & 5 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -890,12 +890,20 @@ 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 and the [`unhandledRejection`][] hook is unset.

* `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.
* `none`: Silence all warnings.
One of three modes can be chosen:

* `strict`: Emit [`unhandledRejection`][]. If this hook is not set, raise the
unhandled rejection as an uncaught exception.
* `warn`: Emit [`unhandledRejection`][]. If this hook is not set, trigger a
warning. This is the default.
* `none`: Emit [`unhandledRejection`][] and do nothing further.

In all three modes, setting the [`unhandledRejection`][] hook will suppress
the exception/warning. The hook implementation can raise an exception (as in
`strict` mode), log a warning (as in `warn` mode), or do nothing (as in `none`
mode).

### `--use-bundled-ca`, `--use-openssl-ca`
<!-- YAML
Expand Down
63 changes: 30 additions & 33 deletions lib/internal/process/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,17 @@ 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:
// Emit 'unhandledRejection', then emit 'UnhandledPromiseRejectionWarning'.
const kAlwaysWarnUnhandledRejections = 1;
// --unhandled-rejection=strict:
// Emit 'uncaughtException'. If it's not handled, print the error to stderr
// --unhandled-rejections=warn:
// Emit 'unhandledRejection', if it's unhandled, emit
// 'UnhandledPromiseRejectionWarning'.
const kWarnUnhandledRejections = 1;
// --unhandled-rejections=strict:
// Emit 'unhandledRejection', if it's unhandled, 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
Expand All @@ -62,10 +62,10 @@ function getUnhandledRejectionsMode() {
switch (getOptionValue('--unhandled-rejections')) {
case 'none':
return kIgnoreUnhandledRejections;
case 'warn':
return kAlwaysWarnUnhandledRejections;
case 'strict':
return kThrowUnhandledRejections;
case 'warn':
return kWarnUnhandledRejections;
default:
return kDefaultUnhandledRejections;
}
Expand Down Expand Up @@ -138,7 +138,8 @@ function emitUnhandledRejectionWarning(uid, reason) {
'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 ' +
'To suppress this warning, use process.on("unhandledRejection"). ' +
'Or, 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: ${uid})`
Expand Down Expand Up @@ -187,34 +188,30 @@ function processPromiseRejections() {
}
promiseInfo.warned = true;
const { reason, uid } = promiseInfo;
switch (unhandledRejectionsMode) {
case kThrowUnhandledRejections: {
const err = reason instanceof Error ?
reason : generateUnhandledRejectionError(reason);
triggerUncaughtException(err, true /* fromPromise */);
const handled = process.emit('unhandledRejection', reason, promise);
if (!handled) emitUnhandledRejectionWarning(uid, reason);
break;
}
case kIgnoreUnhandledRejections: {
process.emit('unhandledRejection', reason, promise);
break;
}
case kAlwaysWarnUnhandledRejections: {
process.emit('unhandledRejection', reason, promise);
emitUnhandledRejectionWarning(uid, reason);
break;
}
case kDefaultUnhandledRejections: {
const handled = process.emit('unhandledRejection', reason, promise);
if (!handled) {
const handled = process.emit('unhandledRejection', reason, promise);
if (!handled) {
switch (unhandledRejectionsMode) {
case kThrowUnhandledRejections: {
const err = reason instanceof Error ?
reason : generateUnhandledRejectionError(reason);
triggerUncaughtException(err, true /* fromPromise */);
break;
}
case kIgnoreUnhandledRejections: {
break;
}
case kWarnUnhandledRejections: {
emitUnhandledRejectionWarning(uid, reason);
break;
}
case kDefaultUnhandledRejections: {
emitUnhandledRejectionWarning(uid, reason);
if (!deprecationWarned) {
emitDeprecationWarning();
deprecationWarned = true;
}
break;
}
break;
}
}
maybeScheduledTicksOrMicrotasks = true;
Expand Down
4 changes: 3 additions & 1 deletion test/message/promise_always_throw_unhandled.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Flags: --unhandled-rejections=strict
'use strict';

require('../common');
const common = require('../common');

common.disableCrashOnUnhandledRejection();

// Check that the process will exit on the first unhandled rejection in case the
// unhandled rejections mode is set to `'strict'`.
Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-promise-unhandled-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const ref = new Promise(() => {
Promise.reject(null);

process.on('warning', common.mustNotCall('warning'));
process.on('unhandledRejection', common.mustCall(2));
process.on('rejectionHandled', common.mustNotCall('rejectionHandled'));
process.on('exit', assert.strictEqual.bind(null, 0));

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-promise-unhandled-warn.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Promise.reject('test');

// Unhandled rejections trigger two warning per rejection. One is the rejection
// reason and the other is a note where this warning is coming from.
process.on('warning', common.mustCall(4));
process.on('warning', common.mustNotCall());
process.on('uncaughtException', common.mustNotCall('uncaughtException'));
process.on('rejectionHandled', common.mustCall(2));

Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-promises-unhandled-proxy-rejections.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ const expectedPromiseWarning = ['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, ' +
'not handled with .catch(). To suppress this warning, ' +
'use process.on("unhandledRejection"). Or, 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)'];
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-promises-unhandled-symbol-rejections.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ const expectedPromiseWarning = ['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, ' +
'not handled with .catch(). To suppress this warning, ' +
'use process.on("unhandledRejection"). Or, 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)'];
Expand Down

0 comments on commit d5409f4

Please sign in to comment.