process: change default --unhandled-rejections=throw

This is a semver-major change that resolves DEP0018. All users that have set an unhandledRejection hook or set a non-default value for the --unhandled-rejections flag will see no change in behavior after this change. Refs: https://nodejs.org/dist/latest/docs/api/deprecations.html#deprecations_dep0018_unhandled_promise_rejections PR-URL: nodejs#33021 Fixes: nodejs#20392 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Mary Marchini <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]>
joesepi · Sep 22, 2020 · 3b10f7f · 3b10f7f
1 parent 25d8456
commit 3b10f7f
Show file tree

Hide file tree

Showing 15 changed files with 73 additions and 97 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -1007,15 +1007,11 @@ added:
  - v10.17.0
 -->
 
-By default all unhandled rejections trigger a warning plus a deprecation warning
-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 the following modes can be chosen:
 
 * `throw`: Emit [`unhandledRejection`][]. If this hook is not set, raise the
-  unhandled rejection as an uncaught exception.
+  unhandled rejection as an uncaught exception. This is the default.
 * `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.

diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js
@@ -57,11 +57,6 @@ const kThrowUnhandledRejections = 3;
 
 const kWarnWithErrorCodeUnhandledRejections = 4;
 
-// --unhandled-rejections is unset:
-// Emit 'unhandledRejection', if it's unhandled, emit
-// 'UnhandledPromiseRejectionWarning', then emit deprecation warning.
-const kDefaultUnhandledRejections = 5;
-
 let unhandledRejectionsMode;
 
 function setHasRejectionToWarn(value) {
@@ -86,7 +81,7 @@ function getUnhandledRejectionsMode() {
     case 'warn-with-error-code':
       return kWarnWithErrorCodeUnhandledRejections;
     default:
-      return kDefaultUnhandledRejections;
+      return kThrowUnhandledRejections;
   }
 }
 
@@ -175,15 +170,6 @@ function emitUnhandledRejectionWarning(uid, reason) {
   process.emitWarning(warning);
 }
 
-let deprecationWarned = false;
-function emitDeprecationWarning() {
-  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');
-}
-
 // If this method returns true, we've executed user code or triggered
 // a warning to be emitted which requires the microtask and next tick
 // queues to be drained again.
@@ -241,17 +227,6 @@ function processPromiseRejections() {
         }
         break;
       }
-      case kDefaultUnhandledRejections: {
-        const handled = process.emit('unhandledRejection', reason, promise);
-        if (!handled) {
-          emitUnhandledRejectionWarning(uid, reason);
-          if (!deprecationWarned) {
-            emitDeprecationWarning();
-            deprecationWarned = true;
-          }
-        }
-        break;
-      }
     }
     maybeScheduledTicksOrMicrotasks = true;
   }

diff --git a/test/common/index.js b/test/common/index.js
@@ -599,11 +599,8 @@ function getBufferSources(buf) {
   return [...getArrayBufferViews(buf), new Uint8Array(buf).buffer];
 }
 
-// Crash the process on unhandled rejections.
-const crashOnUnhandledRejection = (err) => { throw err; };
-process.on('unhandledRejection', crashOnUnhandledRejection);
 function disableCrashOnUnhandledRejection() {
-  process.removeListener('unhandledRejection', crashOnUnhandledRejection);
+  process.on('unhandledRejection', () => {});
 }
 
 function getTTYfd() {

diff --git a/test/es-module/test-esm-cjs-load-error-note.mjs b/test/es-module/test-esm-cjs-load-error-note.mjs
@@ -63,8 +63,7 @@ pImport2.stderr.on('data', (data) => {
   pImport2Stderr += data;
 });
 pImport2.on('close', mustCall((code) => {
-  // As this exits normally we expect 0
-  assert.strictEqual(code, 0);
+  assert.strictEqual(code, expectedCode);
   assert.ok(!pImport2Stderr.includes(expectedNote),
             `${expectedNote} must not be included in ${pImport2Stderr}`);
 }));
@@ -94,15 +93,15 @@ pImport4.on('close', mustCall((code) => {
             `${expectedNote} not found in ${pImport4Stderr}`);
 }));
 
-// Must exit with zero and show note
+// Must exit non-zero and show note
 const pImport5 = spawn(process.execPath, [Import5]);
 let pImport5Stderr = '';
 pImport5.stderr.setEncoding('utf8');
 pImport5.stderr.on('data', (data) => {
   pImport5Stderr += data;
 });
 pImport5.on('close', mustCall((code) => {
-  assert.strictEqual(code, 0);
+  assert.strictEqual(code, expectedCode);
   assert.ok(!pImport5Stderr.includes(expectedNote),
             `${expectedNote} must not be included in ${pImport5Stderr}`);
 }));
diff --git a/test/message/promise_unhandled_warn_with_error.js b/test/message/promise_unhandled_warn_with_error.js
@@ -1,10 +1,8 @@
 // Flags: --unhandled-rejections=warn-with-error-code
 'use strict';
 
-const common = require('../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/unhandled_promise_trace_warnings.js b/test/message/unhandled_promise_trace_warnings.js
@@ -1,6 +1,5 @@
-// Flags: --trace-warnings
+// Flags: --trace-warnings --unhandled-rejections=warn
 'use strict';
-const common = require('../common');
-common.disableCrashOnUnhandledRejection();
+require('../common');
 const p = Promise.reject(new Error('This was rejected'));
 setImmediate(() => p.catch(() => {}));
diff --git a/test/message/unhandled_promise_trace_warnings.out b/test/message/unhandled_promise_trace_warnings.out
@@ -17,10 +17,6 @@
     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 *
 (node:*) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
     at handledRejection (internal/process/promises.js:*)
     at promiseRejectHandler (internal/process/promises.js:*)

diff --git a/test/parallel/test-promise-unhandled-default.js b/test/parallel/test-promise-unhandled-default.js
@@ -0,0 +1,52 @@
+'use strict';
+
+const common = require('../common');
+const Countdown = require('../common/countdown');
+const assert = require('assert');
+
+// 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));
diff --git a/test/parallel/test-promise-unhandled-throw.js b/test/parallel/test-promise-unhandled-throw.js
@@ -5,8 +5,6 @@ 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.
 

diff --git a/test/parallel/test-promise-unhandled-warn-no-hook.js b/test/parallel/test-promise-unhandled-warn-no-hook.js
@@ -3,10 +3,7 @@
 
 const common = require('../common');
 
-common.disableCrashOnUnhandledRejection();
-
-// Verify that ignoring unhandled rejection works fine and that no warning is
-// logged.
+// Verify that --unhandled-rejections=warn works fine
 
 new Promise(() => {
   throw new Error('One');

diff --git a/test/parallel/test-promises-unhandled-proxy-rejections.js b/test/parallel/test-promises-unhandled-proxy-rejections.js
@@ -3,21 +3,6 @@ const common = require('../common');
 
 common.disableCrashOnUnhandledRejection();
 
-const expectedDeprecationWarning = ['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.', 'DEP0018'];
-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, ' +
-  'use the CLI flag `--unhandled-rejections=strict` (see ' +
-  'https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). ' +
-  '(rejection id: 1)'];
-
 function throwErr() {
   throw new Error('Error from proxy');
 }
@@ -38,10 +23,7 @@ const thorny = new Proxy({}, {
   construct: throwErr
 });
 
-common.expectWarning({
-  DeprecationWarning: expectedDeprecationWarning,
-  UnhandledPromiseRejectionWarning: expectedPromiseWarning,
-});
+process.on('warning', common.mustNotCall());
 
 // Ensure this doesn't crash
 Promise.reject(thorny);
diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js
@@ -668,6 +668,7 @@ asyncTest('Throwing an error inside a rejectionHandled handler goes to' +
           ' unhandledException, and does not cause .catch() to throw an ' +
           'exception', function(done) {
   clean();
+  common.disableCrashOnUnhandledRejection();
   const e = new Error();
   const e2 = new Error();
   const tearDownException = setupException(function(err) {
@@ -702,19 +703,17 @@ asyncTest('Rejected promise inside unhandledRejection allows nextTick loop' +
 });
 
 asyncTest(
-  'Unhandled promise rejection emits a warning immediately',
+  'Promise rejection triggers unhandledRejection immediately',
   function(done) {
     clean();
     Promise.reject(0);
-    const { emitWarning } = process;
-    process.emitWarning = common.mustCall((...args) => {
+    process.on('unhandledRejection', common.mustCall((err) => {
       if (timer) {
         clearTimeout(timer);
         timer = null;
         done();
       }
-      emitWarning(...args);
-    }, 2);
+    }));
 
     let timer = setTimeout(common.mustNotCall(), 10000);
   },

diff --git a/test/parallel/test-promises-unhandled-symbol-rejections.js b/test/parallel/test-promises-unhandled-symbol-rejections.js
@@ -1,14 +1,10 @@
+// Flags: --unhandled-rejections=warn
 'use strict';
 const common = require('../common');
 
 common.disableCrashOnUnhandledRejection();
 
 const expectedValueWarning = ['Symbol()'];
-const expectedDeprecationWarning = ['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.', 'DEP0018'];
 const expectedPromiseWarning = ['Unhandled promise rejection. ' +
   'This error originated either by throwing ' +
   'inside of an async function without a catch ' +
@@ -20,7 +16,6 @@ const expectedPromiseWarning = ['Unhandled promise rejection. ' +
   '(rejection id: 1)'];
 
 common.expectWarning({
-  DeprecationWarning: expectedDeprecationWarning,
   UnhandledPromiseRejectionWarning: [
     expectedValueWarning,
     expectedPromiseWarning

diff --git a/test/parallel/test-promises-warning-on-unhandled-rejection.js b/test/parallel/test-promises-warning-on-unhandled-rejection.js
@@ -1,4 +1,4 @@
-// Flags: --no-warnings
+// Flags: --no-warnings --unhandled-rejections=warn
 'use strict';
 
 // Test that warnings are emitted when a Promise experiences an uncaught
@@ -7,8 +7,6 @@
 const common = require('../common');
 const assert = require('assert');
 
-common.disableCrashOnUnhandledRejection();
-
 let b = 0;
 
 process.on('warning', common.mustCall((warning) => {
@@ -27,14 +25,10 @@ process.on('warning', common.mustCall((warning) => {
       );
       break;
     case 2:
-      // One time deprecation warning, first unhandled rejection
-      assert.strictEqual(warning.name, 'DeprecationWarning');
-      break;
-    case 3:
       // Number rejection error displayed. Note it's been stringified
       assert.strictEqual(warning.message, '42');
       break;
-    case 4:
+    case 3:
       // Unhandled rejection warning (won't be handled next tick)
       assert.strictEqual(warning.name, 'UnhandledPromiseRejectionWarning');
       assert(
@@ -43,13 +37,13 @@ process.on('warning', common.mustCall((warning) => {
         `but did not. Had "${warning.message}" instead.`
       );
       break;
-    case 5:
+    case 4:
       // Rejection handled asynchronously.
       assert.strictEqual(warning.name, 'PromiseRejectionHandledWarning');
       assert(/Promise rejection was handled asynchronously/
         .test(warning.message));
   }
-}, 6));
+}, 5));
 
 const p = Promise.reject('This was rejected'); // Reject with a string
 setImmediate(common.mustCall(() => p.catch(() => { })));

diff --git a/test/sequential/test-inspector-async-call-stack-abort.js b/test/sequential/test-inspector-async-call-stack-abort.js
@@ -9,7 +9,6 @@ const { strictEqual } = require('assert');
 const eyecatcher = 'nou, houdoe he?';
 
 if (process.argv[2] === 'child') {
-  common.disableCrashOnUnhandledRejection();
   const { Session } = require('inspector');
   const { promisify } = require('util');
   const { internalBinding } = require('internal/test/binding');
@@ -31,7 +30,7 @@ if (process.argv[2] === 'child') {
   const options = { encoding: 'utf8' };
   const proc = spawnSync(
     process.execPath, ['--expose-internals', __filename, 'child'], options);
-  strictEqual(proc.status, 0);
+  strictEqual(proc.status, 1);
   strictEqual(proc.signal, null);
   strictEqual(proc.stderr.includes(eyecatcher), true);
 }