Skip to content

Commit

Permalink
lib,src: throw on unhanded promise rejections
Browse files Browse the repository at this point in the history
src: use std::map for the promise reject map

Refs: nodejs#5292
Refs: nodejs/promises#26
Refs: nodejs#6355
Refs: nodejs#6375
  • Loading branch information
Fishrock123 authored and BridgeAR committed Sep 26, 2017
1 parent 456d8e2 commit 711d699
Show file tree
Hide file tree
Showing 18 changed files with 493 additions and 40 deletions.
4 changes: 2 additions & 2 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@
const { clearIdStack, asyncIdStackSize } = async_wrap;
const { kAfter, kCurrentAsyncId, kInitTriggerId } = async_wrap.constants;

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

// It's possible that kInitTriggerId was set for a constructor call that
Expand All @@ -369,7 +369,7 @@
if (process.domain && process.domain._errorHandler)
caught = process.domain._errorHandler(er);

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

// If someone handled it, then great. otherwise, die in C++ land
Expand Down
32 changes: 7 additions & 25 deletions lib/internal/process/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,13 @@ const { safeToString } = process.binding('util');

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) {
let deprecationWarned = false;
const promiseInternals = {};

process._setupPromises(function(event, promise, reason) {
if (event === promiseRejectEvent.unhandled)
Expand All @@ -25,34 +19,23 @@ function setupPromises(scheduleMicrotasks) {
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);
});
}

Expand Down Expand Up @@ -90,9 +73,8 @@ function setupPromises(scheduleMicrotasks) {
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
37 changes: 36 additions & 1 deletion node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,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 @@ -270,6 +271,8 @@
'src/node_revert.h',
'src/node_i18n.h',
'src/pipe_wrap.h',
'src/track-promise.h',
'src/track-promise-inl.h',
'src/tty_wrap.h',
'src/tcp_wrap.h',
'src/udp_wrap.h',
Expand Down Expand Up @@ -654,7 +657,39 @@
'<(SHARED_INTERMEDIATE_DIR)', # for node_natives.h
],

'defines': [ 'NODE_WANT_INTERNALS=1' ],
'libraries': [
'<(OBJ_GEN_PATH)/node_javascript.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_debug_options.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/async-wrap.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/env.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_buffer.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_i18n.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_url.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/debug-agent.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/util.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/string_bytes.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/string_search.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/stream_base.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_constants.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/node_revert.<(OBJ_SUFFIX)',
'<(OBJ_TRACING_PATH)/agent.<(OBJ_SUFFIX)',
'<(OBJ_TRACING_PATH)/node_trace_buffer.<(OBJ_SUFFIX)',
'<(OBJ_TRACING_PATH)/node_trace_writer.<(OBJ_SUFFIX)',
'<(OBJ_TRACING_PATH)/trace_event.<(OBJ_SUFFIX)',
'<(OBJ_PATH)/track-promise.<(OBJ_SUFFIX)',
],

'defines': [
# gtest's ASSERT macros conflict with our own.
'GTEST_DONT_DEFINE_ASSERT_EQ=1',
'GTEST_DONT_DEFINE_ASSERT_GE=1',
'GTEST_DONT_DEFINE_ASSERT_GT=1',
'GTEST_DONT_DEFINE_ASSERT_LE=1',
'GTEST_DONT_DEFINE_ASSERT_LT=1',
'GTEST_DONT_DEFINE_ASSERT_NE=1',
'NODE_WANT_INTERNALS=1',
],

'sources': [
'src/node_platform.cc',
Expand Down
3 changes: 2 additions & 1 deletion src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ inline Environment* Environment::GetCurrent(

inline Environment::Environment(IsolateData* isolate_data,
v8::Local<v8::Context> context)
: isolate_(context->GetIsolate()),
: promise_tracker_(this),
isolate_(context->GetIsolate()),
isolate_data_(isolate_data),
async_hooks_(context->GetIsolate()),
timer_base_(uv_now(isolate_data->event_loop())),
Expand Down
11 changes: 9 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#endif
#include "handle_wrap.h"
#include "req-wrap.h"
#include "track-promise.h"
#include "tree.h"
#include "util.h"
#include "uv.h"
#include "v8.h"
Expand Down Expand Up @@ -236,6 +238,7 @@ class ModuleWrap;
V(preference_string, "preference") \
V(priority_string, "priority") \
V(produce_cached_data_string, "produceCachedData") \
V(promise_rejection_index_string, "_promiseRejectionIndex") \
V(raw_string, "raw") \
V(read_host_object_string, "_readHostObject") \
V(readable_string, "readable") \
Expand Down Expand Up @@ -294,6 +297,7 @@ class ModuleWrap;
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 @@ -312,8 +316,8 @@ class ModuleWrap;
V(performance_entry_callback, v8::Function) \
V(performance_entry_template, v8::Function) \
V(process_object, v8::Object) \
V(promise_reject_function, v8::Function) \
V(promise_wrap_template, v8::ObjectTemplate) \
V(promise_unhandled_rejection_function, v8::Function) \
V(promise_unhandled_rejection, v8::Function) \
V(push_values_to_array_function, v8::Function) \
V(randombytes_constructor_template, v8::ObjectTemplate) \
V(script_context_constructor_template, v8::FunctionTemplate) \
Expand Down Expand Up @@ -678,6 +682,9 @@ class Environment {
bool RemovePromiseHook(promise_hook_func fn, void* arg);
bool EmitNapiWarning();

PromiseTracker promise_tracker_;
int64_t promise_tracker_index_ = 0;

private:
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
const char* errmsg);
Expand Down
Loading

0 comments on commit 711d699

Please sign in to comment.