From 3ae5cf205fade294a1120b99bea9638f3f06d92e Mon Sep 17 00:00:00 2001 From: vdeturckheim Date: Thu, 30 Nov 2017 16:40:05 +0100 Subject: [PATCH] events: move domain handling from events to domain Backport-PR-URL: https://github.com/nodejs/node/pull/18487 PR-URL: https://github.com/nodejs/node/pull/17403 Reviewed-By: Anna Henningsen Reviewed-By: Benjamin Gruenbaum Reviewed-By: Andreas Madsen Reviewed-By: Timothy Gu Reviewed-By: Anatoli Papirovski --- lib/domain.js | 51 ++++++++++++++++++++++++++++++++++++++++++++++----- lib/events.js | 48 ++++++------------------------------------------ 2 files changed, 52 insertions(+), 47 deletions(-) diff --git a/lib/domain.js b/lib/domain.js index 6c85ca2b17277b..cef09dca1c8857 100644 --- a/lib/domain.js +++ b/lib/domain.js @@ -31,11 +31,6 @@ const EventEmitter = require('events'); const errors = require('internal/errors'); const { createHook } = require('async_hooks'); -// communicate with events module, but don't require that -// module to have to load this one, since this module has -// a few side effects. -EventEmitter.usingDomains = true; - // overwrite process.domain with a getter/setter that will allow for more // effective optimizations var _domain = [null]; @@ -387,3 +382,49 @@ Domain.prototype.bind = function(cb) { return runBound; }; + +// Override EventEmitter methods to make it domain-aware. +EventEmitter.usingDomains = true; + +const eventInit = EventEmitter.init; +EventEmitter.init = function() { + this.domain = null; + if (exports.active && !(this instanceof exports.Domain)) { + this.domain = exports.active; + } + + return eventInit.call(this); +}; + +const eventEmit = EventEmitter.prototype.emit; +EventEmitter.prototype.emit = function emit(...args) { + const domain = this.domain; + if (domain === null || domain === undefined || this === process) { + return eventEmit.apply(this, args); + } + + const type = args[0]; + // edge case: if there is a domain and an existing non error object is given, + // it should not be errorized + // see test/parallel/test-event-emitter-no-error-provided-to-error-event.js + if (type === 'error' && args.length > 1 && args[1] && + !(args[1] instanceof Error)) { + domain.emit('error', args[1]); + return false; + } + + domain.enter(); + try { + return eventEmit.apply(this, args); + } catch (er) { + if (typeof er === 'object' && er !== null) { + er.domainEmitter = this; + er.domain = domain; + er.domainThrown = false; + } + domain.emit('error', er); + return false; + } finally { + domain.exit(); + } +}; diff --git a/lib/events.js b/lib/events.js index d8c542a01264ef..ed2c9343d9b445 100644 --- a/lib/events.js +++ b/lib/events.js @@ -21,7 +21,6 @@ 'use strict'; -var domain; var spliceOne; function EventEmitter() { @@ -32,9 +31,6 @@ module.exports = EventEmitter; // Backwards-compat with node 0.10.x EventEmitter.EventEmitter = EventEmitter; -EventEmitter.usingDomains = false; - -EventEmitter.prototype.domain = undefined; EventEmitter.prototype._events = undefined; EventEmitter.prototype._eventsCount = 0; EventEmitter.prototype._maxListeners = undefined; @@ -67,14 +63,6 @@ Object.defineProperty(EventEmitter, 'defaultMaxListeners', { }); EventEmitter.init = function() { - this.domain = null; - if (EventEmitter.usingDomains) { - // if there is an active domain, then attach to it. - domain = domain || require('domain'); - if (domain.active && !(this instanceof domain.Domain)) { - this.domain = domain.active; - } - } if (this._events === undefined || this._events === Object.getPrototypeOf(this)._events) { @@ -115,34 +103,19 @@ EventEmitter.prototype.emit = function emit(type, ...args) { else if (!doError) return false; - const domain = this.domain; - // If there is no 'error' event listener then throw. if (doError) { let er; if (args.length > 0) er = args[0]; - if (domain !== null && domain !== undefined) { - if (!er) { - const errors = lazyErrors(); - er = new errors.Error('ERR_UNHANDLED_ERROR'); - } - if (typeof er === 'object' && er !== null) { - er.domainEmitter = this; - er.domain = domain; - er.domainThrown = false; - } - domain.emit('error', er); - } else if (er instanceof Error) { + if (er instanceof Error) { throw er; // Unhandled 'error' event - } else { - // At least give some kind of context to the user - const errors = lazyErrors(); - const err = new errors.Error('ERR_UNHANDLED_ERROR', er); - err.context = er; - throw err; } - return false; + // At least give some kind of context to the user + const errors = lazyErrors(); + const err = new errors.Error('ERR_UNHANDLED_ERROR', er); + err.context = er; + throw err; } const handler = events[type]; @@ -150,12 +123,6 @@ EventEmitter.prototype.emit = function emit(type, ...args) { if (handler === undefined) return false; - let needDomainExit = false; - if (domain !== null && domain !== undefined && this !== process) { - domain.enter(); - needDomainExit = true; - } - if (typeof handler === 'function') { handler.apply(this, args); } else { @@ -165,9 +132,6 @@ EventEmitter.prototype.emit = function emit(type, ...args) { listeners[i].apply(this, args); } - if (needDomainExit) - domain.exit(); - return true; };