Skip to content

Commit

Permalink
process: unhandledRejection support more errors
Browse files Browse the repository at this point in the history
Support cross realm errors where `instanceof` errors in our
unhandledRejection warning to print better error with stack traces.

PR-URL: nodejs#41682
Refs: nodejs#41676
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
  • Loading branch information
benjamingr authored Jan 28, 2022
1 parent 0172d1d commit 64c4b55
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 6 deletions.
22 changes: 18 additions & 4 deletions lib/internal/process/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const {
ArrayPrototypeShift,
Error,
ObjectDefineProperty,
ObjectPrototypeHasOwnProperty,
SafeWeakMap,
} = primordials;

Expand Down Expand Up @@ -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')) {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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 */);
Expand All @@ -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 */);
Expand Down
19 changes: 17 additions & 2 deletions test/parallel/test-promise-unhandled-warn.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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!
Expand Down

0 comments on commit 64c4b55

Please sign in to comment.