promises: more robust stringification

Backport-PR-URL: #17833 PR-URL: #13784 Fixes: #13771 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jan 18, 2018 · 3e7f0fc · 3e7f0fc
1 parent 3639d0c
commit 3e7f0fc
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 4 deletions.
diff --git 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
@@ -54,6 +54,12 @@ static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(ret);
 }
 
+// Side effect-free stringification that will never throw exceptions.
+static void SafeToString(const FunctionCallbackInfo<Value>& args) {
+  auto context = args.GetIsolate()->GetCurrentContext();
+  args.GetReturnValue().Set(args[0]->ToDetailString(context).ToLocalChecked());
+}
+
 static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
@@ -122,6 +128,7 @@ void Initialize(Local<Object> 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
@@ -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);