diff --git a/lib/domain.js b/lib/domain.js index 4fefb913c8..32259d7a73 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -46,27 +46,6 @@ exports._stack = stack; exports.active = null; -// loading this file the first time sets up the global -// uncaughtException handler. -process.on('uncaughtException', uncaughtHandler); - -function uncaughtHandler(er) { - // if there's an active domain, then handle this there. - // Note that if this error emission throws, then it'll just crash. - if (exports.active && !exports.active._disposed) { - util._extend(er, { - domain: exports.active, - domain_thrown: true - }); - exports.active.emit('error', er); - if (exports.active) exports.active.exit(); - } else if (process.listeners('uncaughtException').length === 1) { - // if there are other handlers, then they'll take care of it. - // but if not, then we need to crash now. - throw er; - } -} - inherits(Domain, EventEmitter); function Domain() { diff --git a/lib/timers.js b/lib/timers.js index 48353c04ef..8ec6b2c852 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -86,23 +86,28 @@ function insert(item, msecs) { if (!first._onTimeout) continue; - // v0.4 compatibility: if the timer callback throws and the user's - // uncaughtException handler ignores the exception, other timers that - // expire on this tick should still run. If #2582 goes through, this - // hack should be removed. + // v0.4 compatibility: if the timer callback throws and the + // domain or uncaughtException handler ignore the exception, + // other timers that expire on this tick should still run. // // https://github.com/joyent/node/issues/2631 - if (first.domain) { - if (first.domain._disposed) continue; - first.domain.enter(); - } + var domain = first.domain; + if (domain && domain._disposed) continue; try { + if (domain) + domain.enter(); + var threw = true; first._onTimeout(); - } catch (e) { - if (!process.listeners('uncaughtException').length) throw e; - process.emit('uncaughtException', e); + if (domain) + domain.exit(); + threw = false; + } finally { + if (threw) { + process.nextTick(function() { + list.ontimeout(); + }); + } } - if (first.domain) first.domain.exit(); } } diff --git a/src/node.cc b/src/node.cc index 3e8d2d6158..3829268d77 100644 --- a/src/node.cc +++ b/src/node.cc @@ -110,9 +110,7 @@ static Persistent rss_symbol; static Persistent heap_total_symbol; static Persistent heap_used_symbol; -static Persistent listeners_symbol; -static Persistent uncaught_exception_symbol; -static Persistent emit_symbol; +static Persistent fatal_exception_symbol; static Persistent process_makeCallback; @@ -1883,47 +1881,36 @@ static void OnFatalError(const char* location, const char* message) { void FatalException(TryCatch &try_catch) { HandleScope scope; - if (listeners_symbol.IsEmpty()) { - listeners_symbol = NODE_PSYMBOL("listeners"); - uncaught_exception_symbol = NODE_PSYMBOL("uncaughtException"); - emit_symbol = NODE_PSYMBOL("emit"); - } - - Local listeners_v = process->Get(listeners_symbol); - assert(listeners_v->IsFunction()); - - Local listeners = Local::Cast(listeners_v); - - Local uncaught_exception_symbol_l = Local::New(uncaught_exception_symbol); - Local argv[1] = { uncaught_exception_symbol_l }; - Local ret = listeners->Call(process, 1, argv); + if (fatal_exception_symbol.IsEmpty()) + fatal_exception_symbol = NODE_PSYMBOL("_fatalException"); - assert(ret->IsArray()); + Local fatal_v = process->Get(fatal_exception_symbol); - Local listener_array = Local::Cast(ret); - - uint32_t length = listener_array->Length(); - // Report and exit if process has no "uncaughtException" listener - if (length == 0) { + if (!fatal_v->IsFunction()) { + // failed before the process._fatalException function was added! + // this is probably pretty bad. Nothing to do but report and exit. ReportException(try_catch, true); exit(1); } - // Otherwise fire the process "uncaughtException" event - Local emit_v = process->Get(emit_symbol); - assert(emit_v->IsFunction()); - - Local emit = Local::Cast(emit_v); + Local fatal_f = Local::Cast(fatal_v); Local error = try_catch.Exception(); - Local event_argv[2] = { uncaught_exception_symbol_l, error }; + Local argv[] = { error }; + + TryCatch fatal_try_catch; + + // this will return true if the JS layer handled it, false otherwise + Local caught = fatal_f->Call(process, ARRAY_SIZE(argv), argv); - TryCatch event_try_catch; - emit->Call(process, 2, event_argv); + if (fatal_try_catch.HasCaught()) { + // the fatal exception function threw, so we must exit + ReportException(fatal_try_catch, true); + exit(1); + } - if (event_try_catch.HasCaught()) { - // the uncaught exception event threw, so we must exit. - ReportException(event_try_catch, true); + if (false == caught->BooleanValue()) { + ReportException(try_catch, true); exit(1); } } diff --git a/src/node.js b/src/node.js index 6cce75f68e..bfd312b30f 100644 --- a/src/node.js +++ b/src/node.js @@ -38,6 +38,9 @@ process.EventEmitter = EventEmitter; // process.EventEmitter is deprecated + // do this good and early, since it handles errors. + startup.processFatal(); + startup.globalVariables(); startup.globalTimeouts(); startup.globalConsole(); @@ -211,6 +214,47 @@ return startup._lazyConstants; }; + startup.processFatal = function() { + // call into the active domain, or emit uncaughtException, + // and exit if there are no listeners. + process._fatalException = function(er) { + var caught = false; + if (process.domain) { + var domain = process.domain; + + // ignore errors on disposed domains. + // + // XXX This is a bit stupid. We should probably get rid of + // domain.dispose() altogether. It's almost always a terrible + // idea. --isaacs + if (domain._disposed) + return true; + + er.domain = domain; + er.domain_thrown = true; + // wrap this in a try/catch so we don't get infinite throwing + try { + // One of three things will happen here. + // + // 1. There is a handler, caught = true + // 2. There is no handler, caught = false + // 3. It throws, caught = false + // + // If caught is false after this, then there's no need to exit() + // the domain, because we're going to crash the process anyway. + caught = domain.emit('error', er); + domain.exit(); + } catch (er2) { + caught = false; + } + } else { + caught = process.emit('uncaughtException', er); + } + // if someone handled it, then great. otherwise, die in C++ land + return caught; + }; + }; + var assert; startup.processAssert = function() { // Note that calls to assert() are pre-processed out by JS2C for the diff --git a/test/simple/test-domain.js b/test/simple/test-domain.js index a478c1e91d..183e2deb92 100644 --- a/test/simple/test-domain.js +++ b/test/simple/test-domain.js @@ -26,14 +26,15 @@ var common = require('../common'); var assert = require('assert'); var domain = require('domain'); var events = require('events'); +var fs = require('fs'); var caught = 0; -var expectCaught = 8; +var expectCaught = 0; var d = new domain.Domain(); var e = new events.EventEmitter(); d.on('error', function(er) { - console.error('caught', er); + console.error('caught', er && (er.message || er)); var er_message = er.message; var er_path = er.path @@ -110,24 +111,58 @@ d.on('error', function(er) { break; default: - console.error('unexpected error, throwing %j', er.message); + console.error('unexpected error, throwing %j', er.message || er); throw er; } caught++; }); + + process.on('exit', function() { - console.error('exit'); - assert.equal(caught, expectCaught); + console.error('exit', caught, expectCaught); + assert.equal(caught, expectCaught, 'caught the expected number of errors'); console.log('ok'); }); +// catch thrown errors no matter how many times we enter the event loop +// this only uses implicit binding, except for the first function +// passed to d.run(). The rest are implicitly bound by virtue of being +// set up while in the scope of the d domain. +d.run(function() { + process.nextTick(function() { + var i = setInterval(function () { + clearInterval(i); + setTimeout(function() { + fs.stat('this file does not exist', function(er, stat) { + // uh oh! stat isn't set! + // pretty common error. + console.log(stat.isDirectory()); + }); + }); + }); + }); +}); +expectCaught++; + + + +// implicit addition of a timer created within a domain-bound context. +d.run(function() { + setTimeout(function() { + throw new Error('implicit timer'); + }); +}); +expectCaught++; + + // Event emitters added to the domain have their errors routed. d.add(e); e.emit('error', new Error('emitted')); +expectCaught++; @@ -141,6 +176,9 @@ function fn(er) { var bound = d.intercept(fn); bound(new Error('bound')); +expectCaught++; + + // intercepted should never pass first argument to callback function fn2(data) { @@ -169,36 +207,16 @@ function thrower() { throw new Error('thrown'); } setTimeout(d.bind(thrower), 100); +expectCaught++; // Pass an intercepted function to an fs operation that fails. -var fs = require('fs'); fs.open('this file does not exist', 'r', d.intercept(function(er) { console.error('should not get here!', er); throw new Error('should not get here!'); }, true)); - - - -// catch thrown errors no matter how many times we enter the event loop -// this only uses implicit binding, except for the first function -// passed to d.run(). The rest are implicitly bound by virtue of being -// set up while in the scope of the d domain. -d.run(function() { - process.nextTick(function() { - var i = setInterval(function () { - clearInterval(i); - setTimeout(function() { - fs.stat('this file does not exist', function(er, stat) { - // uh oh! stat isn't set! - // pretty common error. - console.log(stat.isDirectory()); - }); - }); - }); - }); -}); +expectCaught++; @@ -213,16 +231,9 @@ setTimeout(function() { // escape from the domain, but implicit is still bound to it. implicit.emit('error', new Error('implicit')); }, 10); +expectCaught++; - -// implicit addition of a timer created within a domain-bound context. -d.run(function() { - setTimeout(function() { - throw new Error('implicit timer'); - }); -}); - var result = d.run(function () { return 'return value'; }); @@ -231,3 +242,4 @@ assert.equal(result, 'return value'); var fst = fs.createReadStream('stream for nonexistent file') d.add(fst) +expectCaught++;