From 9a9aa88797b74fb47f4453fef4242f73dbab114e Mon Sep 17 00:00:00 2001 From: Madara Uchiha Date: Mon, 20 Nov 2017 14:22:37 -0500 Subject: [PATCH] process: improve unhandled rejection message PR-URL: https://github.com/nodejs/node/pull/17158 Refs: https://github.com/nodejs/node/pull/16768 Reviewed-By: Refael Ackermann Reviewed-By: Benjamin Gruenbaum Reviewed-By: Evan Lucas Reviewed-By: James M Snell --- lib/internal/process/promises.js | 20 ++++++++-- .../unhandled_promise_trace_warnings.out | 18 +++++++++ ...est-promises-unhandled-proxy-rejections.js | 7 +++- ...st-promises-unhandled-symbol-rejections.js | 13 +++++-- ...promises-warning-on-unhandled-rejection.js | 37 ++++++++++++++++--- 5 files changed, 81 insertions(+), 14 deletions(-) diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 663e6f5fec6985..82f15f9f9df51e 100644 --- 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 index 77e7edcf4c6d88..410b3bf4e36d41 100644 --- 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 index 91808af14588d3..f632857d6373ba 100644 --- 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 index a60a5f2e8aac30..3e687d4e49bf08 100644 --- 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 index 10f95162a09597..3ac7d8698beb37 100644 --- 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