Skip to content
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.

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

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 49 additions & 5 deletions lib/domain.js
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,52 @@ 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;

const type = args[0];
const shouldEmitError = type === 'error' &&
this.listenerCount(type) > 0;

// Just call original `emit` if current EE instance has `error`
// handler, there's no active domain or this is process
if (shouldEmitError || domain === null || domain === undefined ||
this === process) {
return Reflect.apply(eventEmit, this, args);
}

if (type === 'error') {
const er = args.length > 1 && args[1] ?
args[1] : new errors.Error('ERR_UNHANDLED_ERROR');

if (typeof er === 'object') {
er.domainEmitter = this;
er.domain = domain;
er.domainThrown = false;
}

domain.emit('error', er);
return false;
}

domain.enter();
const ret = Reflect.apply(eventEmit, this, args);
domain.exit();

return ret;
};
54 changes: 9 additions & 45 deletions lib/events.js
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,59 +103,35 @@ 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);
Reflect.apply(handler, this, args);
} else {
const len = handler.length;
const listeners = arrayClone(handler, len);
for (var i = 0; i < len; ++i)
listeners[i].apply(this, args);
Reflect.apply(listeners[i], this, args);
}

if (needDomainExit)
domain.exit();

return true;
};

Expand Down Expand Up @@ -252,7 +216,7 @@ function onceWrapper(...args) {
if (!this.fired) {
this.target.removeListener(this.type, this.wrapFn);
this.fired = true;
this.listener.apply(this.target, args);
Reflect.apply(this.listener, this.target, args);
}
}

Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-domain-ee-error-listener.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const domain = require('domain').create();
const EventEmitter = require('events');

domain.on('error', common.mustNotCall());

const ee = new EventEmitter();

const plainObject = { justAn: 'object' };
ee.once('error', common.mustCall((err) => {
assert.deepStrictEqual(err, plainObject);
}));
ee.emit('error', plainObject);

const err = new Error('test error');
ee.once('error', common.expectsError(err));
ee.emit('error', err);