From f019b8a44aa8059634811d87c2d523215212a33a Mon Sep 17 00:00:00 2001 From: Bret Comnes Date: Fri, 2 Oct 2026 10:28:13 -0700 Subject: [PATCH 1/6] fix: await watch child shutdown on signals --- lib/watch/index.js | 37 +++++++++++++++++++-- test/data/delayed-close-plugin.js | 13 ++++++++ test/watch-shutdown.test.js | 55 +++++++++++++++++++++++++++++++ test/watch-unit.test.js | 32 +++++++++++++++++- 4 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 test/data/delayed-close-plugin.js create mode 100644 test/watch-shutdown.test.js diff --git a/lib/watch/index.js b/lib/watch/index.js index db58786f..013a305a 100644 --- a/lib/watch/index.js +++ b/lib/watch/index.js @@ -15,6 +15,28 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { const emitter = new EventEmitter() let allStop = false let childs = [] + const liveChildren = new Set() + let stopping + const shutdown = (signal) => { + if (stopping) return stopping + allStop = true + stopping = Promise.all([ + watcher.close(), + ...Array.from(liveChildren, child => new Promise(resolve => { + child.once('close', resolve) + // IPC avoids delivering a second signal when Ctrl-C also reaches the child. + if (child.connected) child.send(GRACEFUL_SHUT, () => {}) + })) + ]).then(() => { + process.removeListener('SIGINT', onSigint) + process.removeListener('SIGTERM', onSigterm) + process.removeListener('uncaughtException', onUncaughtException) + process.exitCode = signal === 'SIGINT' ? 130 : 143 + }) + return stopping + } + const onSigint = () => { shutdown('SIGINT') } + const onSigterm = () => { shutdown('SIGTERM') } const stop = (watcher = null, err = null) => { childs.forEach(function (child) { child.kill() @@ -26,14 +48,21 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { } if (watcher) { allStop = true + process.removeListener('SIGINT', onSigint) + process.removeListener('SIGTERM', onSigterm) + process.removeListener('uncaughtException', onUncaughtException) return watcher.close() } } - process.on('uncaughtException', () => { + const onUncaughtException = () => { + if (allStop) return stop() childs.push(run('restart')) - }) + } + process.on('uncaughtException', onUncaughtException) + process.on('SIGINT', onSigint) + process.on('SIGTERM', onSigterm) let readyEmitted = false @@ -47,6 +76,9 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { encoding: 'utf8' }) + liveChildren.add(_child) + _child.once('close', () => liveChildren.delete(_child)) + _child.on('exit', function (code, signal) { if (childs.length === 0 && !allStop) { childs.push(run('restart')) @@ -83,6 +115,7 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { const watcher = chokidar.watch(watchDir, { ignored: ignoredPattern }) watcher.on('ready', function () { watcher.on('all', function (event, filepath) { + if (allStop) return if (verboseWatch) { logWatchVerbose(event, filepath) } diff --git a/test/data/delayed-close-plugin.js b/test/data/delayed-close-plugin.js new file mode 100644 index 00000000..badc6e98 --- /dev/null +++ b/test/data/delayed-close-plugin.js @@ -0,0 +1,13 @@ +'use strict' + +const { setTimeout } = require('node:timers/promises') + +module.exports = async function (fastify, opts) { + fastify.addHook('onListen', async () => { + setImmediate(() => process.stdout.write('application-ready\n')) + }) + fastify.addHook('onClose', async () => { + await setTimeout(200) + process.stdout.write('application-closed\n') + }) +} diff --git a/test/watch-shutdown.test.js b/test/watch-shutdown.test.js new file mode 100644 index 00000000..9e104a8a --- /dev/null +++ b/test/watch-shutdown.test.js @@ -0,0 +1,55 @@ +'use strict' + +const { test } = require('node:test') +const { spawn } = require('node:child_process') +const { once } = require('node:events') +const path = require('node:path') +const net = require('node:net') + +const cases = ['SIGINT', 'SIGTERM'].flatMap(signal => + (process.platform === 'win32' ? [false] : [false, true]).map(group => ({ signal, group })) +) + +for (const { signal, group } of cases) { + test(`watch supervisor waits for application cleanup on ${signal} (${group ? 'process group' : 'parent only'})`, { timeout: 15000 }, async t => { + const reservation = net.createServer().listen(0, '127.0.0.1') + await once(reservation, 'listening') + const { port } = reservation.address() + await new Promise(resolve => reservation.close(resolve)) + const child = spawn(process.execPath, [ + path.join(__dirname, '..', 'cli.js'), 'start', '--watch', + '--port', String(port), '--address', '127.0.0.1', + '--follow-watch', path.join(__dirname, 'data', 'delayed-close-plugin.js'), + path.join(__dirname, 'data', 'delayed-close-plugin.js') + ], { stdio: ['ignore', 'pipe', 'pipe'], detached: group }) + t.after(() => { + if (group) { + try { process.kill(-child.pid, 'SIGKILL') } catch (err) { + if (err.code !== 'ESRCH') throw err + } + } else if (child.exitCode === null && child.signalCode === null) { + child.kill('SIGKILL') + } + }) + let output = '' + let errors = '' + child.stderr.on('data', chunk => { errors += chunk }) + const closed = once(child, 'close') + await new Promise((resolve, reject) => { + child.once('error', reject) + child.once('exit', () => reject(new Error(`Exited before ready: ${errors}`))) + child.stdout.on('data', chunk => { + output += chunk + if (output.includes('application-ready')) resolve() + }) + }) + let outputAtExit + child.once('exit', () => { outputAtExit = output }) + if (group) process.kill(-child.pid, signal) + else child.kill(signal) + const [code, exitSignal] = await closed + t.assert.match(outputAtExit, /application-closed/) + t.assert.strictEqual(code, signal === 'SIGINT' ? 130 : 143, errors) + t.assert.strictEqual(exitSignal, null) + }) +} diff --git a/test/watch-unit.test.js b/test/watch-unit.test.js index 31d65545..97f9b660 100644 --- a/test/watch-unit.test.js +++ b/test/watch-unit.test.js @@ -13,6 +13,7 @@ function setup (t) { const childProcessMock = { fork () { const child = new EventEmitter() + child.connected = true child.kill = t.mock.fn() child.send = t.mock.fn() forks.push(child) @@ -25,8 +26,10 @@ function setup (t) { const chokidarMock = { watch: () => watcher } const uncaught = [] + const signals = {} t.mock.method(process, 'on', (event, listener) => { if (event === 'uncaughtException') uncaught.push(listener) + if (event === 'SIGINT' || event === 'SIGTERM') signals[event] = listener }) t.mock.method(console, 'log', () => {}) @@ -35,7 +38,7 @@ function setup (t) { 'node:child_process': childProcessMock }) - return { watch, forks, watcher, uncaught } + return { watch, forks, watcher, uncaught, signals } } test('should restart the child when a watched file changes', t => { @@ -120,6 +123,33 @@ test('should restart the child on an uncaught exception', t => { t.assert.strictEqual(forks.length, 2) }) +for (const signal of ['SIGINT', 'SIGTERM']) { + test(`should await child close before finishing ${signal} shutdown`, async t => { + const { watch, forks, watcher, signals } = setup(t) + const originalExitCode = process.exitCode + t.after(() => { process.exitCode = originalExitCode }) + watch(['app.js'], 'node_modules', false) + watcher.emit('ready') + + // A restarting child is no longer in the restart queue but still needs draining. + watcher.emit('all', 'change', 'app.js') + signals[signal]() + signals[signal]() + watcher.emit('all', 'change', 'app.js') + + t.assert.strictEqual(watcher.close.mock.callCount(), 1) + t.assert.strictEqual(forks[0].kill.mock.callCount(), 0) + t.assert.strictEqual(forks[0].send.mock.calls[1].arguments[0], GRACEFUL_SHUT) + forks[0].emit('exit', 0, null) + await Promise.resolve() + t.assert.strictEqual(process.exitCode, originalExitCode) + t.assert.strictEqual(forks.length, 1) + forks[0].emit('close', 0, null) + await new Promise(resolve => setImmediate(resolve)) + t.assert.strictEqual(process.exitCode, signal === 'SIGINT' ? 130 : 143) + }) +} + test('logWatchVerbose should print the relative path', t => { t.mock.method(console, 'log', () => {}) logWatchVerbose('add', `${process.cwd()}/lib/a.js`) From 52b552fa6622a5e933a7ad23ab2eaf71168e6a0b Mon Sep 17 00:00:00 2001 From: Bret Comnes Date: Fri, 2 Oct 2026 11:10:31 -0700 Subject: [PATCH 2/6] test: cover exceptions during watch shutdown --- test/watch-unit.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/watch-unit.test.js b/test/watch-unit.test.js index 97f9b660..8274104a 100644 --- a/test/watch-unit.test.js +++ b/test/watch-unit.test.js @@ -125,7 +125,7 @@ test('should restart the child on an uncaught exception', t => { for (const signal of ['SIGINT', 'SIGTERM']) { test(`should await child close before finishing ${signal} shutdown`, async t => { - const { watch, forks, watcher, signals } = setup(t) + const { watch, forks, watcher, signals, uncaught } = setup(t) const originalExitCode = process.exitCode t.after(() => { process.exitCode = originalExitCode }) watch(['app.js'], 'node_modules', false) @@ -135,9 +135,11 @@ for (const signal of ['SIGINT', 'SIGTERM']) { watcher.emit('all', 'change', 'app.js') signals[signal]() signals[signal]() + uncaught[0](new Error('exception during shutdown')) watcher.emit('all', 'change', 'app.js') t.assert.strictEqual(watcher.close.mock.callCount(), 1) + t.assert.strictEqual(forks.length, 1, 'an exception during shutdown must not restart the child') t.assert.strictEqual(forks[0].kill.mock.callCount(), 0) t.assert.strictEqual(forks[0].send.mock.calls[1].arguments[0], GRACEFUL_SHUT) forks[0].emit('exit', 0, null) From cec5c23e0658501ace5d9cd36a219a7c5b3495cb Mon Sep 17 00:00:00 2001 From: Bret Comnes Date: Fri, 2 Oct 2026 13:45:00 -0700 Subject: [PATCH 3/6] fix: honor close grace delay in watch mode --- README.md | 2 +- lib/watch/constants.js | 3 +- lib/watch/fork.js | 10 +-- lib/watch/index.js | 62 +++++++++++++------ test/watch-fork.test.js | 13 ++++ test/watch-shutdown.test.js | 119 +++++++++++++++++++++++------------- test/watch-unit.test.js | 65 ++++++++++++++++++-- 7 files changed, 200 insertions(+), 74 deletions(-) diff --git a/README.md b/README.md index a18971cd..1b6e81b4 100644 --- a/README.md +++ b/README.md @@ -171,7 +171,7 @@ You can pass the following options via CLI arguments. You can also use `--config | Set the prefix | `-x` | `--prefix` | `FASTIFY_PREFIX` | | Set the plugin timeout | `-T` | `--plugin-timeout` | `FASTIFY_PLUGIN_TIMEOUT` | | Defines the maximum payload, in bytes,
that the server is allowed to accept | | `--body-limit` | `FASTIFY_BODY_LIMIT` | -| Set the maximum ms delay before forcefully closing pending requests after receiving SIGTERM or SIGINT signals; and uncaughtException or unhandledRejection errors (default: 500) | `-g` | `--close-grace-delay` | `FASTIFY_CLOSE_GRACE_DELAY` | +| Set the maximum ms delay for graceful shutdown after SIGTERM, SIGINT, uncaughtException, or unhandledRejection, and for watch-mode restarts (default: 500) | `-g` | `--close-grace-delay` | `FASTIFY_CLOSE_GRACE_DELAY` | | Set the boolean value for `trustProxy` (1st precedence) | | `--trust-proxy-enabled` | `FASTIFY_TRUST_PROXY_ENABLED` | | Set the IP/CIDR value for `trustProxy` (2nd precedence) | | `--trust-proxy-ips` | `FASTIFY_TRUST_PROXY_IPS` | | Set the nth hop value for `trustProxy` (3rd precedence) | | `--trust-proxy-hop` | `FASTIFY_TRUST_PROXY_HOP` | diff --git a/lib/watch/constants.js b/lib/watch/constants.js index 742c31bc..07879fb8 100644 --- a/lib/watch/constants.js +++ b/lib/watch/constants.js @@ -2,6 +2,5 @@ module.exports = { GRACEFUL_SHUT: 'GRACEFUL SHUTDOWN', - READY: 'ready', - TIMEOUT: 5000 + READY: 'ready' } diff --git a/lib/watch/fork.js b/lib/watch/fork.js index d2827dd2..454d477e 100644 --- a/lib/watch/fork.js +++ b/lib/watch/fork.js @@ -2,11 +2,13 @@ const chalk = require('chalk').default const { runFastify } = require('../../start') +const parseArgs = require('../../args') +const args = process.argv.slice(2) +const { closeGraceDelay } = parseArgs(args) const { GRACEFUL_SHUT, - READY, - TIMEOUT + READY } = require('./constants.js') let fastify @@ -19,7 +21,7 @@ function exit () { process.on('message', function (event) { if (event === GRACEFUL_SHUT) { const message = chalk.red('[fastify-cli] process forced end') - setTimeout(exit.bind({ message }), TIMEOUT).unref() + setTimeout(exit.bind({ message }), closeGraceDelay).unref() if (fastify) { fastify.close(() => { process.exit(0) @@ -37,7 +39,7 @@ process.on('uncaughtException', (err) => { }) const main = async () => { - fastify = await runFastify(process.argv.splice(2)) + fastify = await runFastify(args) const type = process.env.childEvent process.send({ type, err: null }) diff --git a/lib/watch/index.js b/lib/watch/index.js index 013a305a..4b18d519 100644 --- a/lib/watch/index.js +++ b/lib/watch/index.js @@ -5,6 +5,7 @@ const cp = require('node:child_process') const chalk = require('chalk').default const { arrayToRegExp, logWatchVerbose } = require('./utils') const { GRACEFUL_SHUT } = require('./constants.js') +const parseArgs = require('../../args') const EventEmitter = require('node:events') const chokidar = require('chokidar') @@ -12,33 +13,62 @@ const { loadEnvQuitely } = require('../../env-loader.js') const forkPath = path.join(__dirname, './fork.js') const watch = function (args, ignoreWatch, verboseWatch, followWatch) { + const { closeGraceDelay } = parseArgs(args) const emitter = new EventEmitter() let allStop = false let childs = [] + // Restart requests remove children from childs before their cleanup completes. const liveChildren = new Set() let stopping - const shutdown = (signal) => { + + function removeListeners () { + process.removeListener('SIGINT', onSigint) + process.removeListener('SIGTERM', onSigterm) + process.removeListener('uncaughtException', onUncaughtException) + } + + function closeChild (child) { + return new Promise(resolve => { + // Give the child's own deadline time to fire, but also bound failed IPC + // and children whose event loop cannot process the shutdown request. + const timer = setTimeout(() => child.kill('SIGKILL'), Number(closeGraceDelay) + 1000) + child.once('close', () => { + clearTimeout(timer) + resolve() + }) + // IPC avoids a second signal when terminal Ctrl-C also reaches the child. + // If delivery fails, the deadline above still terminates the child. + try { + if (child.connected) child.send(GRACEFUL_SHUT, () => {}) + } catch { + // The IPC channel can close between checking connected and sending. + } + }) + } + + function shutdown (signal) { if (stopping) return stopping allStop = true - stopping = Promise.all([ - watcher.close(), - ...Array.from(liveChildren, child => new Promise(resolve => { - child.once('close', resolve) - // IPC avoids delivering a second signal when Ctrl-C also reaches the child. - if (child.connected) child.send(GRACEFUL_SHUT, () => {}) - })) - ]).then(() => { - process.removeListener('SIGINT', onSigint) - process.removeListener('SIGTERM', onSigterm) - process.removeListener('uncaughtException', onUncaughtException) - process.exitCode = signal === 'SIGINT' ? 130 : 143 + const childrenClosed = Array.from(liveChildren, closeChild) + stopping = Promise.allSettled([ + Promise.resolve().then(() => watcher.close()), + ...childrenClosed + ]).then(results => { + removeListeners() + const failure = results.find(result => result.status === 'rejected') + if (failure) { + console.error(failure.reason) + process.exitCode = 1 + } else { + process.exitCode = signal === 'SIGINT' ? 130 : 143 + } }) return stopping } const onSigint = () => { shutdown('SIGINT') } const onSigterm = () => { shutdown('SIGTERM') } const stop = (watcher = null, err = null) => { - childs.forEach(function (child) { + liveChildren.forEach(function (child) { child.kill() }) @@ -48,9 +78,7 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { } if (watcher) { allStop = true - process.removeListener('SIGINT', onSigint) - process.removeListener('SIGTERM', onSigterm) - process.removeListener('uncaughtException', onUncaughtException) + removeListeners() return watcher.close() } } diff --git a/test/watch-fork.test.js b/test/watch-fork.test.js index ffdf1cf7..e3a07a9d 100644 --- a/test/watch-fork.test.js +++ b/test/watch-fork.test.js @@ -77,6 +77,19 @@ test('should force the exit when the server does not close in time', testOptions t.assert.match(output(), /process forced end/) }) +test('should use the configured IPC shutdown deadline', testOptions, async t => { + const { child, message, output } = forkChild(t, [ + '-p', '0', '--close-grace-delay', '50', + './test/data/hanging-close-plugin.js' + ]) + await message('ready') + child.send(GRACEFUL_SHUT) + const [code] = await once(child, 'close') + + t.assert.strictEqual(code, 1) + t.assert.match(output(), /process forced end/) +}) + test('should exit immediately on graceful shutdown when the server is not up yet', testOptions, async (t) => { const { child } = forkChild(t, ['-p', '0', './test/data/slow-plugin.js']) child.send(GRACEFUL_SHUT) diff --git a/test/watch-shutdown.test.js b/test/watch-shutdown.test.js index 9e104a8a..0043784e 100644 --- a/test/watch-shutdown.test.js +++ b/test/watch-shutdown.test.js @@ -6,50 +6,81 @@ const { once } = require('node:events') const path = require('node:path') const net = require('node:net') -const cases = ['SIGINT', 'SIGTERM'].flatMap(signal => - (process.platform === 'win32' ? [false] : [false, true]).map(group => ({ signal, group })) -) - -for (const { signal, group } of cases) { - test(`watch supervisor waits for application cleanup on ${signal} (${group ? 'process group' : 'parent only'})`, { timeout: 15000 }, async t => { - const reservation = net.createServer().listen(0, '127.0.0.1') - await once(reservation, 'listening') - const { port } = reservation.address() - await new Promise(resolve => reservation.close(resolve)) - const child = spawn(process.execPath, [ - path.join(__dirname, '..', 'cli.js'), 'start', '--watch', - '--port', String(port), '--address', '127.0.0.1', - '--follow-watch', path.join(__dirname, 'data', 'delayed-close-plugin.js'), - path.join(__dirname, 'data', 'delayed-close-plugin.js') - ], { stdio: ['ignore', 'pipe', 'pipe'], detached: group }) - t.after(() => { - if (group) { - try { process.kill(-child.pid, 'SIGKILL') } catch (err) { - if (err.code !== 'ESRCH') throw err - } - } else if (child.exitCode === null && child.signalCode === null) { - child.kill('SIGKILL') - } - }) - let output = '' - let errors = '' - child.stderr.on('data', chunk => { errors += chunk }) - const closed = once(child, 'close') - await new Promise((resolve, reject) => { - child.once('error', reject) - child.once('exit', () => reject(new Error(`Exited before ready: ${errors}`))) - child.stdout.on('data', chunk => { - output += chunk - if (output.includes('application-ready')) resolve() - }) +const cliPath = path.join(__dirname, '..', 'cli.js') +const pluginPath = path.join(__dirname, 'data', 'delayed-close-plugin.js') +// Windows kill() terminates processes rather than delivering catchable POSIX signals. +const testOptions = { timeout: 15000, skip: process.platform === 'win32' } +const signals = [ + { signal: 'SIGINT', expectedCode: 130 }, + { signal: 'SIGTERM', expectedCode: 143 } +] +const targets = ['parent only'] +// Negative PIDs address process groups on POSIX, but are not supported on Windows. +if (process.platform !== 'win32') targets.push('process group') + +async function getAvailablePort (t) { + const server = net.createServer().listen(0, '127.0.0.1') + t.after(() => server.close()) + await once(server, 'listening') + const { port } = server.address() + await new Promise(resolve => server.close(resolve)) + return port +} + +async function startWatcher (t) { + const port = await getAvailablePort(t) + const child = spawn(process.execPath, [ + cliPath, 'start', '--watch', + '--port', String(port), '--address', '127.0.0.1', + '--follow-watch', pluginPath, + pluginPath + ], { stdio: ['ignore', 'pipe', 'pipe'], detached: true }) + + t.after(() => { + // Clean up the whole tree even when the test signals only the supervisor. + try { + process.kill(-child.pid, 'SIGKILL') + } catch (err) { + if (err.code !== 'ESRCH') throw err + } + }) + + let stdout = '' + let stderr = '' + let outputAtExit = '' + child.stderr.on('data', chunk => { stderr += chunk }) + child.once('exit', () => { outputAtExit = stdout }) + const closed = once(child, 'close') + const ready = new Promise((resolve, reject) => { + child.once('error', reject) + child.once('exit', () => reject(new Error(`Watcher exited before ready: ${stderr}`))) + child.stdout.on('data', chunk => { + stdout += chunk + if (stdout.includes('application-ready')) resolve() }) - let outputAtExit - child.once('exit', () => { outputAtExit = output }) - if (group) process.kill(-child.pid, signal) - else child.kill(signal) - const [code, exitSignal] = await closed - t.assert.match(outputAtExit, /application-closed/) - t.assert.strictEqual(code, signal === 'SIGINT' ? 130 : 143, errors) - t.assert.strictEqual(exitSignal, null) }) + + return { child, ready, closed, outputAtExit: () => outputAtExit, errors: () => stderr } +} + +for (const { signal, expectedCode } of signals) { + for (const target of targets) { + test(`should await application cleanup on ${signal} (${target})`, testOptions, async t => { + const processGroup = target === 'process group' + const watcher = await startWatcher(t) + await watcher.ready + + if (processGroup) { + process.kill(-watcher.child.pid, signal) + } else { + watcher.child.kill(signal) + } + const [code, exitSignal] = await watcher.closed + + // Checking final stdout alone would also accept cleanup after the parent exited. + t.assert.match(watcher.outputAtExit(), /application-closed/) + t.assert.strictEqual(code, expectedCode, watcher.errors()) + t.assert.strictEqual(exitSignal, null) + }) + } } diff --git a/test/watch-unit.test.js b/test/watch-unit.test.js index 8274104a..0fb6fe2f 100644 --- a/test/watch-unit.test.js +++ b/test/watch-unit.test.js @@ -2,6 +2,7 @@ const { test } = require('node:test') const EventEmitter = require('node:events') +const { setImmediate: nextTurn } = require('node:timers/promises') const proxyquire = require('proxyquire') const { logWatchVerbose } = require('../lib/watch/utils') const { GRACEFUL_SHUT } = require('../lib/watch/constants') @@ -130,28 +131,80 @@ for (const signal of ['SIGINT', 'SIGTERM']) { t.after(() => { process.exitCode = originalExitCode }) watch(['app.js'], 'node_modules', false) watcher.emit('ready') + const child = forks[0] // A restarting child is no longer in the restart queue but still needs draining. watcher.emit('all', 'change', 'app.js') signals[signal]() + + // Repeated signals, exceptions, and file changes must not start another shutdown or child. signals[signal]() uncaught[0](new Error('exception during shutdown')) watcher.emit('all', 'change', 'app.js') + await nextTurn() t.assert.strictEqual(watcher.close.mock.callCount(), 1) t.assert.strictEqual(forks.length, 1, 'an exception during shutdown must not restart the child') - t.assert.strictEqual(forks[0].kill.mock.callCount(), 0) - t.assert.strictEqual(forks[0].send.mock.calls[1].arguments[0], GRACEFUL_SHUT) - forks[0].emit('exit', 0, null) - await Promise.resolve() + t.assert.strictEqual(child.kill.mock.callCount(), 0) + t.assert.strictEqual(child.send.mock.callCount(), 2, 'one restart request and one shutdown request') + t.assert.strictEqual(child.send.mock.calls[1].arguments[0], GRACEFUL_SHUT) + + // Exiting is not enough: the supervisor must also wait for the child's streams to close. + child.emit('exit', 0, null) + await nextTurn() t.assert.strictEqual(process.exitCode, originalExitCode) t.assert.strictEqual(forks.length, 1) - forks[0].emit('close', 0, null) - await new Promise(resolve => setImmediate(resolve)) + + child.emit('close', 0, null) + await nextTurn() t.assert.strictEqual(process.exitCode, signal === 'SIGINT' ? 130 : 143) }) } +for (const { name, args, delay } of [ + { name: 'default', args: [], delay: 500 }, + { name: 'CLI option', args: ['--close-grace-delay', '2500'], delay: 2500 }, + { name: 'config file', args: ['--config', './test/data/custom-config.js'], delay: 1000 } +]) { + test(`should use the ${name} shutdown deadline for a disconnected child`, async t => { + const { watch, forks, signals } = setup(t) + const originalExitCode = process.exitCode + t.after(() => { process.exitCode = originalExitCode }) + t.mock.timers.enable({ apis: ['setTimeout'] }) + watch([...args, 'app.js'], 'node_modules', false) + const child = forks[0] + child.connected = false + + signals.SIGINT() + t.mock.timers.tick(delay + 999) + t.assert.strictEqual(child.kill.mock.callCount(), 0) + t.mock.timers.tick(1) + t.assert.deepStrictEqual(child.kill.mock.calls[0].arguments, ['SIGKILL']) + child.emit('close', null, 'SIGKILL') + await nextTurn() + t.assert.strictEqual(process.exitCode, 130) + }) +} + +test('should still drain children when watcher cleanup fails and IPC throws', async t => { + const { watch, forks, watcher, signals } = setup(t) + const originalExitCode = process.exitCode + t.after(() => { process.exitCode = originalExitCode }) + t.mock.method(console, 'error', () => {}) + const error = new Error('watcher close failed') + watcher.close = () => { throw error } + watch(['app.js'], 'node_modules', false) + forks[0].send = () => { throw new Error('IPC channel closed') } + + signals.SIGTERM() + await nextTurn() + t.assert.strictEqual(process.exitCode, originalExitCode) + forks[0].emit('close', 0, null) + await nextTurn() + t.assert.strictEqual(process.exitCode, 1) + t.assert.strictEqual(console.error.mock.calls[0].arguments[0], error) +}) + test('logWatchVerbose should print the relative path', t => { t.mock.method(console, 'log', () => {}) logWatchVerbose('add', `${process.cwd()}/lib/a.js`) From 1a17ece8f3896f2a6bf9f4696630e14c636421a6 Mon Sep 17 00:00:00 2001 From: Bret Comnes Date: Fri, 2 Oct 2026 13:51:26 -0700 Subject: [PATCH 4/6] refactor: await watch shutdown cleanup --- lib/watch/index.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/watch/index.js b/lib/watch/index.js index 4b18d519..584d704f 100644 --- a/lib/watch/index.js +++ b/lib/watch/index.js @@ -27,6 +27,10 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { process.removeListener('uncaughtException', onUncaughtException) } + async function closeWatcher () { + await watcher.close() + } + function closeChild (child) { return new Promise(resolve => { // Give the child's own deadline time to fire, but also bound failed IPC @@ -50,10 +54,11 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { if (stopping) return stopping allStop = true const childrenClosed = Array.from(liveChildren, closeChild) - stopping = Promise.allSettled([ - Promise.resolve().then(() => watcher.close()), - ...childrenClosed - ]).then(results => { + stopping = (async () => { + const results = await Promise.allSettled([ + closeWatcher(), + ...childrenClosed + ]) removeListeners() const failure = results.find(result => result.status === 'rejected') if (failure) { @@ -62,7 +67,7 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { } else { process.exitCode = signal === 'SIGINT' ? 130 : 143 } - }) + })() return stopping } const onSigint = () => { shutdown('SIGINT') } From e6b2ec417c6022fa1049ab7574853a2ca14c73b8 Mon Sep 17 00:00:00 2001 From: Bret Comnes Date: Fri, 2 Oct 2026 14:01:42 -0700 Subject: [PATCH 5/6] refactor: extract watch shutdown completion --- lib/watch/index.js | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/watch/index.js b/lib/watch/index.js index 584d704f..3be80e8e 100644 --- a/lib/watch/index.js +++ b/lib/watch/index.js @@ -50,24 +50,26 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { }) } + async function finishShutdown (signal, childrenClosed) { + const results = await Promise.allSettled([ + closeWatcher(), + ...childrenClosed + ]) + removeListeners() + const failure = results.find(result => result.status === 'rejected') + if (failure) { + console.error(failure.reason) + process.exitCode = 1 + } else { + process.exitCode = signal === 'SIGINT' ? 130 : 143 + } + } + function shutdown (signal) { if (stopping) return stopping allStop = true const childrenClosed = Array.from(liveChildren, closeChild) - stopping = (async () => { - const results = await Promise.allSettled([ - closeWatcher(), - ...childrenClosed - ]) - removeListeners() - const failure = results.find(result => result.status === 'rejected') - if (failure) { - console.error(failure.reason) - process.exitCode = 1 - } else { - process.exitCode = signal === 'SIGINT' ? 130 : 143 - } - })() + stopping = finishShutdown(signal, childrenClosed) return stopping } const onSigint = () => { shutdown('SIGINT') } From a15e9143c38cc6b371f38ce3edb58170b9b23e61 Mon Sep 17 00:00:00 2001 From: Bret Comnes Date: Fri, 2 Oct 2026 14:04:20 -0700 Subject: [PATCH 6/6] style: order watch shutdown helpers top down --- lib/watch/index.js | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/watch/index.js b/lib/watch/index.js index 3be80e8e..7650c90b 100644 --- a/lib/watch/index.js +++ b/lib/watch/index.js @@ -27,6 +27,29 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { process.removeListener('uncaughtException', onUncaughtException) } + function shutdown (signal) { + if (stopping) return stopping + allStop = true + const childrenClosed = Array.from(liveChildren, closeChild) + stopping = finishShutdown(signal, childrenClosed) + return stopping + } + + async function finishShutdown (signal, childrenClosed) { + const results = await Promise.allSettled([ + closeWatcher(), + ...childrenClosed + ]) + removeListeners() + const failure = results.find(result => result.status === 'rejected') + if (failure) { + console.error(failure.reason) + process.exitCode = 1 + } else { + process.exitCode = signal === 'SIGINT' ? 130 : 143 + } + } + async function closeWatcher () { await watcher.close() } @@ -49,29 +72,6 @@ const watch = function (args, ignoreWatch, verboseWatch, followWatch) { } }) } - - async function finishShutdown (signal, childrenClosed) { - const results = await Promise.allSettled([ - closeWatcher(), - ...childrenClosed - ]) - removeListeners() - const failure = results.find(result => result.status === 'rejected') - if (failure) { - console.error(failure.reason) - process.exitCode = 1 - } else { - process.exitCode = signal === 'SIGINT' ? 130 : 143 - } - } - - function shutdown (signal) { - if (stopping) return stopping - allStop = true - const childrenClosed = Array.from(liveChildren, closeChild) - stopping = finishShutdown(signal, childrenClosed) - return stopping - } const onSigint = () => { shutdown('SIGINT') } const onSigterm = () => { shutdown('SIGTERM') } const stop = (watcher = null, err = null) => {