fix: handle signals more correctly

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.

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)

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])
-})