From 83261d2005e8e640c53e3fb10eb5cc473c62ca25 Mon Sep 17 00:00:00 2001
From: Timothy Gu <timothygu99@gmail.com>
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

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              | 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<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
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);