Exit with 130 after receiving SIGINT

pnpm should exit with non-zero code when aborting
abnormally due to SIGINT

Fixes: pnpm/pnpm#9626

Author: isaacl

index.js

@@ -318,8 +318,9 @@ function runCmd_ (cmd, pkg, env, wd, opts, stage, unsafe, uid, gid, cb_) {
       er.pkgname = pkg.name
     }
     process.removeListener('SIGTERM', procKill)
-    process.removeListener('SIGTERM', procInterrupt)
     process.removeListener('SIGINT', procKill)
+    process.removeListener('SIGINT', procInterrupt)
+    process.removeListener('exit', procKill)
     return cb(er)
   }
   let called = false
@@ -329,10 +330,8 @@ function runCmd_ (cmd, pkg, env, wd, opts, stage, unsafe, uid, gid, cb_) {
     proc.kill()
   }
   function procInterrupt () {
+    proc.on('close', () => process.exit(130))
     proc.kill('SIGINT')
-    proc.on('exit', () => {
-      process.exit()
-    })
     process.once('SIGINT', procKill)
   }
 }

test/fixtures/count-to-10/package.json

@@ -3,7 +3,7 @@
   "version": "1.0.0",
   "scripts": {
     "postinstall": "node postinstall",
-    "signal-abrt": "echo 'signal-exit script' && kill -s ABRT $$",
+    "signal-abrt": "echo 'signal-abrt script' && kill -s ABRT $$",
     "signal-int": "echo 'signal-int script' && kill -s INT $$"
   }
 }

test/fixtures/sleep/package.json

@@ -0,0 +1,18 @@
+{
+  "name": "sleep",
+  "version": "1.0.0",
+  "//": [
+    "Notes about the sleep-notify script below:",
+    "1)  The complexity of the script is necessary because we're signalling",
+    "    sh only, rather than what Posix shells do on Ctrl-C, which is to",
+    "    send SIGINT to the entire foreground process group.",
+    "2)  We trap SIGINT and exit with a code rather. This avoids the",
+    "    special handling of SIGINT exits which are treated as a success.",
+    "3)  The trap also seems to be necessary to make sh exit promptly.",
+    "4)  We don't want the parent process to send SIGINT to child before it",
+    "    has full started, so we send a signal to parent to coordinate the test."
+  ],
+  "scripts": {
+    "sleep-notify": "trap 'exit 130' SIGINT; kill -USR1 $PPID; for i in {1..100}; do sleep .01; done"
+  }
+}

test/index.js

@@ -265,3 +265,66 @@ test('no error on INT signal from child', async function (t) {
     'INT signal reported'
   )
 })
+
+test('node exits 130 when it receives SIGINT', async function (t) {
+  if (isWindows()) {
+    // On Windows there is no way to get the INT signal
+    return
+  }
+  const fixture = path.join(__dirname, 'fixtures', 'sleep')
+
+  const verbose = sinon.spy()
+  const silly = sinon.spy()
+  const info = sinon.spy()
+
+  const stubProcessExit = sinon.stub(process, 'exit').callsFake(noop)
+
+  const log = {
+    level: 'silent',
+    info: noop,
+    warn: noop,
+    silly,
+    verbose,
+    pause: noop,
+    resume: noop,
+    clearProgress: noop,
+    showProgress: noop
+  }
+
+  const dir = fixture
+  const pkg = require(path.join(fixture, 'package.json'))
+
+  // On Ctrl-C, posix shells send SIGINT to the foreground process group.
+  // But here we aren't necessarily a process group leader, nor is the
+  // script running as a separate process group. So the best we can do
+  // is send SIGINT and let sh handle it in a slightly different way,
+  // where it waits for the active command to finish...
+  // This is annoying, because it means the lifecycle script sleep has
+  // to finish, so it's set to a relatively low value.
+  process.once('SIGUSR1', () => process.kill(process.pid, 'SIGINT'))
+
+  // We're trapping SIGINT in the script, to avoid the special cased
+  // handling of an exit with an unhandled SIGINT (which is ignored).
+  await t.rejects(async () =>
+    lifecycle(pkg, 'sleep-notify', fixture, {
+      stdio: 'pipe',
+      log,
+      dir,
+      config: {}
+    })
+  )
+
+  stubProcessExit.restore()
+  stubProcessExit.calledOnceWith(130)
+  t.ok(
+    silly.calledWithMatch(
+      'lifecycle',
+      'undefined~sleep-notify:',
+      'Returned: code:',
+      130,
+      ' signal:',
+      null
+    ),
+    'lifecycle script exited with 130'
+  )
+})