fix: revert handle signal exits gracefully (#8524)

owlstronaut · web-flow · commit 75ce64a5b21b · 2025-08-22T12:59:10.000-07:00
This reverts commit c8d8397.
diff --git a/lib/cli/exit-handler.js b/lib/cli/exit-handler.js
@@ -43,16 +43,6 @@ class ExitHandler {
   registerUncaughtHandlers () {
     this.#process.on('uncaughtException', this.#handleExit)
     this.#process.on('unhandledRejection', this.#handleExit)
-
-    // Handle signals that might bypass normal exit flow
-    // These signals can cause the process to exit without calling the exit handler
-    const signalsToHandle = ['SIGTERM', 'SIGINT', 'SIGHUP']
-    for (const signal of signalsToHandle) {
-      this.#process.on(signal, () => {
-        // Call the exit handler to ensure proper cleanup
-        this.#handleExit(new Error(`Process received ${signal}`))
-      })
-    }
   }
 
   exit (err) {
@@ -67,17 +57,6 @@ class ExitHandler {
     this.#process.off('exit', this.#handleProcesExitAndReset)
     this.#process.off('uncaughtException', this.#handleExit)
     this.#process.off('unhandledRejection', this.#handleExit)
-
-    const signalsToCleanup = ['SIGTERM', 'SIGINT', 'SIGHUP']
-    for (const signal of signalsToCleanup) {
-      try {
-        this.#process.off(signal, this.#handleExit)
-      } catch (err) {
-        // Ignore errors during cleanup - this is defensive programming for edge cases
-        // where the process object might be in an unexpected state during shutdown
-      }
-    }
-
     if (this.#loaded) {
       this.#npm.unload()
     }
diff --git a/test/lib/cli/exit-handler.js b/test/lib/cli/exit-handler.js
@@ -4,7 +4,7 @@ const EventEmitter = require('node:events')
 const os = require('node:os')
 const t = require('tap')
 const fsMiniPass = require('fs-minipass')
-const { output, time, log } = require('proc-log')
+const { output, time } = require('proc-log')
 const errorMessage = require('../../../lib/utils/error-message.js')
 const ExecCommand = require('../../../lib/commands/exec.js')
 const { load: loadMockNpm } = require('../../fixtures/mock-npm')
@@ -707,136 +707,3 @@ t.test('do no fancy handling for shellouts', async t => {
     })
   })
 })
-
-t.test('container scenarios that trigger exit handler bug', async t => {
-  t.test('process.exit() called before exit handler cleanup', async (t) => {
-    // Simulates when npm process exits directly without going through proper cleanup
-
-    let exitHandlerNeverCalledLogged = false
-    let npmBugReportLogged = false
-
-    await mockExitHandler(t, {
-      config: { loglevel: 'notice' },
-    })
-
-    // Override log.error to capture the specific error messages
-    const originalLogError = log.error
-    log.error = (prefix, msg) => {
-      if (msg === 'Exit handler never called!') {
-        exitHandlerNeverCalledLogged = true
-      }
-      if (msg === 'This is an error with npm itself. Please report this error at:') {
-        npmBugReportLogged = true
-      }
-      return originalLogError(prefix, msg)
-    }
-
-    t.teardown(() => {
-      log.error = originalLogError
-    })
-
-    // This happens when containers are stopped/killed before npm can clean up properly
-    process.emit('exit', 1)
-
-    // Verify the bug is detected and logged correctly
-    t.equal(exitHandlerNeverCalledLogged, true, 'should log "Exit handler never called!" error')
-    t.equal(npmBugReportLogged, true, 'should log npm bug report message')
-  })
-
-  t.test('SIGTERM signal is handled properly', (t) => {
-    // This test verifies that our fix handles SIGTERM signals
-
-    const ExitHandler = tmock(t, '{LIB}/cli/exit-handler.js')
-    const exitHandler = new ExitHandler({ process })
-
-    const initialSigtermCount = process.listeners('SIGTERM').length
-    const initialSigintCount = process.listeners('SIGINT').length
-    const initialSighupCount = process.listeners('SIGHUP').length
-
-    // Register signal handlers
-    exitHandler.registerUncaughtHandlers()
-
-    const finalSigtermCount = process.listeners('SIGTERM').length
-    const finalSigintCount = process.listeners('SIGINT').length
-    const finalSighupCount = process.listeners('SIGHUP').length
-
-    // Verify the fix: signal handlers should be registered
-    t.ok(finalSigtermCount > initialSigtermCount, 'SIGTERM handler should be registered')
-    t.ok(finalSigintCount > initialSigintCount, 'SIGINT handler should be registered')
-    t.ok(finalSighupCount > initialSighupCount, 'SIGHUP handler should be registered')
-
-    // Clean up listeners to avoid affecting other tests
-    const sigtermListeners = process.listeners('SIGTERM')
-    const sigintListeners = process.listeners('SIGINT')
-    const sighupListeners = process.listeners('SIGHUP')
-
-    for (const listener of sigtermListeners) {
-      process.removeListener('SIGTERM', listener)
-    }
-    for (const listener of sigintListeners) {
-      process.removeListener('SIGINT', listener)
-    }
-    for (const listener of sighupListeners) {
-      process.removeListener('SIGHUP', listener)
-    }
-
-    t.end()
-  })
-
-  t.test('signal handler execution', async (t) => {
-    const ExitHandler = tmock(t, '{LIB}/cli/exit-handler.js')
-    const exitHandler = new ExitHandler({ process })
-
-    // Register signal handlers
-    exitHandler.registerUncaughtHandlers()
-
-    process.emit('SIGTERM')
-    process.emit('SIGINT')
-    process.emit('SIGHUP')
-
-    // Clean up listeners
-    process.removeAllListeners('SIGTERM')
-    process.removeAllListeners('SIGINT')
-    process.removeAllListeners('SIGHUP')
-
-    t.pass('signal handlers executed successfully')
-    t.end()
-  })
-
-  t.test('hanging async operation interrupted by signal', async (t) => {
-    // This test simulates the scenario where npm hangs on a long operation and receives SIGTERM/SIGKILL before it can complete
-
-    let exitHandlerNeverCalledLogged = false
-
-    const { exitHandler } = await mockExitHandler(t, {
-      config: { loglevel: 'notice' },
-    })
-
-    // Override log.error to detect the bug message
-    const originalLogError = log.error
-    log.error = (prefix, msg) => {
-      if (msg === 'Exit handler never called!') {
-        exitHandlerNeverCalledLogged = true
-      }
-      return originalLogError(prefix, msg)
-    }
-
-    t.teardown(() => {
-      log.error = originalLogError
-    })
-
-    // Track if exit handler was called properly
-    let exitHandlerCalled = false
-    exitHandler.exit = () => {
-      exitHandlerCalled = true
-    }
-
-    // Simulate sending signal to the process without proper cleanup
-    // This mimics what happens when a container is terminated
-    process.emit('exit', 1)
-
-    // Verify the bug conditions
-    t.equal(exitHandlerCalled, false, 'exit handler should not be called in this scenario')
-    t.equal(exitHandlerNeverCalledLogged, true, 'should detect and log the exit handler bug')
-  })
-})
