promises: more robust stringification

PR-URL: https://github.com/nodejs/node/pull/13784
Fixes: https://github.com/nodejs/node/issues/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>

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();
@@ -58,13 +60,18 @@ function setupPromises(scheduleMicrotasks) {
   }
 
   function emitWarning(uid, reason) {
-    const warning = new Error('Unhandled promise rejection ' +
-                              `(rejection id: ${uid}): ${String(reason)}`);
+    const warning = new Error(
+      `Unhandled promise rejection (rejection id: ${uid}): ` +
+      safeToString(reason));
     warning.name = 'UnhandledPromiseRejectionWarning';
     warning.id = uid;
+    try {
       if (reason instanceof Error) {
         warning.stack = reason.stack;
       }
+    } catch (err) {
+      // ignored
+    }
     process.emitWarning(warning);
     if (!deprecationWarned) {
       deprecationWarned = true;

src/node_util.cc

@@ -84,6 +84,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());
+}
+
 inline Local<Private> IndexToPrivateSymbol(Environment* env, uint32_t index) {
 #define V(name, _) &Environment::name,
   static Local<Private> (Environment::*const methods[])() const = {
@@ -221,6 +227,7 @@ void Initialize(Local<Object> target,
   env->SetMethod(target, "setHiddenValue", SetHiddenValue);
   env->SetMethod(target, "getPromiseDetails", GetPromiseDetails);
   env->SetMethod(target, "getProxyDetails", GetProxyDetails);
+  env->SetMethod(target, "safeToString", SafeToString);
 
   env->SetMethod(target, "startSigintWatchdog", StartSigintWatchdog);
   env->SetMethod(target, "stopSigintWatchdog", StopSigintWatchdog);

test/parallel/test-promises-unhandled-proxy-rejections.js

@@ -0,0 +1,38 @@
+'use strict';
+const common = require('../common');
+
+const expectedDeprecationWarning = 'Unhandled promise rejections are ' +
+                                   'deprecated. In the future, promise ' +
+                                   'rejections that are not handled will ' +
+                                   'terminate the Node.js process with a ' +
+                                   'non-zero exit code.';
+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({
+  DeprecationWarning: expectedDeprecationWarning,
+  UnhandledPromiseRejectionWarning: expectedPromiseWarning,
+});
+
+// ensure this doesn't crash
+Promise.reject(thorny);