promises: more robust stringification

TimothyGu · MylesBorins · commit c64a73ba6ca4 · 2018-02-12T19:28:11.000-05:00
Backport-PR-URL: #17833 PR-URL: #13784 Fixes: #13771 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
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);
