Skip to content

Commit

Permalink
lib,src: throw on unhanded promise rejections
Browse files Browse the repository at this point in the history
  • Loading branch information
Fishrock123 committed Apr 28, 2017
1 parent b2c7a51 commit f6e35fe
Show file tree
Hide file tree
Showing 15 changed files with 172 additions and 114 deletions.
4 changes: 2 additions & 2 deletions lib/internal/bootstrap_node.js
Expand Up @@ -304,13 +304,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
45 changes: 2 additions & 43 deletions lib/internal/process/promises.js
Expand Up @@ -2,17 +2,10 @@

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) {
process._setupPromises(function(event, promise, reason) {
if (event === promiseRejectEvent.unhandled)
Expand All @@ -25,65 +18,31 @@ function setupPromises(scheduleMicrotasks) {

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);
}
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);
process.promiseFatal(promise);
} else {
hadListeners = true;
}
Expand Down
37 changes: 32 additions & 5 deletions src/node.cc
Expand Up @@ -1286,6 +1286,21 @@ void SetupPromises(const FunctionCallbackInfo<Value>& args) {
FIXED_ONE_BYTE_STRING(args.GetIsolate(), "_setupPromises")).FromJust();
}

void PromiseFatal(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsPromise());

Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();

Local<Promise> promise = args[0].As<Promise>();

CHECK(promise->State() == Promise::PromiseState::kRejected);
Local<Value> err = promise->Result();
Local<Message> message = Exception::CreateMessage(isolate, err);

InternalFatalException(isolate, err, message, true);
}

} // anonymous namespace


Expand Down Expand Up @@ -1719,10 +1734,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 @@ -2615,6 +2629,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 @@ -2637,9 +2659,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 @@ -3495,6 +3520,8 @@ void SetupProcessObject(Environment* env,
env->SetMethod(process, "_setupPromises", SetupPromises);
env->SetMethod(process, "_setupDomainUse", SetupDomainUse);

env->SetMethod(process, "promiseFatal", PromiseFatal);

// pre-set _events object for faster emit checks
Local<Object> events_obj = Object::New(env->isolate());
CHECK(events_obj->SetPrototype(env->context(),
Expand Down
5 changes: 5 additions & 0 deletions src/node_internals.h
Expand Up @@ -168,6 +168,11 @@ void AppendExceptionLine(Environment* env,
v8::Local<v8::Message> message,
enum ErrorHandlingMode mode);

void InternalFatalException(v8::Isolate* isolate,
v8::Local<v8::Value> error,
v8::Local<v8::Message> message,
bool from_promise);

NO_RETURN void FatalError(const char* location, const char* message);

void ProcessEmitWarning(Environment* env, const char* fmt, ...);
Expand Down
26 changes: 26 additions & 0 deletions test/message/promise_fast_handled_reject.js
@@ -0,0 +1,26 @@
'use strict';
const common = require('../common');

const p1 = new Promise((res, rej) => {
consol.log('One'); // eslint-disable-line no-undef
});

const p2 = new Promise((res, rej) => { // eslint-disable-line no-unused-vars
consol.log('Two'); // eslint-disable-line no-undef
});

const p3 = new Promise((res, rej) => {
consol.log('Three'); // eslint-disable-line no-undef
});

new Promise((res, rej) => {
setTimeout(common.mustCall(() => {
p1.catch(() => {});
p3.catch(() => {});
}), 1);
});

process.on('uncaughtException', (err) =>
common.fail('Should not trigger uncaught exception'));

process.on('exit', () => process._rawDebug('exit event emitted'));
16 changes: 16 additions & 0 deletions test/message/promise_fast_handled_reject.out
@@ -0,0 +1,16 @@
exit event emitted
*test*message*promise_fast_handled_reject.js:*
consol.log('One'); // eslint-disable-line no-undef
^

ReferenceError: consol is not defined
at Promise (*test*message*promise_fast_handled_reject.js:*:*)
at Promise (<anonymous>)
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 (bootstrap_node.js:*:*)
18 changes: 18 additions & 0 deletions test/message/promise_fast_reject.js
@@ -0,0 +1,18 @@
'use strict';

// We should always have the stacktrace of the oldest rejection.

const common = 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
});

process.on('uncaughtException', (err) =>
common.fail('Should not trigger uncaught exception'));

process.on('exit', () => process._rawDebug('exit event emitted'));
16 changes: 16 additions & 0 deletions test/message/promise_fast_reject.out
@@ -0,0 +1,16 @@
exit event emitted
*test*message*promise_fast_reject.js:*
consol.log('One'); // eslint-disable-line no-undef
^

ReferenceError: consol is not defined
at *test*message*promise_fast_reject.js:*:*
at Promise (<anonymous>)
at Object.<anonymous> (*test*message*promise_fast_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 (bootstrap_node.js:*:*)
18 changes: 18 additions & 0 deletions test/message/promise_reject.js
@@ -0,0 +1,18 @@
'use strict';
const common = require('../common');


Promise.reject(new Error('oops'));

setImmediate(() => {
common.fail('Should not reach Immediate');
});

process.on('beforeExit', () =>
common.fail('beforeExit should not be reached'));

process.on('uncaughtException', (err) => {
common.fail('Should not trigger uncaught exception');
});

process.on('exit', () => process._rawDebug('exit event emitted'));
15 changes: 15 additions & 0 deletions test/message/promise_reject.out
@@ -0,0 +1,15 @@
exit event emitted
*test*message*promise_reject.js:*
Promise.reject(new Error('oops'));
^

Error: oops
at *test*message*promise_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 (bootstrap_node.js:*:*)
at bootstrap_node.js:*:*
5 changes: 0 additions & 5 deletions test/message/unhandled_promise_trace_warnings.js

This file was deleted.

29 changes: 0 additions & 29 deletions test/message/unhandled_promise_trace_warnings.out

This file was deleted.

19 changes: 19 additions & 0 deletions test/parallel/test-promises-handled-reject.js
@@ -0,0 +1,19 @@
'use strict';
const common = require('../common');

// Flags: --expose-gc

const p = new Promise((res, rej) => {
consol.log('oops'); // eslint-disable-line no-undef
});

// Manually call GC due to possible memory contraints with attempting to
// trigger it "naturally".
process.nextTick(common.mustCall(() => {
p.catch(() => {});
/* eslint-disable no-undef */
gc();
gc();
gc();
/* eslint-enable no-undef */
}, 1));
4 changes: 3 additions & 1 deletion test/parallel/test-promises-unhandled-rejections.js
@@ -1,5 +1,5 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const domain = require('domain');

Expand Down Expand Up @@ -673,6 +673,8 @@ asyncTest('Throwing an error inside a rejectionHandled handler goes to' +
tearDownException();
done();
});
// Prevent fatal unhandled error.
process.on('unhandledRejection', common.noop);
process.on('rejectionHandled', function() {
throw e2;
});
Expand Down

0 comments on commit f6e35fe

Please sign in to comment.