Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 35471bc

Browse filesBrowse files
apapirovskiMylesBorins
authored andcommitted
domain: fix error emit handling
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>
1 parent 28edc1d commit 35471bc
Copy full SHA for 35471bc

File tree

Expand file treeCollapse file tree

2 files changed

+40
-17
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+40
-17
lines changed
Open diff view settings
Collapse file

‎lib/domain.js‎

Copy file name to clipboardExpand all lines: lib/domain.js
+20-17Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -399,32 +399,35 @@ EventEmitter.init = function() {
399399
const eventEmit = EventEmitter.prototype.emit;
400400
EventEmitter.prototype.emit = function emit(...args) {
401401
const domain = this.domain;
402-
if (domain === null || domain === undefined || this === process) {
403-
return Reflect.apply(eventEmit, this, args);
404-
}
405402

406403
const type = args[0];
407-
// edge case: if there is a domain and an existing non error object is given,
408-
// it should not be errorized
409-
// see test/parallel/test-event-emitter-no-error-provided-to-error-event.js
410-
if (type === 'error' && args.length > 1 && args[1] &&
411-
!(args[1] instanceof Error)) {
412-
domain.emit('error', args[1]);
413-
return false;
414-
}
404+
const shouldEmitError = type === 'error' &&
405+
this.listenerCount(type) > 0;
415406

416-
domain.enter();
417-
try {
407+
// Just call original `emit` if current EE instance has `error`
408+
// handler, there's no active domain or this is process
409+
if (shouldEmitError || domain === null || domain === undefined ||
410+
this === process) {
418411
return Reflect.apply(eventEmit, this, args);
419-
} catch (er) {
420-
if (typeof er === 'object' && er !== null) {
412+
}
413+
414+
if (type === 'error') {
415+
const er = args.length > 1 && args[1] ?
416+
args[1] : new errors.Error('ERR_UNHANDLED_ERROR');
417+
418+
if (typeof er === 'object') {
421419
er.domainEmitter = this;
422420
er.domain = domain;
423421
er.domainThrown = false;
424422
}
423+
425424
domain.emit('error', er);
426425
return false;
427-
} finally {
428-
domain.exit();
429426
}
427+
428+
domain.enter();
429+
const ret = Reflect.apply(eventEmit, this, args);
430+
domain.exit();
431+
432+
return ret;
430433
};
Collapse file
+20Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const domain = require('domain').create();
6+
const EventEmitter = require('events');
7+
8+
domain.on('error', common.mustNotCall());
9+
10+
const ee = new EventEmitter();
11+
12+
const plainObject = { justAn: 'object' };
13+
ee.once('error', common.mustCall((err) => {
14+
assert.deepStrictEqual(err, plainObject);
15+
}));
16+
ee.emit('error', plainObject);
17+
18+
const err = new Error('test error');
19+
ee.once('error', common.expectsError(err));
20+
ee.emit('error', err);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.