From 67e4966baca78d78e06add1d6a24d68e8f91b492 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Wed, 11 Nov 2020 14:52:07 +0200 Subject: [PATCH 1/3] promise: emit error on domain unhandled rejections --- lib/internal/process/promises.js | 22 ++++++++++++++-------- test/parallel/test-domain-promise.js | 10 ++++++++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 2b806e7e8e41b4..ad21152dd12a08 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -117,7 +117,8 @@ function unhandledRejection(promise, reason) { maybeUnhandledPromises.set(promise, { reason, uid: ++lastPromiseId, - warned: false + warned: false, + domain: process.domain }); // This causes the promise to be referenced at least for one tick. pendingUnhandledRejections.push(promise); @@ -192,26 +193,32 @@ function processPromiseRejections() { } promiseInfo.warned = true; const { reason, uid } = promiseInfo; + function emit(reason, promise, promiseInfo) { + if (promiseInfo.domain) { + return promiseInfo.domain.emit('error', reason); + } + return process.emit('unhandledRejection', reason, promise); + } switch (unhandledRejectionsMode) { case kStrictUnhandledRejections: { const err = reason instanceof Error ? reason : generateUnhandledRejectionError(reason); triggerUncaughtException(err, true /* fromPromise */); - const handled = process.emit('unhandledRejection', reason, promise); + const handled = emit(reason, promise, promiseInfo); if (!handled) emitUnhandledRejectionWarning(uid, reason); break; } case kIgnoreUnhandledRejections: { - process.emit('unhandledRejection', reason, promise); + emit(reason, promise, promiseInfo); break; } case kAlwaysWarnUnhandledRejections: { - process.emit('unhandledRejection', reason, promise); + emit(reason, promise, promiseInfo); emitUnhandledRejectionWarning(uid, reason); break; } case kThrowUnhandledRejections: { - const handled = process.emit('unhandledRejection', reason, promise); + const handled = emit(reason, promise, promiseInfo); if (!handled) { const err = reason instanceof Error ? reason : generateUnhandledRejectionError(reason); @@ -220,7 +227,7 @@ function processPromiseRejections() { break; } case kWarnWithErrorCodeUnhandledRejections: { - const handled = process.emit('unhandledRejection', reason, promise); + const handled = emit(reason, promise, promiseInfo); if (!handled) { emitUnhandledRejectionWarning(uid, reason); process.exitCode = 1; @@ -266,10 +273,9 @@ function generateUnhandledRejectionError(reason) { function listenForRejections() { setPromiseRejectCallback(promiseRejectHandler); } - module.exports = { hasRejectionToWarn, setHasRejectionToWarn, listenForRejections, - processPromiseRejections + processPromiseRejections, }; diff --git a/test/parallel/test-domain-promise.js b/test/parallel/test-domain-promise.js index 2a127f3d40272b..d3b24eba9fb8ec 100644 --- a/test/parallel/test-domain-promise.js +++ b/test/parallel/test-domain-promise.js @@ -126,3 +126,13 @@ process.on('warning', common.mustNotCall()); })); })); } +{ + // Unhandled rejections become errors on the domain + const d = domain.create(); + d.on('error', common.mustCall((e) => { + assert.strictEqual(e.message, 'foo'); + })); + d.run(common.mustCall(() => { + Promise.reject(new Error('foo')); + })); +} From f6f23e5fb16865e04fd45d3757cd845adb4831c6 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Wed, 11 Nov 2020 16:00:00 +0200 Subject: [PATCH 2/3] fixup! behaviour change, remove test for old behaviour --- .../test-promises-unhandled-rejections.js | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js index bfde806b0572ec..6abfced2a31315 100644 --- a/test/parallel/test-promises-unhandled-rejections.js +++ b/test/parallel/test-promises-unhandled-rejections.js @@ -622,30 +622,6 @@ asyncTest('setImmediate + promise microtasks is too late to attach a catch' + }); }); -asyncTest( - 'Promise unhandledRejection handler does not interfere with domain' + - ' error handlers being given exceptions thrown from nextTick.', - function(done) { - const d = domain.create(); - let domainReceivedError; - d.on('error', function(e) { - domainReceivedError = e; - }); - d.run(function() { - const e = new Error('error'); - const domainError = new Error('domain error'); - onUnhandledSucceed(done, function(reason, promise) { - assert.strictEqual(reason, e); - assert.strictEqual(domainReceivedError, domainError); - }); - Promise.reject(e); - process.nextTick(function() { - throw domainError; - }); - }); - } -); - asyncTest('nextTick is immediately scheduled when called inside an event' + ' handler', function(done) { clean(); From f234181e18da1093e75b8ae1c9b96f879b0f17d4 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Wed, 11 Nov 2020 16:26:43 +0200 Subject: [PATCH 3/3] fixup! linter --- test/parallel/test-promises-unhandled-rejections.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js index 6abfced2a31315..fc2ad945bae36b 100644 --- a/test/parallel/test-promises-unhandled-rejections.js +++ b/test/parallel/test-promises-unhandled-rejections.js @@ -1,7 +1,6 @@ 'use strict'; const common = require('../common'); const assert = require('assert'); -const domain = require('domain'); const { inspect } = require('util'); common.disableCrashOnUnhandledRejection();