diff --git a/lib/run-script-pkg.js b/lib/run-script-pkg.js index cbb0a0b..a551828 100644 --- a/lib/run-script-pkg.js +++ b/lib/run-script-pkg.js @@ -94,7 +94,11 @@ const runScriptPkg = async options => { return p.catch(er => { const { signal } = er if (stdio === 'inherit' && signal) { + // by the time we reach here, the child has already exited. we send the + // signal back to ourselves again so that npm will exit with the same + // status as the child process.kill(process.pid, signal) + // just in case we don't die, reject after 500ms // this also keeps the node process open long enough to actually // get the signal, rather than terminating gracefully. diff --git a/lib/signal-manager.js b/lib/signal-manager.js index 7e10f85..efc00b4 100644 --- a/lib/signal-manager.js +++ b/lib/signal-manager.js @@ -1,17 +1,19 @@ const runningProcs = new Set() let handlersInstalled = false +// NOTE: these signals aren't actually forwarded anywhere. they're trapped and +// ignored until all child processes have exited. in our next breaking change +// we should rename this const forwardedSignals = [ 'SIGINT', 'SIGTERM', ] -const handleSignal = signal => { - for (const proc of runningProcs) { - proc.kill(signal) - } -} - +// no-op, this is so receiving the signal doesn't cause us to exit immediately +// instead, we exit after all children have exited when we re-send the signal +// to ourselves. see the catch handler at the bottom of run-script-pkg.js +// istanbul ignore next - this function does nothing +const handleSignal = () => {} const setupListeners = () => { for (const signal of forwardedSignals) { process.on(signal, handleSignal) diff --git a/test/signal-manager.js b/test/signal-manager.js index afc0ab3..7a0544f 100644 --- a/test/signal-manager.js +++ b/test/signal-manager.js @@ -44,24 +44,3 @@ test('adds only one handler for each signal, removes handlers when children have t.end() }) - -test('forwards signals to child process', t => { - const proc = new EventEmitter() - proc.kill = (signal) => { - t.equal(signal, signalManager.forwardedSignals[0], 'child receives correct signal') - proc.emit('exit', 0) - for (const forwarded of signalManager.forwardedSignals) { - t.equal( - process.listeners(forwarded).includes(signalManager.handleSignal), - false, 'listener has been removed') - } - t.end() - } - - signalManager.add(proc) - // passing the signal name here is necessary to fake the effects of actually - // receiving the signal per nodejs documentation signal handlers receive the - // name of the signal as their first parameter - // https://nodejs.org/api/process.html#process_signal_events - process.emit(signalManager.forwardedSignals[0], signalManager.forwardedSignals[0]) -})