Skip to content

[v9.x backport] PRs: 17403, 17456 and 17588 #18487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
apapirovski wants to merge 3 commits into from
Closed

[v9.x backport] PRs: 17403, 17456 and 17588 #18487

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4f5ac7f
events: move domain handling from events to domain
vdeturckheim Nov 30, 2017
a3417fe
events: use Reflect.apply
apapirovski Dec 4, 2017
9b98742
domain: fix error emit handling
apapirovski Dec 10, 2017
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
Next Next commit
events: move domain handling from events to domain 
PR-URL: #17403
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
  • Loading branch information
@vdeturckheim @apapirovski
vdeturckheim authored and apapirovski committed Jan 31, 2018
commit 4f5ac7fa84ec7ebb0d2b096b324cc5d3cdb1ebc0
51 changes: 46 additions & 5 deletions lib/domain.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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();
}
};
48 changes: 6 additions & 42 deletions lib/events.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

'use strict';

var domain;
var spliceOne;

function EventEmitter() {
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -115,47 +103,26 @@ 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];

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 {
Expand All @@ -165,9 +132,6 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
listeners[i].apply(this, args);
}

if (needDomainExit)
domain.exit();

return true;
};

Expand Down