From 467e00ed0203b11646d027ed1f94a4129de6a9d4 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Wed, 21 Aug 2013 16:12:17 -0700 Subject: [PATCH] domain: move error handling directly into instance Instead of doing all the domain handling in core, allow the domain to set an error handler that'll take care of it all. This way the domain error handling can be abstracted enough for any user to use it. --- lib/domain.js | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/node.js | 57 +++++++-------------------------------------------------- 2 files changed, 61 insertions(+), 52 deletions(-) diff --git a/lib/domain.js b/lib/domain.js index 493c73c..ba514b7 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -54,8 +54,8 @@ process._setupDomainUse(_domain, _domain_flag); exports.Domain = Domain; -exports.create = exports.createDomain = function(cb) { - return new Domain(cb); +exports.create = exports.createDomain = function() { + return new Domain(); }; // it's possible to enter one domain while already inside @@ -74,6 +74,58 @@ function Domain() { this.members = []; } +Domain.prototype.members = undefined; +Domain.prototype._disposed = undefined; + +// Called by process._fatalException in case an error was thrown. +Domain.prototype._errorHandler = function errorHandler(er) { + var caught = false; + // 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 (this._disposed) + return true; + + er.domain = this; + er.domainThrown = 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 = this.emit('error', er); + + // Exit all domains on the stack. Uncaught exceptions end the + // current tick and no domains should be left on the stack + // between ticks. + stack.length = 0; + exports.active = process.domain = null; + } catch (er2) { + // The domain error handler threw! oh no! + // See if another domain can catch THIS error, + // or else crash on the original one. + // If the user already exited it, then don't double-exit. + if (this === exports.active) { + stack.pop(); + } + if (stack.length) { + exports.active = process.domain = stack[stack.length - 1]; + caught = process._fatalException(er2); + } else { + caught = false; + } + return caught; + } + return caught; +}; + Domain.prototype.enter = function() { if (this._disposed) return; diff --git a/src/node.js b/src/node.js index d827152..131745f 100644 --- a/src/node.js +++ b/src/node.js @@ -216,60 +216,15 @@ }; 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; - var domainModule = NativeModule.require('domain'); - var domainStack = domainModule._stack; - // 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.domainThrown = 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); - - // Exit all domains on the stack. Uncaught exceptions end the - // current tick and no domains should be left on the stack - // between ticks. - var domainModule = NativeModule.require('domain'); - domainStack.length = 0; - domainModule.active = process.domain = null; - } catch (er2) { - // The domain error handler threw! oh no! - // See if another domain can catch THIS error, - // or else crash on the original one. - // If the user already exited it, then don't double-exit. - if (domain === domainModule.active) - domainStack.pop(); - if (domainStack.length) { - var parentDomain = domainStack[domainStack.length - 1]; - process.domain = domainModule.active = parentDomain; - caught = process._fatalException(er2); - } else - caught = false; - } + if (process.domain && process.domain._errorHandler) { + caught = process.domain._errorHandler(er); } else { caught = process.emit('uncaughtException', er); } + // if someone handled it, then great. otherwise, die in C++ land // since that means that we'll exit the process, emit the 'exit' event if (!caught) { @@ -281,10 +236,12 @@ } catch (er) { // nothing to be done about it at this point. } - } + // if we handled an error, then make sure any ticks get processed - if (caught) + } else { setImmediate(process._tickCallback); + } + return caught; }; }; -- 2.7.4