Skip to content

Commit

Permalink
domain: fix error emit handling
Browse files Browse the repository at this point in the history
Fix an issue where error is never emitted on the original EventEmitter
in situations where a listener for error does exist.

Refactor to eliminate unnecessary try/catch/finally block.

Backport-PR-URL: #18487
PR-URL: #17588
Refs: #17403
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
apapirovski authored and MylesBorins committed Feb 20, 2018
1 parent 28edc1d commit 35471bc
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 17 deletions.
37 changes: 20 additions & 17 deletions lib/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -399,32 +399,35 @@ EventEmitter.init = function() {
const eventEmit = EventEmitter.prototype.emit;
EventEmitter.prototype.emit = function emit(...args) {
const domain = this.domain;
if (domain === null || domain === undefined || this === process) {
return Reflect.apply(eventEmit, 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;
}
const shouldEmitError = type === 'error' &&
this.listenerCount(type) > 0;

domain.enter();
try {
// 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);
} catch (er) {
if (typeof er === 'object' && er !== null) {
}

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;
} finally {
domain.exit();
}

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

return ret;
};
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);

0 comments on commit 35471bc

Please sign in to comment.