From cfaa1e224d009f2152eb074e7e15aacc741b125a Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Fri, 28 Jan 2022 21:11:26 +0200 Subject: [PATCH] process: unhandledRejection support more errors Support cross realm errors where `instanceof` errors in our unhandledRejection warning to print better error with stack traces. PR-URL: https://github.com/nodejs/node/pull/41682 Refs: https://github.com/nodejs/node/issues/41676 Reviewed-By: Nitzan Uziely Reviewed-By: Tierney Cyren --- lib/internal/process/promises.js | 22 ++++++++++++++++---- test/parallel/test-promise-unhandled-warn.js | 19 +++++++++++++++-- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 76099b1032a466..fd5cb73fbbe4bb 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -5,6 +5,7 @@ const { ArrayPrototypeShift, Error, ObjectDefineProperty, + ObjectPrototypeHasOwnProperty, SafeWeakMap, } = primordials; @@ -79,6 +80,12 @@ function hasRejectionToWarn() { return tickInfo[kHasRejectionToWarn] === 1; } +function isErrorLike(o) { + return typeof o === 'object' && + o !== null && + ObjectPrototypeHasOwnProperty(o, 'stack'); +} + function getUnhandledRejectionsMode() { const { getOptionValue } = require('internal/options'); switch (getOptionValue('--unhandled-rejections')) { @@ -179,14 +186,21 @@ function emitUnhandledRejectionWarning(uid, reason) { `(rejection id: ${uid})` ); try { - if (reason instanceof Error) { + if (isErrorLike(reason)) { warning.stack = reason.stack; process.emitWarning(reason.stack, unhandledRejectionErrName); } else { process.emitWarning( noSideEffectsToString(reason), unhandledRejectionErrName); } - } catch {} + } catch { + try { + process.emitWarning( + noSideEffectsToString(reason), unhandledRejectionErrName); + } catch { + // Ignore. + } + } process.emitWarning(warning); } @@ -232,7 +246,7 @@ function processPromiseRejections() { try { switch (unhandledRejectionsMode) { case kStrictUnhandledRejections: { - const err = reason instanceof Error ? + const err = isErrorLike(reason) ? reason : generateUnhandledRejectionError(reason); // This destroys the async stack, don't clear it after triggerUncaughtException(err, true /* fromPromise */); @@ -259,7 +273,7 @@ function processPromiseRejections() { case kThrowUnhandledRejections: { const handled = emit(reason, promise, promiseInfo); if (!handled) { - const err = reason instanceof Error ? + const err = isErrorLike(reason) ? reason : generateUnhandledRejectionError(reason); // This destroys the async stack, don't clear it after triggerUncaughtException(err, true /* fromPromise */); diff --git a/test/parallel/test-promise-unhandled-warn.js b/test/parallel/test-promise-unhandled-warn.js index aab07973be40aa..f13e6d29e2f087 100644 --- a/test/parallel/test-promise-unhandled-warn.js +++ b/test/parallel/test-promise-unhandled-warn.js @@ -2,6 +2,7 @@ 'use strict'; const common = require('../common'); +const assert = require('assert'); // Verify that ignoring unhandled rejection works fine and that no warning is // logged. @@ -12,11 +13,25 @@ new Promise(() => { Promise.reject('test'); +function lookForMeInStackTrace() { + Promise.reject(new class ErrorLike { + constructor() { + Error.captureStackTrace(this); + this.message = 'ErrorLike'; + } + }()); +} +lookForMeInStackTrace(); + // Unhandled rejections trigger two warning per rejection. One is the rejection // reason and the other is a note where this warning is coming from. -process.on('warning', common.mustCall(4)); +process.on('warning', common.mustCall((reason) => { + if (reason.message.includes('ErrorLike')) { + assert.match(reason.stack, /lookForMeInStackTrace/); + } +}, 6)); process.on('uncaughtException', common.mustNotCall('uncaughtException')); -process.on('rejectionHandled', common.mustCall(2)); +process.on('rejectionHandled', common.mustCall(3)); process.on('unhandledRejection', (reason, promise) => { // Handle promises but still warn!