From 0b9e9fd99a34d1c70d8ff2ea121927b11160de61 Mon Sep 17 00:00:00 2001 From: Aaron Jensen Date: Fri, 2 Nov 2018 07:55:48 -0700 Subject: [PATCH] fix: restart on change for non-default signals (#1409) When nodemon kills the child app in preparation for a restart, the app may exit with a non-zero status code, especially when using custom signals like SIGINT. Previously, this would be detected as a clean exit rather than a restart, at least on macOS. There was an existing flag, `killedAfterChange` which causes the desired behavior, but it was only set on Windows. It seems that setting it always leaked between tests and caused problems, so we only set it if a restart is expected. Fixes #1409 --- lib/monitor/run.js | 6 +++--- test/fixtures/app.js | 4 +++- test/lib/require.test.js | 27 ++++++++++++++++++++++++++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/lib/monitor/run.js b/lib/monitor/run.js index cd143477..6254fdeb 100644 --- a/lib/monitor/run.js +++ b/lib/monitor/run.js @@ -243,9 +243,9 @@ function run(options) { process.stdin.unpipe(child.stdin); } - if (utils.isWindows) { - // For the on('exit', ...) handler above the following looks like a - // crash, so we set the killedAfterChange flag + // For the on('exit', ...) handler above the following looks like a + // crash, so we set the killedAfterChange flag if a restart is planned + if (!noRestart) { killedAfterChange = true; } diff --git a/test/fixtures/app.js b/test/fixtures/app.js index 48ffa31c..380dde3a 100644 --- a/test/fixtures/app.js +++ b/test/fixtures/app.js @@ -1,3 +1,5 @@ // process.title = 'node (fixtures/app)'; // console.log('starting up @ ' + (process.env.PORT || 8000)); -require('http').createServer(function (req, res) { console.log('Request in'); res.end('ok'); }).listen(process.env.PORT || 8000); \ No newline at end of file +require('http').createServer(function (req, res) { console.log('Request in'); res.end('ok'); }).listen(process.env.PORT || 8000); + +process.on('SIGINT', function() { process.exit(130) }) diff --git a/test/lib/require.test.js b/test/lib/require.test.js index 1d060dbc..a3cf8851 100644 --- a/test/lib/require.test.js +++ b/test/lib/require.test.js @@ -54,9 +54,34 @@ describe('require-able', function () { setTimeout(function () { touch.sync(appjs); }, 1000); + }).on('start', function() { + if (restarted) { + setTimeout(function() { nodemon.emit('quit') }); + } + }).on('restart', function () { + restarted = true; + }).on('quit', function () { + assert(restarted, 'nodemon restarted and quit properly'); + nodemon.reset(done); + }).on('log', function (event) { + // console.log(event.message); + }); + }); + + it('should restart on file change with custom signal', function (done) { + var restarted = false; + + utils.port++; + nodemon({ script: appjs, verbose: true, env: { PORT: utils.port }, signal: 'SIGINT' }).on('start', function () { + setTimeout(function () { + touch.sync(appjs); + }, 1000); + }).on('start', function() { + if (restarted) { + setTimeout(function() { nodemon.emit('quit') }); + } }).on('restart', function () { restarted = true; - nodemon.emit('quit'); }).on('quit', function () { assert(restarted, 'nodemon restarted and quit properly'); nodemon.reset(done);