Skip to content

Commit

Permalink
lib,src: throw on unhanded promise rejections
Browse files Browse the repository at this point in the history 
Refs: nodejs#5292
Refs: nodejs/promises#26
Refs: nodejs#6355
PR-URL: nodejs#6375
  • Loading branch information
@Fishrock123
Fishrock123 committed Feb 8, 2017
1 parent a8734af commit 8a2e21a
Show file tree
Hide file tree
Showing 18 changed files with 406 additions and 88 deletions.
4 changes: 2 additions & 2 deletions lib/internal/bootstrap_node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,13 +282,13 @@

function setupProcessFatal() {

process._fatalException = function(er) {
process._fatalException = function(er, fromPromise) {
var caught;

if (process.domain && process.domain._errorHandler)
caught = process.domain._errorHandler(er) || caught;

if (!caught)
if (!caught && !fromPromise)
caught = process.emit('uncaughtException', er);

// If someone handled it, then great. otherwise, die in C++ land
Expand Down
52 changes: 8 additions & 44 deletions lib/internal/process/promises.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,88 +2,52 @@

const promiseRejectEvent = process._promiseRejectEvent;
const hasBeenNotifiedProperty = new WeakMap();
const promiseToGuidProperty = new WeakMap();
const pendingUnhandledRejections = [];
let lastPromiseId = 1;

exports.setup = setupPromises;

function getAsynchronousRejectionWarningObject(uid) {
return new Error('Promise rejection was handled ' +
`asynchronously (rejection id: ${uid})`);
}

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);
promiseToGuidProperty.set(promise, lastPromiseId++);
addPendingUnhandledRejection(promise, reason);
}

function rejectionHandled(promise) {
const hasBeenNotified = hasBeenNotifiedProperty.get(promise);
if (hasBeenNotified !== undefined) {
hasBeenNotifiedProperty.delete(promise);
const uid = promiseToGuidProperty.get(promise);
promiseToGuidProperty.delete(promise);
if (hasBeenNotified === true) {
let warning = null;
if (!process.listenerCount('rejectionHandled')) {
// Generate the warning object early to get a good stack trace.
warning = getAsynchronousRejectionWarningObject(uid);
}
promiseInternals.untrackPromise(promise);
process.nextTick(function() {
if (!process.emit('rejectionHandled', promise)) {
if (warning === null)
warning = getAsynchronousRejectionWarningObject(uid);
warning.name = 'PromiseRejectionHandledWarning';
warning.id = uid;
process.emitWarning(warning);
}
process.emit('rejectionHandled', promise);
});
}

}
}

function emitWarning(uid, reason) {
const warning = new Error('Unhandled promise rejection ' +
`(rejection id: ${uid}): ${reason}`);
warning.name = 'UnhandledPromiseRejectionWarning';
warning.id = uid;
if (reason instanceof Error) {
warning.stack = reason.stack;
}
process.emitWarning(warning);
if (!deprecationWarned) {
deprecationWarned = true;
process.emitWarning(
'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.',
'DeprecationWarning', 'DEP0018');
}
}
var deprecationWarned = false;
function emitPendingUnhandledRejections() {
let hadListeners = false;
while (pendingUnhandledRejections.length > 0) {
const promise = pendingUnhandledRejections.shift();
const reason = pendingUnhandledRejections.shift();
if (hasBeenNotifiedProperty.get(promise) === false) {
hasBeenNotifiedProperty.set(promise, true);
const uid = promiseToGuidProperty.get(promise);
if (!process.emit('unhandledRejection', reason, promise)) {
emitWarning(uid, reason);
promiseInternals.trackPromise(promise);
} else {
hadListeners = true;
}
Expand Down
2 changes: 2 additions & 0 deletions node.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@
'src/stream_wrap.cc',
'src/tcp_wrap.cc',
'src/timer_wrap.cc',
'src/track-promise.cc',
'src/tracing/agent.cc',
'src/tracing/node_trace_buffer.cc',
'src/tracing/node_trace_writer.cc',
Expand Down Expand Up @@ -220,6 +221,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: 5 additions & 1 deletion src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,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 @@ -250,7 +251,10 @@ namespace node {
V(module_load_list_array, v8::Array) \
V(pipe_constructor_template, v8::FunctionTemplate) \
V(process_object, v8::Object) \
V(promise_reject_function, v8::Function) \
V(promise_unhandled_rejection_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
123 changes: 116 additions & 7 deletions src/node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "req-wrap-inl.h"
#include "string_bytes.h"
#include "tracing/agent.h"
#include "track-promise.h"
#include "util.h"
#include "uv.h"
#if NODE_USE_V8_PLATFORM
Expand Down Expand Up @@ -102,6 +103,7 @@ using v8::Array;
using v8::ArrayBuffer;
using v8::Boolean;
using v8::Context;
using v8::Debug;
using v8::EscapableHandleScope;
using v8::Exception;
using v8::Float64Array;
Expand All @@ -117,6 +119,7 @@ using v8::MaybeLocal;
using v8::Message;
using v8::Name;
using v8::NamedPropertyHandlerConfiguration;
using v8::NativeWeakMap;
using v8::Null;
using v8::Number;
using v8::Object;
Expand All @@ -126,6 +129,7 @@ using v8::PromiseRejectMessage;
using v8::PropertyCallbackInfo;
using v8::ScriptOrigin;
using v8::SealHandleScope;
using v8::Set;
using v8::String;
using v8::TryCatch;
using v8::Uint32Array;
Expand Down Expand Up @@ -1173,7 +1177,7 @@ void PromiseRejectCallback(PromiseRejectMessage message) {
Local<Integer> event = Integer::New(isolate, message.GetEvent());

Environment* env = Environment::GetCurrent(isolate);
Local<Function> callback = env->promise_reject_function();
Local<Function> callback = env->promise_unhandled_rejection_function();

if (value.IsEmpty())
value = Undefined(isolate);
Expand All @@ -1184,14 +1188,78 @@ 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> internal_props =
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,
&internal_props).ToLocalChecked();
}

void TrackPromise(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> promise_value = 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->Size() > 1000) {
return;
}

if (!unhandled_reject_map->Has(promise_value) &&
!promise_value->IsUndefined()) {
unhandled_reject_map->Set(promise_value, promise);
CHECK(!unhandled_reject_keys->Add(env->context(), promise_value).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_function(args[0].As<Function>());
env->set_promise_unhandled_rejection(args[1].As<Function>());

env->SetMethod(args[2].As<Object>(), "trackPromise", TrackPromise);
env->SetMethod(args[2].As<Object>(), "untrackPromise", UntrackPromise);

env->process_object()->Delete(
env->context(),
Expand Down Expand Up @@ -1621,10 +1689,9 @@ void AppendExceptionLine(Environment* env,
arrow_str).FromMaybe(false));
}


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, FATAL_ERROR);
Expand Down Expand Up @@ -2512,6 +2579,14 @@ NO_RETURN void FatalError(const char* location, const char* message) {
void FatalException(Isolate* isolate,
Local<Value> error,
Local<Message> message) {
InternalFatalException(isolate, error, message, false);
}


void InternalFatalException(Isolate* isolate,
Local<Value> error,
Local<Message> message,
bool from_promise) {
HandleScope scope(isolate);

Environment* env = Environment::GetCurrent(isolate);
Expand All @@ -2534,9 +2609,12 @@ void FatalException(Isolate* isolate,
// Do not call FatalException when _fatalException handler throws
fatal_try_catch.SetVerbose(false);

Local<Value> argv[2] = { error,
Boolean::New(env->isolate(), from_promise) };

// this will return true if the JS layer handled it, false otherwise
Local<Value> caught =
fatal_exception_function->Call(process_object, 1, &error);
fatal_exception_function->Call(process_object, 2, argv);

if (fatal_try_catch.HasCaught()) {
// the fatal exception function threw, so we must exit
Expand Down Expand Up @@ -3464,6 +3542,12 @@ void LoadEnvironment(Environment* env) {
// Add a reference to the global object
Local<Object> global = env->context()->Global();

Local<Object> js_array_object = global->Get(
FIXED_ONE_BYTE_STRING(env->isolate(), "Array")).As<Object>();
Local<Function> js_array_from_function = js_array_object->Get(
FIXED_ONE_BYTE_STRING(env->isolate(), "from")).As<Function>();
env->set_array_from(js_array_from_function);

#if defined HAVE_DTRACE || defined HAVE_ETW
InitDTrace(env, global);
#endif
Expand Down Expand Up @@ -4463,6 +4547,31 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
} 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)) {
Local<Value> promise = unhandled_reject_map->Get(key);
Local<Value> err = GetPromiseReason(&env, promise);
Local<Message> message = Exception::CreateMessage(isolate, err);

// XXX(Fishrock123): Should this just call ReportException and
// set exit_code = 1 instead?
InternalFatalException(isolate, err, message, true);
}
}

env.set_trace_sync_io(false);

const int exit_code = EmitExit(&env);
Expand Down
13 changes: 13 additions & 0 deletions src/node_internals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ constexpr size_t arraysize(const T(&)[N]) { return N; }
bool IsExceptionDecorated(Environment* env, v8::Local<v8::Value> er);

enum ErrorHandlingMode { FATAL_ERROR, CONTEXTIFY_ERROR };

v8::Local<v8::Value> GetPromiseReason(Environment* env,
v8::Local<v8::Value> promise);

void InternalFatalException(v8::Isolate* isolate,
v8::Local<v8::Value> error,
v8::Local<v8::Message> message,
bool from_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
Loading

0 comments on commit 8a2e21a

Please sign in to comment.