process: improve unhandled rejection message

PR-URL: #17158 Refs: #16768 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Dec 12, 2017 · 9a9aa88 · 9a9aa88
1 parent 5cbe0f2
commit 9a9aa88
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 14 deletions.
diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js
@@ -60,11 +60,25 @@ function setupPromises(scheduleMicrotasks) {
   }
 
   function emitWarning(uid, reason) {
+    try {
+      if (reason instanceof Error) {
+        process.emitWarning(reason.stack, 'UnhandledPromiseRejectionWarning');
+      } else {
+        process.emitWarning(
+          safeToString(reason), 'UnhandledPromiseRejectionWarning'
+        );
+      }
+    } catch (e) {
+      // ignored
+    }
+
     const warning = new Error(
-      `Unhandled promise rejection (rejection id: ${uid}): ` +
-      safeToString(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(). ' +
+      `(rejection id: ${uid})`
+    );
     warning.name = 'UnhandledPromiseRejectionWarning';
-    warning.id = uid;
     try {
       if (reason instanceof Error) {
         warning.stack = reason.stack;

diff --git a/test/message/unhandled_promise_trace_warnings.out b/test/message/unhandled_promise_trace_warnings.out
@@ -1,3 +1,21 @@
+(node:*) UnhandledPromiseRejectionWarning: Error: This was rejected
+    at * (*test*message*unhandled_promise_trace_warnings.js:*)
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
 (node:*) Error: This was rejected
     at * (*test*message*unhandled_promise_trace_warnings.js:*)
     at *

diff --git a/test/parallel/test-promises-unhandled-proxy-rejections.js b/test/parallel/test-promises-unhandled-proxy-rejections.js
@@ -6,8 +6,11 @@ const expectedDeprecationWarning = 'Unhandled promise rejections are ' +
                                    'rejections that are not handled will ' +
                                    'terminate the Node.js process with a ' +
                                    'non-zero exit code.';
-const expectedPromiseWarning = 'Unhandled promise rejection (rejection id: ' +
-                               '1): [object Object]';
+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(). (rejection id: 1)';
 
 function throwErr() {
   throw new Error('Error from proxy');

diff --git a/test/parallel/test-promises-unhandled-symbol-rejections.js b/test/parallel/test-promises-unhandled-symbol-rejections.js
@@ -1,17 +1,24 @@
 'use strict';
 const common = require('../common');
 
+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.';
-const expectedPromiseWarning = 'Unhandled promise rejection (rejection id: ' +
-                               '1): Symbol()';
+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(). (rejection id: 1)';
 
 common.expectWarning({
   DeprecationWarning: expectedDeprecationWarning,
-  UnhandledPromiseRejectionWarning: expectedPromiseWarning,
+  UnhandledPromiseRejectionWarning: [
+    expectedPromiseWarning,
+    expectedValueWarning
+  ],
 });
 
 // ensure this doesn't crash

diff --git a/test/parallel/test-promises-warning-on-unhandled-rejection.js b/test/parallel/test-promises-warning-on-unhandled-rejection.js
@@ -12,18 +12,43 @@ 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));
+      // String rejection error displayed
+      assert.strictEqual(warning.message, 'This was rejected');
       break;
     case 1:
-      assert.strictEqual(warning.name, 'DeprecationWarning');
+      // Warning about rejection not being handled (will be next tick)
+      assert.strictEqual(warning.name, 'UnhandledPromiseRejectionWarning');
+      assert(
+        /Unhandled promise rejection/.test(warning.message),
+        'Expected warning message to contain "Unhandled promise rejection" ' +
+        'but did not. Had "' + warning.message + '" instead.'
+      );
       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:
+      // Unhandled rejection warning (won't be handled next tick)
+      assert.strictEqual(warning.name, 'UnhandledPromiseRejectionWarning');
+      assert(
+        /Unhandled promise rejection/.test(warning.message),
+        'Expected warning message to contain "Unhandled promise rejection" ' +
+        'but did not. Had "' + warning.message + '" instead.'
+      );
+      break;
+    case 5:
+      // Rejection handled asynchronously.
       assert.strictEqual(warning.name, 'PromiseRejectionHandledWarning');
       assert(/Promise rejection was handled asynchronously/
         .test(warning.message));
   }
-}, 3));
+}, 6));
 
-const p = Promise.reject('This was rejected');
-setImmediate(common.mustCall(() => p.catch(() => {})));
+const p = Promise.reject('This was rejected'); // Reject with a string
+setImmediate(common.mustCall(() => p.catch(() => { })));
+Promise.reject(42); // Reject with a number