Skip to content

Commit 6adae72

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 023423f commit 6adae72

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
@@ -626,10 +626,15 @@ EventEmitter.prototype.emit = function emit(...args) {
626626

627627
// Change the current active domain
628628
const newActiveDomain = newStack.length > 0 ? newStack[newStack.length - 1] : null;
629+
const prevCurrentDomain = currentDomain;
630+
const prevCurrentStack = currentStack;
631+
629632
if (store) {
630633
setDomainStore({ domain: newActiveDomain, stack: newStack });
631634
}
632635
exports.active = newActiveDomain;
636+
currentDomain = newActiveDomain;
637+
currentStack = newStack;
633638

634639
updateExceptionCapture();
635640

@@ -641,6 +646,8 @@ EventEmitter.prototype.emit = function emit(...args) {
641646
setDomainStore({ domain: origActiveDomain, stack: origDomainsStack });
642647
}
643648
exports.active = origActiveDomain;
649+
currentDomain = prevCurrentDomain;
650+
currentStack = prevCurrentStack;
644651
updateExceptionCapture();
645652

646653
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)