Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny nit: CHECK_EQ? :)

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
Original file line number Diff line number Diff line change
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn’t used outside of node.cc, right?


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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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 */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as in the other PR(s?): Do you need the linter comments if you use global.gc() instead?

}, 1));
4 changes: 3 additions & 1 deletion test/parallel/test-promises-unhandled-rejections.js
Original file line number Diff line number Diff line change
@@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: common.mustCall() instead of common.noop?

process.on('rejectionHandled', function() {
throw e2;
});
Expand Down
Loading