Skip to content

Commit 35471bc

Browse 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 <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 28edc1d commit 35471bc

File tree

2 files changed

+40
-17
lines changed

2 files changed

+40
-17
lines changed

‎lib/domain.js‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -399,32 +399,35 @@ EventEmitter.init = function(){
399399
consteventEmit=EventEmitter.prototype.emit;
400400
EventEmitter.prototype.emit=functionemit(...args){
401401
constdomain=this.domain;
402-
if(domain===null||domain===undefined||this===process){
403-
returnReflect.apply(eventEmit,this,args);
404-
}
405402

406403
consttype=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]instanceofError)){
412-
domain.emit('error',args[1]);
413-
returnfalse;
414-
}
404+
constshouldEmitError=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
returnReflect.apply(eventEmit,this,args);
419-
}catch(er){
420-
if(typeofer==='object'&&er!==null){
412+
}
413+
414+
if(type==='error'){
415+
conster=args.length>1&&args[1] ?
416+
args[1] : newerrors.Error('ERR_UNHANDLED_ERROR');
417+
418+
if(typeofer==='object'){
421419
er.domainEmitter=this;
422420
er.domain=domain;
423421
er.domainThrown=false;
424422
}
423+
425424
domain.emit('error',er);
426425
returnfalse;
427-
}finally{
428-
domain.exit();
429426
}
427+
428+
domain.enter();
429+
constret=Reflect.apply(eventEmit,this,args);
430+
domain.exit();
431+
432+
returnret;
430433
};
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
constassert=require('assert');
5+
constdomain=require('domain').create();
6+
constEventEmitter=require('events');
7+
8+
domain.on('error',common.mustNotCall());
9+
10+
constee=newEventEmitter();
11+
12+
constplainObject={justAn: 'object'};
13+
ee.once('error',common.mustCall((err)=>{
14+
assert.deepStrictEqual(err,plainObject);
15+
}));
16+
ee.emit('error',plainObject);
17+
18+
consterr=newError('test error');
19+
ee.once('error',common.expectsError(err));
20+
ee.emit('error',err);

0 commit comments

Comments
(0)