Skip to content

Commit

Permalink
lib,src: throw on unhanded promise rejections
Browse files Browse the repository at this point in the history
Refs: #5292
Refs: nodejs/promises#26
Refs: #6355
  • Loading branch information
Fishrock123 committed Apr 25, 2016
1 parent 3ee68f7 commit 7e2a08c
Show file tree
Hide file tree
Showing 16 changed files with 321 additions and 5 deletions.
9 changes: 7 additions & 2 deletions lib/internal/process/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
});
Expand All @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
6 changes: 6 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down Expand Up @@ -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) \
Expand All @@ -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) \
Expand Down
105 changes: 102 additions & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
55 changes: 55 additions & 0 deletions src/track-promise.cc
Original file line number Diff line number Diff line change
@@ -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
31 changes: 31 additions & 0 deletions src/track-promise.h
Original file line number Diff line number Diff line change
@@ -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_
21 changes: 21 additions & 0 deletions test/message/promise_fast_handled_reject.js
Original file line number Diff line number Diff line change
@@ -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(() => {});
}));
});
15 changes: 15 additions & 0 deletions test/message/promise_fast_handled_reject.out
Original file line number Diff line number Diff line change
@@ -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:*:*
10 changes: 10 additions & 0 deletions test/message/promise_fast_reject.js
Original file line number Diff line number Diff line change
@@ -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
});
Loading

0 comments on commit 7e2a08c

Please sign in to comment.