From ae85d5f024bad7d7d8dd14693ab52b7551d5566e 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 | 38 +++++++++++++++++++
 3 files changed, 56 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 b4a4582c3095a7..663e6f5fec6985 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();
@@ -58,12 +60,17 @@ 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;
-    if (reason instanceof Error) {
-      warning.stack = reason.stack;
+    try {
+      if (reason instanceof Error) {
+        warning.stack = reason.stack;
+      }
+    } catch (err) {
+      // ignored
     }
     process.emitWarning(warning);
     if (!deprecationWarned) {
diff --git a/src/node_util.cc b/src/node_util.cc
index c1dff77386d927..bbbea9ea2254dd 100644
--- a/src/node_util.cc
+++ b/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);
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..91808af14588d3
--- /dev/null
+++ b/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);