lib,src: throw on unhanded promise rejections

Refs: #5292
Refs: nodejs/promises#26
Refs: #6355

Author: Fishrock123

lib/internal/process/promises.js

@@ -7,14 +7,18 @@ const pendingUnhandledRejections = [];
 exports.setup = setupPromises;
 
 function setupPromises(scheduleMicrotasks) {
+  const promiseInternals = {};
+
   process._setupPromises(function(event, promise, reason) {
     if (event === promiseRejectEvent.unhandled)
       unhandledRejection(promise, reason);
     else if (event === promiseRejectEvent.handled)
       rejectionHandled(promise);
     else
       require('assert').fail(null, null, 'unexpected PromiseRejectEvent');
-  });
+  }, function getPromiseReason(data) {
+    return data[data.indexOf('[[PromiseValue]]') + 1];
+  }, promiseInternals);
 
   function unhandledRejection(promise, reason) {
     hasBeenNotifiedProperty.set(promise, false);
@@ -26,6 +30,7 @@ function setupPromises(scheduleMicrotasks) {
     if (hasBeenNotified !== undefined) {
       hasBeenNotifiedProperty.delete(promise);
       if (hasBeenNotified === true) {
+        promiseInternals.untrackPromise(promise);
         process.nextTick(function() {
           process.emit('rejectionHandled', promise);
         });
@@ -43,7 +48,7 @@ function setupPromises(scheduleMicrotasks) {
         hasBeenNotifiedProperty.set(promise, true);
         if (!process.emit('unhandledRejection', reason, promise)) {
           // Nobody is listening.
-          // TODO(petkaantonov) Take some default action, see #830
+          promiseInternals.onPromiseGC(promise);
         } else {
           hadListeners = true;
         }

node.gyp

@@ -153,6 +153,7 @@
         'src/stream_wrap.cc',
         'src/tcp_wrap.cc',
         'src/timer_wrap.cc',
+        'src/track-promise.cc',
         'src/tty_wrap.cc',
         'src/process_wrap.cc',
         'src/udp_wrap.cc',
@@ -181,6 +182,7 @@
         'src/node_revert.h',
         'src/node_i18n.h',
         'src/pipe_wrap.h',
+        'src/track-promise.h',
         'src/tty_wrap.h',
         'src/tcp_wrap.h',
         'src/udp_wrap.h',

src/env.h

@@ -63,6 +63,8 @@ namespace node {
   V(address_string, "address")                                                \
   V(args_string, "args")                                                      \
   V(argv_string, "argv")                                                      \
+  V(array_class_string, "Array")                                              \
+  V(array_from_string, "from")                                                \
   V(async, "async")                                                           \
   V(async_queue_string, "_asyncQueue")                                        \
   V(atime_string, "atime")                                                    \
@@ -254,6 +256,7 @@ namespace node {
   V(zero_return_string, "ZERO_RETURN")                                        \
 
 #define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)                           \
+  V(array_from, v8::Function)                                                 \
   V(as_external, v8::External)                                                \
   V(async_hooks_destroy_function, v8::Function)                               \
   V(async_hooks_init_function, v8::Function)                                  \
@@ -272,6 +275,9 @@ namespace node {
   V(pipe_constructor_template, v8::FunctionTemplate)                          \
   V(process_object, v8::Object)                                               \
   V(promise_reject_function, v8::Function)                                    \
+  V(promise_unhandled_rejection, v8::Function)                                \
+  V(promise_unhandled_reject_map, v8::NativeWeakMap)                          \
+  V(promise_unhandled_reject_keys, v8::Set)                                   \
   V(push_values_to_array_function, v8::Function)                              \
   V(script_context_constructor_template, v8::FunctionTemplate)                \
   V(script_data_constructor_function, v8::Function)                           \

src/node.cc

@@ -37,6 +37,7 @@
 #include "req-wrap.h"
 #include "req-wrap-inl.h"
 #include "string_bytes.h"
+#include "track-promise.h"
 #include "util.h"
 #include "uv.h"
 #include "libplatform/libplatform.h"
@@ -104,6 +105,7 @@ using v8::Array;
 using v8::ArrayBuffer;
 using v8::Boolean;
 using v8::Context;
+using v8::Debug;
 using v8::EscapableHandleScope;
 using v8::Exception;
 using v8::Function;
@@ -118,6 +120,7 @@ using v8::Locker;
 using v8::MaybeLocal;
 using v8::Message;
 using v8::Name;
+using v8::NativeWeakMap;
 using v8::Null;
 using v8::Number;
 using v8::Object;
@@ -127,6 +130,7 @@ using v8::PromiseRejectMessage;
 using v8::PropertyCallbackInfo;
 using v8::ScriptOrigin;
 using v8::SealHandleScope;
+using v8::Set;
 using v8::String;
 using v8::TryCatch;
 using v8::Uint32;
@@ -1136,14 +1140,77 @@ void PromiseRejectCallback(PromiseRejectMessage message) {
   callback->Call(process, arraysize(args), args);
 }
 
+Local<Value> GetPromiseReason(Environment* env, Local<Value> promise) {
+  Local<Function> fn = env->promise_unhandled_rejection();
+
+  Local<Value> internalProps =
+      Debug::GetInternalProperties(env->isolate(),
+                                   promise).ToLocalChecked().As<Value>();
+
+  // If fn is empty we'll almost certainly have to panic anyways
+  return fn->Call(env->context(), Null(env->isolate()), 1,
+                       &internalProps).ToLocalChecked();
+}
+
+void OnPromiseGC(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+
+  CHECK(args[0]->IsObject());
+  Local<Object> promise = args[0].As<Object>();
+
+  TrackPromise::New(env->isolate(), promise);
+
+  Local<Value> err = GetPromiseReason(env, promise);
+  Local<NativeWeakMap> unhandled_reject_map =
+      env->promise_unhandled_reject_map();
+  Local<Set> unhandled_reject_keys =
+      env->promise_unhandled_reject_keys();
+
+  if (unhandled_reject_keys->AsArray()->Length() > 1000) {
+    return;
+  }
+
+  if (!unhandled_reject_map->Has(err) && !err->IsUndefined()) {
+    unhandled_reject_map->Set(err, promise);
+    CHECK(!unhandled_reject_keys->Add(env->context(), err).IsEmpty());
+  }
+}
+
+void UntrackPromise(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+
+  CHECK(args[0]->IsObject());
+  Local<Value> promise = args[0].As<Value>();
+
+  Local<Value> err = GetPromiseReason(env, promise);
+  Local<NativeWeakMap> unhandled_reject_map =
+      env->promise_unhandled_reject_map();
+  Local<Set> unhandled_reject_keys =
+      env->promise_unhandled_reject_keys();
+
+  if (unhandled_reject_keys->Has(env->context(), err).IsJust()) {
+    CHECK(unhandled_reject_keys->Delete(env->context(), err).IsJust());
+    unhandled_reject_map->Delete(err);
+  }
+}
+
 void SetupPromises(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
 
+  env->set_promise_unhandled_reject_map(NativeWeakMap::New(isolate));
+  env->set_promise_unhandled_reject_keys(Set::New(isolate));\
+
   CHECK(args[0]->IsFunction());
+  CHECK(args[1]->IsFunction());
+  CHECK(args[2]->IsObject());
 
   isolate->SetPromiseRejectCallback(PromiseRejectCallback);
   env->set_promise_reject_function(args[0].As<Function>());
+  env->set_promise_unhandled_rejection(args[1].As<Function>());
+
+  env->SetMethod(args[2].As<Object>(), "onPromiseGC", OnPromiseGC);
+  env->SetMethod(args[2].As<Object>(), "untrackPromise", UntrackPromise);
 
   env->process_object()->Delete(
       env->context(),
@@ -1572,10 +1639,17 @@ void AppendExceptionLine(Environment* env,
   PrintErrorString("\n%s", arrow);
 }
 
+void ReportPromiseRejection(Isolate* isolate, Local<Value> promise) {
+  Environment* env = Environment::GetCurrent(isolate);
+
+  Local<Value> err = GetPromiseReason(env, promise);
+
+  ReportException(env, err, Exception::CreateMessage(isolate, err));
+}
 
-static void ReportException(Environment* env,
-                            Local<Value> er,
-                            Local<Message> message) {
+void ReportException(Environment* env,
+                     Local<Value> er,
+                     Local<Message> message) {
   HandleScope scope(env->isolate());
 
   AppendExceptionLine(env, er, message);
@@ -3307,6 +3381,11 @@ void LoadEnvironment(Environment* env) {
   // Add a reference to the global object
   Local<Object> global = env->context()->Global();
 
+  Local<Object> JSArray = global->Get(env->array_class_string()).As<Object>();
+  Local<Function> JSFrom =
+      JSArray->Get(env->array_from_string()).As<Function>();
+  env->set_array_from(JSFrom);
+
 #if defined HAVE_DTRACE || defined HAVE_ETW
   InitDTrace(env, global);
 #endif
@@ -4322,6 +4401,26 @@ static void StartNodeInstance(void* arg) {
       } while (more == true);
     }
 
+    Local<Value> promise_keys_set =
+        env->promise_unhandled_reject_keys().As<Value>();
+    Local<Function> convert = env->array_from();
+    Local<Value> ret = convert->Call(env->context(),
+        Null(env->isolate()), 1, &promise_keys_set).ToLocalChecked();
+    Local<Array> promise_keys = ret.As<Array>();
+    uint32_t key_count = promise_keys->Length();
+    Local<NativeWeakMap> unhandled_reject_map =
+        env->promise_unhandled_reject_map();
+
+    for (uint32_t key_iter = 0; key_iter < key_count; key_iter++) {
+      Local<Value> key = promise_keys->Get(env->context(),
+                                           key_iter).ToLocalChecked();
+
+      if (unhandled_reject_map->Has(key)) {
+        ReportPromiseRejection(isolate, unhandled_reject_map->Get(key));
+        exit(1);
+      }
+    }
+
     env->set_trace_sync_io(false);
 
     int exit_code = EmitExit(env);

src/node_internals.h

@@ -128,6 +128,12 @@ constexpr size_t arraysize(const T(&)[N]) { return N; }
 
 bool IsExceptionDecorated(Environment* env, v8::Local<v8::Value> er);
 
+void ReportPromiseRejection(v8::Isolate* isolate, v8::Local<v8::Value> promise);
+
+void ReportException(Environment* env,
+                     v8::Local<v8::Value> er,
+                     v8::Local<v8::Message> message);
+
 void AppendExceptionLine(Environment* env,
                          v8::Local<v8::Value> er,
                          v8::Local<v8::Message> message);

src/track-promise.cc

@@ -0,0 +1,55 @@
+#include "track-promise.h"
+#include "env.h"
+#include "env-inl.h"
+#include "node_internals.h"
+
+namespace node {
+
+using v8::Function;
+using v8::Isolate;
+using v8::Local;
+using v8::Object;
+using v8::Persistent;
+using v8::Value;
+using v8::WeakCallbackData;
+
+typedef void (*FreeCallback)(Local<Object> object, Local<Function> fn);
+
+
+TrackPromise* TrackPromise::New(Isolate* isolate,
+                                Local<Object> object) {
+  return new TrackPromise(isolate, object);
+}
+
+
+Persistent<Object>* TrackPromise::persistent() {
+  return &persistent_;
+}
+
+
+TrackPromise::TrackPromise(Isolate* isolate,
+                           Local<Object> object)
+    : persistent_(isolate, object) {
+  persistent_.SetWeak(this, WeakCallback);
+  persistent_.MarkIndependent();
+}
+
+
+TrackPromise::~TrackPromise() {
+  persistent_.Reset();
+}
+
+
+void TrackPromise::WeakCallback(
+    const WeakCallbackData<Object, TrackPromise>& data) {
+  data.GetParameter()->WeakCallback(data.GetIsolate(), data.GetValue());
+}
+
+
+void TrackPromise::WeakCallback(Isolate* isolate, Local<Object> object) {
+  node::ReportPromiseRejection(isolate, object.As<Value>());
+  exit(1);
+  delete this;
+}
+
+}  // namespace node

src/track-promise.h

@@ -0,0 +1,31 @@
+#ifndef SRC_TRACK_PROMISE_H_
+#define SRC_TRACK_PROMISE_H_
+
+#include "v8.h"
+
+namespace node {
+
+class Environment;
+
+class TrackPromise {
+ public:
+  TrackPromise(v8::Isolate* isolate, v8::Local<v8::Object> object);
+  virtual ~TrackPromise();
+
+  static TrackPromise* New(v8::Isolate* isolate,
+                                  v8::Local<v8::Object> object);
+
+  inline v8::Persistent<v8::Object>* persistent();
+
+  static inline void WeakCallback(
+      const v8::WeakCallbackData<v8::Object, TrackPromise>& data);
+
+ private:
+  inline void WeakCallback(v8::Isolate* isolate, v8::Local<v8::Object> object);
+
+  v8::Persistent<v8::Object> persistent_;
+};
+
+}  // namespace node
+
+#endif  // SRC_TRACK_PROMISE_H_

test/message/promise_fast_handled_reject.js

@@ -0,0 +1,21 @@
+'use strict';
+const common = require('../common');
+
+const p1 = new Promise(function(res, rej) {
+  consol.log('One'); // eslint-disable-line no-undef
+});
+
+new Promise(function(res, rej) {
+  consol.log('Two'); // eslint-disable-line no-undef
+});
+
+const p3 = new Promise(function(res, rej) {
+  consol.log('Three'); // eslint-disable-line no-undef
+});
+
+new Promise(function(res, rej) {
+  setTimeout(common.mustCall(() => {
+    p1.catch(() => {});
+    p3.catch(() => {});
+  }));
+});

test/message/promise_fast_handled_reject.out

@@ -0,0 +1,15 @@
+*test*message*promise_fast_handled_reject.js:*
+  consol.log('Two'); // eslint-disable-line no-undef
+  ^
+
+ReferenceError: consol is not defined
+    at *test*message*promise_fast_handled_reject.js:*:*
+    at Object.<anonymous> (*test*message*promise_fast_handled_reject.js:*:*)
+    at Module._compile (module.js:*:*)
+    at Object.Module._extensions..js (module.js:*:*)
+    at Module.load (module.js:*:*)
+    at tryModuleLoad (module.js:*:*)
+    at Function.Module._load (module.js:*:*)
+    at Function.Module.runMain (module.js:*:*)
+    at startup (node.js:*:*)
+    at node.js:*:*

test/message/promise_fast_reject.js

@@ -0,0 +1,10 @@
+'use strict';
+require('../common');
+
+new Promise(function(res, rej) {
+  consol.log('One'); // eslint-disable-line no-undef
+});
+
+new Promise(function(res, rej) {
+  consol.log('Two'); // eslint-disable-line no-undef
+});