process: Don't warn/throw on unhandled rejections when hook is set

dfabulich · dfabulich · commit d5409f41ebf6 · 2020-04-23T00:42:46.000-07:00
--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
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -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
diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js
@@ -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
@@ -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;
   }
@@ -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})`
@@ -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;
diff --git a/test/message/promise_always_throw_unhandled.js b/test/message/promise_always_throw_unhandled.js
@@ -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'`.
diff --git a/test/parallel/test-promise-unhandled-error.js b/test/parallel/test-promise-unhandled-error.js
@@ -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));
 
diff --git a/test/parallel/test-promise-unhandled-warn.js b/test/parallel/test-promise-unhandled-warn.js
@@ -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));
 
diff --git a/test/parallel/test-promises-unhandled-proxy-rejections.js b/test/parallel/test-promises-unhandled-proxy-rejections.js
@@ -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)'];
diff --git a/test/parallel/test-promises-unhandled-symbol-rejections.js b/test/parallel/test-promises-unhandled-symbol-rejections.js
@@ -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)'];
