Skip to content

Commit 1eda843

Browse files
committed
domain: fix recursive error handler call in emit
When an EventEmitter emits 'error' and the domain's error handler throws, the exception should propagate to the parent domain, not recursively call the same domain's error handler. The bug was that currentDomain/currentStack were not updated when temporarily switching to the parent domain context during error emission. Since domainExceptionHandler checks currentDomain first, exceptions would incorrectly route back to the same domain. Fixes by updating currentDomain/currentStack alongside the ALS store.
1 parent 24483f3 commit 1eda843

File tree

2 files changed

+52
-0
lines changed

2 files changed

+52
-0
lines changed

lib/domain.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,10 +642,15 @@ EventEmitter.prototype.emit = function emit(...args) {
642642

643643
// Change the current active domain
644644
const newActiveDomain = newStack.length > 0 ? newStack[newStack.length - 1] : null;
645+
const prevCurrentDomain = currentDomain;
646+
const prevCurrentStack = currentStack;
647+
645648
if (store) {
646649
setDomainStore({ domain: newActiveDomain, stack: newStack });
647650
}
648651
exports.active = newActiveDomain;
652+
currentDomain = newActiveDomain;
653+
currentStack = newStack;
649654

650655
updateExceptionCapture();
651656

@@ -657,6 +662,8 @@ EventEmitter.prototype.emit = function emit(...args) {
657662
setDomainStore({ domain: origActiveDomain, stack: origDomainsStack });
658663
}
659664
exports.active = origActiveDomain;
665+
currentDomain = prevCurrentDomain;
666+
currentStack = prevCurrentStack;
660667
updateExceptionCapture();
661668

662669
return false;
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const domain = require('domain');
5+
6+
// Test that when a domain's error handler throws, the exception does NOT
7+
// recursively call the same domain's error handler. Instead, it should
8+
// propagate to the parent domain or crash the process.
9+
10+
let d1ErrorCount = 0;
11+
let d2ErrorCount = 0;
12+
13+
const d1 = domain.create();
14+
const d2 = domain.create();
15+
16+
d1.on('error', common.mustCall((err) => {
17+
d1ErrorCount++;
18+
assert.strictEqual(err.message, 'from d2 error handler');
19+
// d1 should only be called once - when d2's error handler throws
20+
assert.strictEqual(d1ErrorCount, 1);
21+
}));
22+
23+
d2.on('error', common.mustCall((err) => {
24+
d2ErrorCount++;
25+
// d2's error handler should only be called once for the original error
26+
assert.strictEqual(d2ErrorCount, 1);
27+
assert.strictEqual(err.message, 'original error');
28+
// Throwing here should go to d1, NOT back to d2
29+
throw new Error('from d2 error handler');
30+
}));
31+
32+
d1.run(() => {
33+
d2.run(() => {
34+
// Emit an error on an EventEmitter bound to d2
35+
const EventEmitter = require('events');
36+
const ee = new EventEmitter();
37+
d2.add(ee);
38+
39+
// This should:
40+
// 1. Call d2's error handler with 'original error'
41+
// 2. d2's error handler throws 'from d2 error handler'
42+
// 3. That exception should go to d1's error handler, NOT d2 again
43+
ee.emit('error', new Error('original error'));
44+
});
45+
});

0 commit comments

Comments
 (0)