From 2d6e802bd012468f8b4e6d77c0ae07bfbf55b78b Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 19 Jun 2017 16:19:55 +0800 Subject: [PATCH] promises: more robust stringification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backport-PR-URL: https://github.com/nodejs/node/pull/17833 PR-URL: https://github.com/nodejs/node/pull/13784 Fixes: https://github.com/nodejs/node/issues/13771 Reviewed-By: Anna Henningsen Reviewed-By: Rod Vagg Reviewed-By: Michaƫl Zasso Reviewed-By: Matteo Collina Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell --- lib/internal/process/promises.js | 15 ++++++--- src/node_util.cc | 7 +++++ ...est-promises-unhandled-proxy-rejections.js | 31 +++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-promises-unhandled-proxy-rejections.js diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index d976b9d38a9050..54a3ff41ec40c1 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -1,5 +1,7 @@ 'use strict'; +const { safeToString } = process.binding('util'); + const promiseRejectEvent = process._promiseRejectEvent; const hasBeenNotifiedProperty = new WeakMap(); const promiseToGuidProperty = new WeakMap(); @@ -64,12 +66,17 @@ function setupPromises(scheduleMicrotasks) { hasBeenNotifiedProperty.set(promise, true); const uid = promiseToGuidProperty.get(promise); if (!process.emit('unhandledRejection', reason, promise)) { - const warning = new Error('Unhandled promise rejection ' + - `(rejection id: ${uid}): ${reason}`); + const warning = new Error( + `Unhandled promise rejection (rejection id: ${uid}): ` + + safeToString(reason)); warning.name = 'UnhandledPromiseRejectionWarning'; warning.id = uid; - if (reason instanceof Error) { - warning.stack = reason.stack; + try { + if (reason instanceof Error) { + warning.stack = reason.stack; + } + } catch (err) { + // ignored } process.emitWarning(warning); } else { diff --git a/src/node_util.cc b/src/node_util.cc index f96e91483857c7..8b093ab842869d 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -54,6 +54,12 @@ static void GetProxyDetails(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(ret); } +// Side effect-free stringification that will never throw exceptions. +static void SafeToString(const FunctionCallbackInfo& args) { + auto context = args.GetIsolate()->GetCurrentContext(); + args.GetReturnValue().Set(args[0]->ToDetailString(context).ToLocalChecked()); +} + static void GetHiddenValue(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -122,6 +128,7 @@ void Initialize(Local target, env->SetMethod(target, "getHiddenValue", GetHiddenValue); env->SetMethod(target, "setHiddenValue", SetHiddenValue); env->SetMethod(target, "getProxyDetails", GetProxyDetails); + env->SetMethod(target, "safeToString", SafeToString); env->SetMethod(target, "startSigintWatchdog", StartSigintWatchdog); env->SetMethod(target, "stopSigintWatchdog", StopSigintWatchdog); diff --git a/test/parallel/test-promises-unhandled-proxy-rejections.js b/test/parallel/test-promises-unhandled-proxy-rejections.js new file mode 100644 index 00000000000000..9c3f364de2e53b --- /dev/null +++ b/test/parallel/test-promises-unhandled-proxy-rejections.js @@ -0,0 +1,31 @@ +'use strict'; +const common = require('../common'); + +const expectedPromiseWarning = 'Unhandled promise rejection (rejection id: ' + + '1): [object Object]'; + +function throwErr() { + throw new Error('Error from proxy'); +} + +const thorny = new Proxy({}, { + getPrototypeOf: throwErr, + setPrototypeOf: throwErr, + isExtensible: throwErr, + preventExtensions: throwErr, + getOwnPropertyDescriptor: throwErr, + defineProperty: throwErr, + has: throwErr, + get: throwErr, + set: throwErr, + deleteProperty: throwErr, + ownKeys: throwErr, + apply: throwErr, + construct: throwErr +}); + +common.expectWarning('UnhandledPromiseRejectionWarning', + expectedPromiseWarning); + +// ensure this doesn't crash +Promise.reject(thorny);