Skip to content

Commit 2d9e5e5

Browse files
committed
Fix a couple on exit issues
- run handlers on a natural exit (ie no `process.exit` calls) - preserve exit code when `process.exit` is called during `process.on('exit')` Exit code was not previous preserved because we only intercepted explicit exits (ie calls to `process.exit`) but did not intercept natural exits. `process.exit` in node has two sets of semantics. 1. When called not while exiting, it begins exiting 2. When called while exiting (ie during a `process.on('exit')` handlers) it exits immediately, preventing other exit handlers from firing. Fix #6
1 parent d70f560 commit 2d9e5e5

File tree

4 files changed

+75
-0
lines changed

4 files changed

+75
-0
lines changed

index.js

+25
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ var exit;
44
var handlers = [];
55
var lastTime;
66

7+
process.on('beforeExit', function () {
8+
if (handlers.length === 0) { return; }
9+
10+
return module.exports._flush();
11+
});
12+
713
/*
814
* To allow cooperative async exit handlers, we unfortunately must hijack
915
* process.exit.
@@ -29,6 +35,25 @@ module.exports.captureExit = function() {
2935
exit = process.exit;
3036

3137
process.exit = function(code) {
38+
if (handlers.length === 0 && lastTime === undefined) {
39+
// synchronously exit.
40+
//
41+
// We do this brecause either
42+
//
43+
// 1. The process exited due to a call to `process.exit` but we have no
44+
// async work to do because no handlers had been attached. It
45+
// doesn't really matter whether we take this branch or not in this
46+
// case.
47+
//
48+
// 2. The process exited naturally. We did our async work during
49+
// `beforeExit` and are in this function because someone else has
50+
// called `process.exit` during an `on('exit')` hook. The only way
51+
// for us to preserve the exit code in this case is to exit
52+
// synchronously.
53+
//
54+
return exit.call(process, code);
55+
}
56+
3257
var own = lastTime = module.exports._flush(lastTime, code)
3358
.then(function() {
3459
// if another chain has started, let it exit

test-natural-exit-subprocess-error.js

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
'use strict';
2+
3+
var capture = require('./');
4+
capture.captureExit();
5+
6+
capture.onExit(function () {
7+
console.log('onExit');
8+
});
9+
10+
process.on('exit', function () {
11+
console.log('exit');
12+
process.exit(1);
13+
});
14+

test-natural-exit-subprocess.js

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
3+
var capture = require('./');
4+
capture.captureExit();
5+
6+
capture.onExit(function () {
7+
console.log('onExit');
8+
})
9+
10+
process.on('exit', function () {
11+
console.log('exit');
12+
});

test.js

+24
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ var RSVP = require('rsvp');
33

44
var originalExit = process.exit; // keep this around for good measure.
55
var exit = require('./');
6+
var childProcess = require('child_process');
67

78
describe('capture-exit', function() {
89
beforeEach(function() {
@@ -206,6 +207,29 @@ describe('capture-exit', function() {
206207
});
207208
});
208209

210+
describe('natural exit', function() {
211+
it('runs handlers on a natural exit', function() {
212+
var output = childProcess.execSync('node test-natural-exit-subprocess.js');
213+
expect(output+'').to.include('onExit');
214+
expect(output+'').to.include('exit');
215+
});
216+
217+
it('exits with error code if an on exit handler calls process.exit with code', function() {
218+
var succeeded = false;
219+
try {
220+
var output = childProcess.execSync('node test-natural-exit-subprocess-error.js');
221+
succeeded = true;
222+
} catch(e) {
223+
expect(e.output+'').to.include('onExit');
224+
expect(e.output+'').to.include('exit');
225+
}
226+
227+
if (succeeded) {
228+
throw new Error('Unexpected zero exit status for process.exit(1)');
229+
}
230+
});
231+
});
232+
209233
function delay(milliseconds) {
210234
return new RSVP.Promise(function(resolve) {
211235
setTimeout(resolve, milliseconds);

0 commit comments

Comments
 (0)