From aedb5db8e3549eec572bf40e0f4c0fd97cc82c09 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Thu, 28 Apr 2016 11:11:09 +0300 Subject: [PATCH 1/2] promise: warn on unhandled rejections Log unhandled promise rejections with a guid and emit a process warning. When rejection is eventually handled, emit a secondary warning. PR-URL: https://github.com/nodejs/node/pull/8217 Reviewed-By: Chris Dickinson Reviewed-By: Benjamin Gruenbaum Reviewed-By: Jeremiah Senkpiel --- lib/internal/process/promises.js | 27 ++++++++++++++----- ...promises-warning-on-unhandled-rejection.js | 26 ++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-promises-warning-on-unhandled-rejection.js diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 22f1959784be9a..02c90a928bbc64 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -2,7 +2,9 @@ const promiseRejectEvent = process._promiseRejectEvent; const hasBeenNotifiedProperty = new WeakMap(); +const promiseToGuidProperty = new WeakMap(); const pendingUnhandledRejections = []; +let lastPromiseId = 1; exports.setup = setupPromises; @@ -18,6 +20,7 @@ function setupPromises(scheduleMicrotasks) { function unhandledRejection(promise, reason) { hasBeenNotifiedProperty.set(promise, false); + promiseToGuidProperty.set(promise, lastPromiseId++); addPendingUnhandledRejection(promise, reason); } @@ -25,9 +28,17 @@ function setupPromises(scheduleMicrotasks) { var hasBeenNotified = hasBeenNotifiedProperty.get(promise); if (hasBeenNotified !== undefined) { hasBeenNotifiedProperty.delete(promise); + const uid = promiseToGuidProperty.get(promise); + promiseToGuidProperty.delete(promise); if (hasBeenNotified === true) { process.nextTick(function() { - process.emit('rejectionHandled', promise); + if (!process.emit('rejectionHandled', promise)) { + const warning = new Error('Promise rejection was handled ' + + `asynchronously (rejection id: ${uid})`); + warning.name = 'PromiseRejectionHandledWarning'; + warning.id = uid; + process.emitWarning(warning); + } }); } @@ -35,15 +46,19 @@ function setupPromises(scheduleMicrotasks) { } function emitPendingUnhandledRejections() { - var hadListeners = false; + let hadListeners = false; while (pendingUnhandledRejections.length > 0) { - var promise = pendingUnhandledRejections.shift(); - var reason = pendingUnhandledRejections.shift(); + const promise = pendingUnhandledRejections.shift(); + const reason = pendingUnhandledRejections.shift(); if (hasBeenNotifiedProperty.get(promise) === false) { hasBeenNotifiedProperty.set(promise, true); + const uid = promiseToGuidProperty.get(promise); if (!process.emit('unhandledRejection', reason, promise)) { - // Nobody is listening. - // TODO(petkaantonov) Take some default action, see #830 + const warning = new Error('Unhandled promise rejection ' + + `(rejection id: ${uid}): ${reason}`); + warning.name = 'UnhandledPromiseRejectionWarning'; + warning.id = uid; + process.emitWarning(warning); } else { hadListeners = true; } diff --git a/test/parallel/test-promises-warning-on-unhandled-rejection.js b/test/parallel/test-promises-warning-on-unhandled-rejection.js new file mode 100644 index 00000000000000..9b1e7f3266aba0 --- /dev/null +++ b/test/parallel/test-promises-warning-on-unhandled-rejection.js @@ -0,0 +1,26 @@ +// Flags: --no-warnings +'use strict'; + +// Test that warnings are emitted when a Promise experiences an uncaught +// rejection, and then again if the rejection is handled later on. + +const common = require('../common'); +const assert = require('assert'); + +var b = 0; + +process.on('warning', common.mustCall((warning) => { + switch (b++) { + case 0: + assert.strictEqual(warning.name, 'UnhandledPromiseRejectionWarning'); + assert(/Unhandled promise rejection/.test(warning.message)); + break; + case 1: + assert.strictEqual(warning.name, 'PromiseRejectionHandledWarning'); + assert(/Promise rejection was handled asynchronously/ + .test(warning.message)); + } +}, 2)); + +const p = Promise.reject('This was rejected'); +setImmediate(common.mustCall(() => p.catch(() => {}))); From 95ac7bf7163d94ca48bbaced932ff7d01cf8517d Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 21 Aug 2016 21:16:40 -0700 Subject: [PATCH 2/2] promise: hard deprecation for unhandled promise rejection PR-URL: https://github.com/nodejs/node/pull/8217 Reviewed-By: Chris Dickinson Reviewed-By: Benjamin Gruenbaum Reviewed-By: Jeremiah Senkpiel --- lib/internal/process/promises.js | 9 +++++++++ .../test-promises-warning-on-unhandled-rejection.js | 5 ++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 02c90a928bbc64..e364822fa3a7bf 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -45,6 +45,7 @@ function setupPromises(scheduleMicrotasks) { } } + var deprecationWarned = false; function emitPendingUnhandledRejections() { let hadListeners = false; while (pendingUnhandledRejections.length > 0) { @@ -59,6 +60,14 @@ function setupPromises(scheduleMicrotasks) { warning.name = 'UnhandledPromiseRejectionWarning'; warning.id = uid; process.emitWarning(warning); + if (!deprecationWarned) { + deprecationWarned = true; + 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'); + } } else { hadListeners = true; } diff --git a/test/parallel/test-promises-warning-on-unhandled-rejection.js b/test/parallel/test-promises-warning-on-unhandled-rejection.js index 9b1e7f3266aba0..b50a5cae416942 100644 --- a/test/parallel/test-promises-warning-on-unhandled-rejection.js +++ b/test/parallel/test-promises-warning-on-unhandled-rejection.js @@ -16,11 +16,14 @@ process.on('warning', common.mustCall((warning) => { assert(/Unhandled promise rejection/.test(warning.message)); break; case 1: + assert.strictEqual(warning.name, 'DeprecationWarning'); + break; + case 2: assert.strictEqual(warning.name, 'PromiseRejectionHandledWarning'); assert(/Promise rejection was handled asynchronously/ .test(warning.message)); } -}, 2)); +}, 3)); const p = Promise.reject('This was rejected'); setImmediate(common.mustCall(() => p.catch(() => {})));