From 872702d9b712fc60d4e6b06a1e1526e0506abda0 Mon Sep 17 00:00:00 2001 From: Petka Antonov Date: Sun, 22 Feb 2015 14:44:12 +0200 Subject: [PATCH] node: implement unhandled rejection tracking Implement unhandled rejection tracking for promises as specified in https://gist.github.com/benjamingr/0237932cee84712951a2 Fixes #256 PR-URL: https://github.com/iojs/io.js/pull/758 Reviewed-By: Trevor Norris Reviewed-By: Ben Noordhuis Reviewed-By: Domenic Denicola --- src/env.h | 1 + src/node.cc | 56 ++ src/node.js | 58 +- .../parallel/test-promises-unhandled-rejections.js | 606 +++++++++++++++++++++ 4 files changed, 720 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-promises-unhandled-rejections.js diff --git a/src/env.h b/src/env.h index 18fed18..74544e4 100644 --- a/src/env.h +++ b/src/env.h @@ -235,6 +235,7 @@ 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(script_context_constructor_template, v8::FunctionTemplate) \ V(script_data_constructor_function, v8::Function) \ V(secure_context_constructor_template, v8::FunctionTemplate) \ diff --git a/src/node.cc b/src/node.cc index e060304..507e5dc 100644 --- a/src/node.cc +++ b/src/node.cc @@ -98,6 +98,8 @@ using v8::Message; using v8::Number; using v8::Object; using v8::ObjectTemplate; +using v8::Promise; +using v8::PromiseRejectMessage; using v8::PropertyCallbackInfo; using v8::String; using v8::TryCatch; @@ -982,6 +984,37 @@ void SetupNextTick(const FunctionCallbackInfo& args) { FIXED_ONE_BYTE_STRING(args.GetIsolate(), "_setupNextTick")); } +void PromiseRejectCallback(PromiseRejectMessage message) { + Local promise = message.GetPromise(); + Isolate* isolate = promise->GetIsolate(); + Local value = message.GetValue(); + Local event = Integer::New(isolate, message.GetEvent()); + + Environment* env = Environment::GetCurrent(isolate); + Local callback = env->promise_reject_function(); + + if (value.IsEmpty()) + value = Undefined(isolate); + + Local args[] = { event, promise, value }; + Local process = env->process_object(); + + callback->Call(process, ARRAY_SIZE(args), args); +} + +void SetupPromises(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + + CHECK(args[0]->IsFunction()); + + isolate->SetPromiseRejectCallback(PromiseRejectCallback); + env->set_promise_reject_function(args[0].As()); + + env->process_object()->Delete( + FIXED_ONE_BYTE_STRING(args.GetIsolate(), "_setupPromises")); +} + Handle MakeCallback(Environment* env, Handle recv, @@ -2572,6 +2605,14 @@ void StopProfilerIdleNotifier(const FunctionCallbackInfo& args) { obj->ForceSet(OneByteString(env->isolate(), str), var, v8::ReadOnly); \ } while (0) +#define READONLY_DONT_ENUM_PROPERTY(obj, str, var) \ + do { \ + obj->ForceSet(OneByteString(env->isolate(), str), \ + var, \ + static_cast(v8::ReadOnly | \ + v8::DontEnum)); \ + } while (0) + void SetupProcessObject(Environment* env, int argc, @@ -2632,6 +2673,20 @@ void SetupProcessObject(Environment* env, "modules", FIXED_ONE_BYTE_STRING(env->isolate(), node_modules_version)); + // process._promiseRejectEvent + Local promiseRejectEvent = Object::New(env->isolate()); + READONLY_DONT_ENUM_PROPERTY(process, + "_promiseRejectEvent", + promiseRejectEvent); + READONLY_PROPERTY(promiseRejectEvent, + "unhandled", + Integer::New(env->isolate(), + v8::kPromiseRejectWithNoHandler)); + READONLY_PROPERTY(promiseRejectEvent, + "handled", + Integer::New(env->isolate(), + v8::kPromiseHandlerAddedAfterReject)); + #if HAVE_OPENSSL // Stupid code to slice out the version string. { // NOLINT(whitespace/braces) @@ -2790,6 +2845,7 @@ void SetupProcessObject(Environment* env, env->SetMethod(process, "_linkedBinding", LinkedBinding); env->SetMethod(process, "_setupNextTick", SetupNextTick); + env->SetMethod(process, "_setupPromises", SetupPromises); env->SetMethod(process, "_setupDomainUse", SetupDomainUse); // pre-set _events object for faster emit checks diff --git a/src/node.js b/src/node.js index f0bee17..e14592c 100644 --- a/src/node.js +++ b/src/node.js @@ -31,6 +31,7 @@ startup.processAssert(); startup.processConfig(); startup.processNextTick(); + startup.processPromises(); startup.processStdio(); startup.processKillAndExit(); startup.processSignalHandlers(); @@ -264,8 +265,11 @@ }); }; + var addPendingUnhandledRejection; + var hasBeenNotifiedProperty = new WeakMap(); startup.processNextTick = function() { var nextTickQueue = []; + var pendingUnhandledRejections = []; var microtasksScheduled = false; // Used to run V8's micro task queue. @@ -318,7 +322,8 @@ microtasksScheduled = false; _runMicrotasks(); - if (tickInfo[kIndex] < tickInfo[kLength]) + if (tickInfo[kIndex] < tickInfo[kLength] || + emitPendingUnhandledRejections()) scheduleMicrotasks(); } @@ -388,6 +393,57 @@ nextTickQueue.push(obj); tickInfo[kLength]++; } + + function emitPendingUnhandledRejections() { + var hadListeners = false; + while (pendingUnhandledRejections.length > 0) { + var promise = pendingUnhandledRejections.shift(); + var reason = pendingUnhandledRejections.shift(); + if (hasBeenNotifiedProperty.get(promise) === false) { + hasBeenNotifiedProperty.set(promise, true); + if (!process.emit('unhandledRejection', reason, promise)) { + // Nobody is listening. + // TODO(petkaantonov) Take some default action, see #830 + } else + hadListeners = true; + } + } + return hadListeners; + } + + addPendingUnhandledRejection = function(promise, reason) { + pendingUnhandledRejections.push(promise, reason); + scheduleMicrotasks(); + }; + }; + + startup.processPromises = function() { + var promiseRejectEvent = process._promiseRejectEvent; + + function unhandledRejection(promise, reason) { + hasBeenNotifiedProperty.set(promise, false); + addPendingUnhandledRejection(promise, reason); + } + + function rejectionHandled(promise) { + var hasBeenNotified = hasBeenNotifiedProperty.get(promise); + if (hasBeenNotified !== undefined) { + hasBeenNotifiedProperty.delete(promise); + if (hasBeenNotified === true) + process.emit('rejectionHandled', promise); + } + } + + process._setupPromises(function(event, promise, reason) { + if (event === promiseRejectEvent.unhandled) + unhandledRejection(promise, reason); + else if (event === promiseRejectEvent.handled) + process.nextTick(function() { + rejectionHandled(promise); + }); + else + NativeModule.require('assert').fail('unexpected PromiseRejectEvent'); + }); }; function evalScript(name) { diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js new file mode 100644 index 0000000..997d147 --- /dev/null +++ b/test/parallel/test-promises-unhandled-rejections.js @@ -0,0 +1,606 @@ +var common = require('../common'); +var assert = require('assert'); +var domain = require('domain'); + +var asyncTest = (function() { + var asyncTestsEnabled = false; + var asyncTestLastCheck; + var asyncTestQueue = []; + var asyncTestHandle; + var currentTest = null; + + function fail(error) { + var stack = currentTest + ? error.stack + '\nFrom previous event:\n' + currentTest.stack + : error.stack; + + if (currentTest) + process.stderr.write('\'' + currentTest.description + '\' failed\n\n'); + + process.stderr.write(stack); + process.exit(2); + } + + function nextAsyncTest() { + var called = false; + function done(err) { + if (called) return fail(new Error('done called twice')); + called = true; + asyncTestLastCheck = Date.now(); + if (arguments.length > 0) return fail(err); + setTimeout(nextAsyncTest, 10); + } + + if (asyncTestQueue.length) { + var test = asyncTestQueue.shift(); + currentTest = test; + test.action(done); + } else { + clearInterval(asyncTestHandle); + } + } + + return function asyncTest(description, fn) { + var stack = new Error().stack.split('\n').slice(1).join('\n'); + asyncTestQueue.push({ + action: fn, + stack: stack, + description: description + }); + if (!asyncTestsEnabled) { + asyncTestsEnabled = true; + asyncTestLastCheck = Date.now(); + process.on('uncaughtException', fail); + asyncTestHandle = setInterval(function() { + var now = Date.now(); + if (now - asyncTestLastCheck > 10000) { + return fail(new Error('Async test timeout exceeded')); + } + }, 10); + setTimeout(nextAsyncTest, 10); + } + }; + +})(); + +function setupException(fn) { + var listeners = process.listeners('uncaughtException'); + process.removeAllListeners('uncaughtException'); + process.on('uncaughtException', fn); + return function clean() { + process.removeListener('uncaughtException', fn); + listeners.forEach(function(listener) { + process.on('uncaughtException', listener); + }); + }; +} + +function clean() { + process.removeAllListeners('unhandledRejection'); + process.removeAllListeners('rejectionHandled'); +} + +function onUnhandledSucceed(done, predicate) { + clean(); + process.on('unhandledRejection', function(reason, promise) { + try { + predicate(reason, promise); + } catch (e) { + return done(e); + } + done(); + }); +} + +function onUnhandledFail(done) { + clean(); + process.on('unhandledRejection', function(reason, promise) { + done(new Error('unhandledRejection not supposed to be triggered')); + }); + process.on('rejectionHandled', function() { + done(new Error('rejectionHandled not supposed to be triggered')); + }); + setTimeout(function() { + done(); + }, 10); +} + +asyncTest('synchronously rejected promise should trigger unhandledRejection', function(done) { + var e = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + }); + Promise.reject(e); +}); + +asyncTest('synchronously rejected promise should trigger unhandledRejection', function(done) { + var e = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + }); + new Promise(function(_, reject) { + reject(e); + }); +}); + +asyncTest('Promise rejected after setImmediate should trigger unhandledRejection', function(done) { + var e = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + }); + new Promise(function(_, reject) { + setImmediate(function() { + reject(e); + }); + }); +}); + +asyncTest('Promise rejected after setTimeout(,1) should trigger unhandled rejection', function(done) { + var e = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + }); + new Promise(function(_, reject) { + setTimeout(function() { + reject(e); + }, 1); + }); +}); + +asyncTest('Catching a promise rejection after setImmediate is not soon enough to stop unhandledRejection', function(done) { + var e = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + }); + var _reject; + var promise = new Promise(function(_, reject) { + _reject = reject; + }) + _reject(e); + setImmediate(function() { + promise.then(assert.fail, function(){}); + }); +}); + +asyncTest('When re-throwing new errors in a promise catch, only the re-thrown error should hit unhandledRejection', function(done) { + var e = new Error(); + var e2 = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e2, reason); + assert.strictEqual(promise2, promise); + }); + var promise2 = Promise.reject(e).then(assert.fail, function(reason) { + assert.strictEqual(e, reason); + throw e2; + }); +}); + +asyncTest('Test params of unhandledRejection for a synchronously-rejected promise', function(done) { + var e = new Error(); + var e2 = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + assert.strictEqual(promise, promise); + }); + var promise = Promise.reject(e); +}); + +asyncTest('When re-throwing new errors in a promise catch, only the re-thrown error should hit unhandledRejection: original promise rejected async with setTimeout(,1)', function(done) { + var e = new Error(); + var e2 = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e2, reason); + assert.strictEqual(promise2, promise); + }); + var promise2 = new Promise(function(_, reject) { + setTimeout(function() { + reject(e); + }, 1); + }).then(assert.fail, function(reason) { + assert.strictEqual(e, reason); + throw e2; + }); +}); + +asyncTest('When re-throwing new errors in a promise catch, only the re-thrown error should hit unhandledRejection: promise catch attached a process.nextTick after rejection', function(done) { + var e = new Error(); + var e2 = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e2, reason); + assert.strictEqual(promise2, promise); + }); + var promise = new Promise(function(_, reject) { + setTimeout(function() { + reject(e); + process.nextTick(function() { + promise2 = promise.then(assert.fail, function(reason) { + assert.strictEqual(e, reason); + throw e2; + }); + }); + }, 1); + }); + var promise2; +}); + +asyncTest('unhandledRejection should not be triggered if a promise catch is attached synchronously upon the promise\'s creation', function(done) { + var e = new Error(); + onUnhandledFail(done); + Promise.reject(e).then(assert.fail, function(){}); +}); + +asyncTest('unhandledRejection should not be triggered if a promise catch is attached synchronously upon the promise\'s creation', function(done) { + var e = new Error(); + onUnhandledFail(done); + new Promise(function(_, reject) { + reject(e); + }).then(assert.fail, function(){}); +}); + +asyncTest('Attaching a promise catch in a process.nextTick is soon enough to prevent unhandledRejection', function(done) { + var e = new Error(); + onUnhandledFail(done); + var promise = Promise.reject(e); + process.nextTick(function() { + promise.then(assert.fail, function(){}); + }); +}); + +asyncTest('Attaching a promise catch in a process.nextTick is soon enough to prevent unhandledRejection', function(done) { + var e = new Error(); + onUnhandledFail(done); + var promise = new Promise(function(_, reject) { + reject(e); + }); + process.nextTick(function() { + promise.then(assert.fail, function(){}); + }); +}); + +// State adapation tests +asyncTest('catching a promise which is asynchronously rejected (via resolution to an asynchronously-rejected promise) prevents unhandledRejection', function(done) { + var e = new Error(); + onUnhandledFail(done); + Promise.resolve().then(function() { + return new Promise(function(_, reject) { + setTimeout(function() { + reject(e); + }, 1); + }); + }).then(assert.fail, function(reason) { + assert.strictEqual(e, reason); + }); +}); + +asyncTest('Catching a rejected promise derived from throwing in a fulfillment handler prevents unhandledRejection', function(done) { + var e = new Error(); + onUnhandledFail(done); + Promise.resolve().then(function() { + throw e; + }).then(assert.fail, function(reason) { + assert.strictEqual(e, reason); + }); +}); + +asyncTest('Catching a rejected promise derived from returning a synchronously-rejected promise in a fulfillment handler prevents unhandledRejection', function(done) { + var e = new Error(); + onUnhandledFail(done); + Promise.resolve().then(function() { + return Promise.reject(e); + }).then(assert.fail, function(reason) { + assert.strictEqual(e, reason); + }); +}); + +asyncTest('A rejected promise derived from returning an asynchronously-rejected promise in a fulfillment handler does trigger unhandledRejection', function(done) { + var e = new Error(); + var _promise; + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + assert.strictEqual(_promise, promise); + }); + _promise = Promise.resolve().then(function() { + return new Promise(function(_, reject) { + setTimeout(function() { + reject(e); + }, 1); + }); + }); +}); + +asyncTest('A rejected promise derived from throwing in a fulfillment handler does trigger unhandledRejection', function(done) { + var e = new Error(); + var _promise; + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + assert.strictEqual(_promise, promise); + }); + _promise = Promise.resolve().then(function() { + throw e; + }); +}); + +asyncTest('A rejected promise derived from returning a synchronously-rejected promise in a fulfillment handler does trigger unhandledRejection', function(done) { + var e = new Error(); + var _promise; + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + assert.strictEqual(_promise, promise); + }); + _promise = Promise.resolve().then(function() { + return Promise.reject(e); + }); +}); + +// Combinations with Promise.all +asyncTest('Catching the Promise.all() of a collection that includes a rejected promise prevents unhandledRejection', function(done) { + var e = new Error(); + onUnhandledFail(done); + Promise.all([Promise.reject(e)]).then(assert.fail, function() {}); +}); + +asyncTest('Catching the Promise.all() of a collection that includes a nextTick-async rejected promise prevents unhandledRejection', function(done) { + var e = new Error(); + onUnhandledFail(done); + var p = new Promise(function(_, reject) { + process.nextTick(function() { + reject(e); + }); + }); + p = Promise.all([p]); + process.nextTick(function() { + p.then(assert.fail, function() {}); + }); +}); + +asyncTest('Failing to catch the Promise.all() of a collection that includes a rejected promise triggers unhandledRejection for the returned promise, not the passed promise', function(done) { + var e = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + assert.strictEqual(p, promise); + }); + var p = Promise.all([Promise.reject(e)]); +}); + +asyncTest('Waiting setTimeout(, 10) to catch a promise causes an unhandledRejection + rejectionHandled pair', function(done) { + clean(); + var unhandledPromises = []; + var e = new Error(); + process.on('unhandledRejection', function(reason, promise) { + assert.strictEqual(e, reason); + unhandledPromises.push(promise); + }); + process.on('rejectionHandled', function(promise) { + assert.strictEqual(1, unhandledPromises.length); + assert.strictEqual(unhandledPromises[0], promise); + assert.strictEqual(thePromise, promise); + done(); + }); + + var thePromise = new Promise(function() { + throw e; + }); + setTimeout(function() { + thePromise.then(assert.fail, function(reason) { + assert.strictEqual(e, reason); + }); + }, 10); +}); + +asyncTest('Waiting for some combination of process.nextTick + promise microtasks to attach a catch handler is still soon enough to prevent unhandledRejection', function(done) { + var e = new Error(); + onUnhandledFail(done); + + + var a = Promise.reject(e); + process.nextTick(function() { + Promise.resolve().then(function() { + process.nextTick(function() { + Promise.resolve().then(function() { + a.catch(function() {}); + }); + }); + }); + }); +}); + +asyncTest('Waiting for some combination of process.nextTick + promise microtasks to attach a catch handler is still soon enough to prevent unhandledRejection: inside setImmediate', function(done) { + var e = new Error(); + onUnhandledFail(done); + + setImmediate(function() { + var a = Promise.reject(e); + process.nextTick(function() { + Promise.resolve().then(function() { + process.nextTick(function() { + Promise.resolve().then(function() { + a.catch(function() {}); + }); + }); + }); + }); + }); +}); + +asyncTest('Waiting for some combination of process.nextTick + promise microtasks to attach a catch handler is still soon enough to prevent unhandledRejection: inside setTimeout', function(done) { + var e = new Error(); + onUnhandledFail(done); + + setTimeout(function() { + var a = Promise.reject(e); + process.nextTick(function() { + Promise.resolve().then(function() { + process.nextTick(function() { + Promise.resolve().then(function() { + a.catch(function() {}); + }); + }); + }); + }); + }, 0); +}); + +asyncTest('Waiting for some combination of promise microtasks + process.nextTick to attach a catch handler is still soon enough to prevent unhandledRejection', function(done) { + var e = new Error(); + onUnhandledFail(done); + + + var a = Promise.reject(e); + Promise.resolve().then(function() { + process.nextTick(function() { + Promise.resolve().then(function() { + process.nextTick(function() { + a.catch(function() {}); + }); + }); + }); + }); +}); + +asyncTest('Waiting for some combination of promise microtasks + process.nextTick to attach a catch handler is still soon enough to prevent unhandledRejection: inside setImmediate', function(done) { + var e = new Error(); + onUnhandledFail(done); + + setImmediate(function() { + var a = Promise.reject(e); + Promise.resolve().then(function() { + process.nextTick(function() { + Promise.resolve().then(function() { + process.nextTick(function() { + a.catch(function() {}); + }); + }); + }); + }); + }); +}); + +asyncTest('Waiting for some combination of promise microtasks + process.nextTick to attach a catch handler is still soon enough to prevent unhandledRejection: inside setTimeout', function(done) { + var e = new Error(); + onUnhandledFail(done); + + setTimeout(function() { + var a = Promise.reject(e); + Promise.resolve().then(function() { + process.nextTick(function() { + Promise.resolve().then(function() { + process.nextTick(function() { + a.catch(function() {}); + }); + }); + }); + }); + }, 0); +}); + +asyncTest('setImmediate + promise microtasks is too late to attach a catch handler; unhandledRejection will be triggered in that case. (setImmediate before promise creation/rejection)', function(done) { + var e = new Error(); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(e, reason); + assert.strictEqual(p, promise); + }); + var p = Promise.reject(e); + setImmediate(function() { + Promise.resolve().then(function () { + p.catch(function(){}); + }); + }); +}); + +asyncTest('setImmediate + promise microtasks is too late to attach a catch handler; unhandledRejection will be triggered in that case (setImmediate before promise creation/rejection)', function(done) { + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(undefined, reason); + assert.strictEqual(p, promise); + }); + setImmediate(function() { + Promise.resolve().then(function () { + Promise.resolve().then(function () { + Promise.resolve().then(function () { + Promise.resolve().then(function () { + p.catch(function(){}); + }); + }); + }); + }); + }); + var p = Promise.reject(); +}); + +asyncTest('setImmediate + promise microtasks is too late to attach a catch handler; unhandledRejection will be triggered in that case (setImmediate after promise creation/rejection)', function(done) { + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(undefined, reason); + assert.strictEqual(p, promise); + }); + var p = Promise.reject(); + setImmediate(function() { + Promise.resolve().then(function () { + Promise.resolve().then(function () { + Promise.resolve().then(function () { + Promise.resolve().then(function () { + p.catch(function(){}); + }); + }); + }); + }); + }); +}); + +asyncTest('Promise unhandledRejection handler does not interfere with domain error handlers being given exceptions thrown from nextTick.', function(done) { + var d = domain.create(); + var domainReceivedError; + d.on('error', function(e) { + domainReceivedError = e; + }); + d.run(function() { + var e = new Error('error'); + var domainError = new Error('domain error'); + onUnhandledSucceed(done, function(reason, promise) { + assert.strictEqual(reason, e); + assert.strictEqual(domainReceivedError, domainError); + d.dispose(); + }); + var a = Promise.reject(e); + process.nextTick(function() { + throw domainError; + }); + }); +}); + +asyncTest('nextTick is immediately scheduled when called inside an event handler', function(done) { + clean(); + var e = new Error('error'); + process.on('unhandledRejection', function(reason, promise) { + var order = []; + process.nextTick(function() { + order.push(1); + }); + setTimeout(function() { + order.push(2); + assert.deepEqual([1,2], order); + done(); + }, 1); + }); + Promise.reject(e); +}); + +asyncTest('Throwing an error inside a rejectionHandled handler goes to unhandledException, and does not cause .catch() to throw an exception', function(done) { + clean(); + var e = new Error(); + var e2 = new Error(); + var tearDownException = setupException(function(err) { + assert.equal(e2, err); + tearDownException(); + done(); + }); + process.on('rejectionHandled', function() { + throw e2; + }); + var p = Promise.reject(e); + setTimeout(function() { + try { + p.catch(function(){}); + } catch (e) { + done(new Error('fail')); + } + }, 1); +}); -- 2.7.4