Skip to content

Commit d94a70e

Browse files
Julien Gillirvagg
Julien Gilli
authored andcommitted
test: fix test-domain-exit-dispose-again
test-domain-exit-dispose-again had been written for node v0.10.x, and was using the fact that callbacks scheduled with `process.nextTick` wouldn't run if the domain attached to it was disposed. This is not longer the case, and as a result the test would not catch any regression: it would always pass. This change rewrites that test to check that the current domain is cleared properly when processing the rest of the timers list if a timer's callback throws an error. This makes the test fail without the original fix, and pass with the original fix, as expected. PR: #3990 PR-URL: #3990 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
1 parent f3c50f5 commit d94a70e

File tree

1 file changed

+23
-40
lines changed

1 file changed

+23
-40
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,39 @@
11
'use strict';
2-
var common = require('../common');
3-
var assert = require('assert');
4-
var domain = require('domain');
5-
var disposalFailed = false;
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const domain = require('domain');
65

7-
// no matter what happens, we should increment a 10 times.
8-
var a = 0;
9-
log();
10-
function log() {
11-
console.log(a++, process.domain);
12-
if (a < 10) setTimeout(log, 20);
13-
}
14-
15-
var secondTimerRan = false;
16-
17-
// in 50ms we'll throw an error.
6+
// Use the same timeout value so that both timers' callbacks are called during
7+
// the same invocation of the underlying native timer's callback (listOnTimeout
8+
// in lib/timers.js).
189
setTimeout(err, 50);
19-
setTimeout(secondTimer, 50);
10+
setTimeout(common.mustCall(secondTimer), 50);
11+
2012
function err() {
21-
var d = domain.create();
22-
d.on('error', handle);
13+
const d = domain.create();
14+
d.on('error', handleDomainError);
2315
d.run(err2);
2416

2517
function err2() {
26-
// this timeout should never be called, since the domain gets
27-
// disposed when the error happens.
28-
setTimeout(function() {
29-
console.error('This should not happen.');
30-
disposalFailed = true;
31-
process.exit(1);
32-
});
33-
3418
// this function doesn't exist, and throws an error as a result.
3519
err3();
3620
}
3721

38-
function handle(e) {
39-
// this should clean up everything properly.
40-
d.dispose();
41-
console.error(e);
42-
console.error('in handler', process.domain, process.domain === d);
22+
function handleDomainError(e) {
23+
// In the domain's error handler, the current active domain should be the
24+
// domain within which the error was thrown.
25+
assert.equal(process.domain, d);
4326
}
4427
}
4528

4629
function secondTimer() {
47-
console.log('In second timer');
48-
secondTimerRan = true;
30+
// secondTimer was scheduled before any domain had been created, so its
31+
// callback should not have any active domain set when it runs.
32+
// Do not use assert here, as it throws errors and if a domain with an error
33+
// handler is active, then asserting wouldn't make the test fail.
34+
if (process.domain !== null) {
35+
console.log('process.domain should be null, but instead is:',
36+
process.domain);
37+
process.exit(1);
38+
}
4939
}
50-
51-
process.on('exit', function() {
52-
assert.equal(a, 10);
53-
assert.equal(disposalFailed, false);
54-
assert(secondTimerRan);
55-
console.log('ok');
56-
});

0 commit comments

Comments
 (0)