feat(signal-termination): handle error codes in the signal value

- fix improper test case termination due to signal termination listener leakage

CHANGELOG.md

@@ -7,6 +7,7 @@
 Note: only supports for `$var` syntax
 - **Upgrade**: Updated `commander` dependency to `v4`
 - **Upgrade**: Updated `sinon` and `nyc` dev dependencies
+- **Fix**: Handle case where the termination signal is the termination code
 
 ## 10.0.1
 

dist/signal-termination.d.ts

@@ -15,7 +15,7 @@ export declare class TermSignals {
     /**
      * Terminate parent process helper
      */
-    _terminateProcess(code?: number, signal?: string): void;
+    _terminateProcess(code?: number, signal?: NodeJS.Signals): void;
     /**
      * Exit event listener clean up helper
      */

dist/signal-termination.js

@@ -17,12 +17,26 @@ class TermSignals {
                 (signal, code) => {
                     this._removeProcessListeners();
                     if (!this._exitCalled) {
-                        if (this.verbose === true) {
+                        if (this.verbose) {
                             console.info(`Parent process exited with signal: ${signal}. Terminating child process...`);
                         }
+                        // Mark shared state so we do not run into a signal/exit loop
                         this._exitCalled = true;
-                        proc.kill(signal);
+                        // Use the signal code if it is an error code
-                        this._terminateProcess(code, signal);
+                        let correctSignal;
+                        if (typeof signal === 'number') {
+                            if (signal > ((code !== null && code !== void 0 ? code : 0))) {
+                                code = signal;
+                                correctSignal = 'SIGINT';
+                            }
+                        }
+                        else {
+                            correctSignal = signal;
+                        }
+                        // Kill the child process
+                        proc.kill((correctSignal !== null && correctSignal !== void 0 ? correctSignal : code));
+                        // Terminate the parent process
+                        this._terminateProcess(code, correctSignal);
                     }
                 };
             process.once(signal, this.terminateSpawnedProcessFuncHandlers[signal]);
@@ -31,14 +45,26 @@ class TermSignals {
         // Terminate parent process if child process receives termination events
         proc.on('exit', (code, signal) => {
             this._removeProcessListeners();
-            const convertedSignal = signal != null ? signal : undefined;
             if (!this._exitCalled) {
-                if (this.verbose === true) {
+                if (this.verbose) {
                     console.info(`Child process exited with code: ${code} and signal: ${signal}. ` +
                         'Terminating parent process...');
                 }
+                // Mark shared state so we do not run into a signal/exit loop
                 this._exitCalled = true;
-                this._terminateProcess(code, convertedSignal);
+                // Use the signal code if it is an error code
+                let correctSignal;
+                if (typeof signal === 'number') {
+                    if (signal > ((code !== null && code !== void 0 ? code : 0))) {
+                        code = signal;
+                        correctSignal = 'SIGINT';
+                    }
+                }
+                else {
+                    correctSignal = (signal !== null && signal !== void 0 ? signal : undefined);
+                }
+                // Terminate the parent process
+                this._terminateProcess(code, correctSignal);
             }
         });
     }

src/signal-termination.ts

@@ -1,4 +1,4 @@
-import { ChildProcess } from 'child_process' // eslint-disable-line
+import { ChildProcess } from 'child_process'
 
 const SIGNALS_TO_HANDLE: NodeJS.Signals[] = [
   'SIGINT', 'SIGTERM', 'SIGHUP'
@@ -17,15 +17,28 @@ export class TermSignals {
     // Terminate child process if parent process receives termination events
     SIGNALS_TO_HANDLE.forEach((signal): void => {
       this.terminateSpawnedProcessFuncHandlers[signal] =
-        (signal: any, code: any): void => {
+        (signal: NodeJS.Signals | number, code: number): void => {
           this._removeProcessListeners()
           if (!this._exitCalled) {
-            if (this.verbose === true) {
+            if (this.verbose) {
               console.info(`Parent process exited with signal: ${signal}. Terminating child process...`)
             }
+            // Mark shared state so we do not run into a signal/exit loop
             this._exitCalled = true
-            proc.kill(signal)
+            // Use the signal code if it is an error code
-            this._terminateProcess(code, signal)
+            let correctSignal: NodeJS.Signals | undefined
+            if (typeof signal === 'number') {
+              if (signal > (code ?? 0)) {
+                code = signal
+                correctSignal = 'SIGINT'
+              }
+            } else {
+              correctSignal = signal
+            }
+            // Kill the child process
+            proc.kill(correctSignal ?? code)
+            // Terminate the parent process
+            this._terminateProcess(code, correctSignal)
           }
         }
       process.once(signal, this.terminateSpawnedProcessFuncHandlers[signal])
@@ -33,18 +46,29 @@ export class TermSignals {
     process.once('exit', this.terminateSpawnedProcessFuncHandlers.SIGTERM)
 
     // Terminate parent process if child process receives termination events
-    proc.on('exit', (code: number | undefined, signal: string | null): void => {
+    proc.on('exit', (code: number | undefined, signal: NodeJS.Signals | number | null): void => {
       this._removeProcessListeners()
-      const convertedSignal = signal != null ? signal : undefined
       if (!this._exitCalled) {
-        if (this.verbose === true) {
+        if (this.verbose) {
           console.info(
             `Child process exited with code: ${code} and signal: ${signal}. ` +
             'Terminating parent process...'
           )
         }
+        // Mark shared state so we do not run into a signal/exit loop
         this._exitCalled = true
-        this._terminateProcess(code, convertedSignal)
+        // Use the signal code if it is an error code
+        let correctSignal: NodeJS.Signals | undefined
+        if (typeof signal === 'number') {
+          if (signal > (code ?? 0)) {
+            code = signal
+            correctSignal = 'SIGINT'
+          }
+        } else {
+          correctSignal = signal ?? undefined
+        }
+        // Terminate the parent process
+        this._terminateProcess(code, correctSignal)
       }
     })
   }
@@ -59,7 +83,7 @@ export class TermSignals {
   /**
    * Terminate parent process helper
    */
-  public _terminateProcess (code?: number, signal?: string): void {
+  public _terminateProcess (code?: number, signal?: NodeJS.Signals): void {
     if (signal !== undefined) {
       return process.kill(process.pid, signal)
     }

test/env-cmd.spec.ts

@@ -1,5 +1,6 @@
 import * as sinon from 'sinon'
 import { assert } from 'chai'
+import * as signalTermLib from '../src/signal-termination'
 import * as parseArgsLib from '../src/parse-args'
 import * as getEnvVarsLib from '../src/get-env-vars'
 import * as expandEnvsLib from '../src/expand-envs'
@@ -7,22 +8,24 @@ import * as spawnLib from '../src/spawn'
 import * as envCmdLib from '../src/env-cmd'
 
 describe('CLI', (): void => {
+  let sandbox: sinon.SinonSandbox
   let parseArgsStub: sinon.SinonStub<any, any>
   let envCmdStub: sinon.SinonStub<any, any>
   let processExitStub: sinon.SinonStub<any, any>
   before((): void => {
-    parseArgsStub = sinon.stub(parseArgsLib, 'parseArgs')
+    sandbox = sinon.createSandbox()
-    envCmdStub = sinon.stub(envCmdLib, 'EnvCmd')
+    parseArgsStub = sandbox.stub(parseArgsLib, 'parseArgs')
-    processExitStub = sinon.stub(process, 'exit')
+    envCmdStub = sandbox.stub(envCmdLib, 'EnvCmd')
+    processExitStub = sandbox.stub(process, 'exit')
   })
 
   after((): void => {
-    sinon.restore()
+    sandbox.restore()
   })
 
   afterEach((): void => {
-    sinon.resetHistory()
+    sandbox.resetHistory()
-    sinon.resetBehavior()
+    sandbox.resetBehavior()
   })
 
   it('should parse the provided args and execute the EnvCmd', async (): Promise<void> => {
@@ -45,25 +48,29 @@ describe('CLI', (): void => {
 })
 
 describe('EnvCmd', (): void => {
+  let sandbox: sinon.SinonSandbox
   let getEnvVarsStub: sinon.SinonStub<any, any>
   let spawnStub: sinon.SinonStub<any, any>
   let expandEnvsSpy: sinon.SinonSpy<any, any>
   before((): void => {
-    getEnvVarsStub = sinon.stub(getEnvVarsLib, 'getEnvVars')
+    sandbox = sinon.createSandbox()
-    spawnStub = sinon.stub(spawnLib, 'spawn')
+    getEnvVarsStub = sandbox.stub(getEnvVarsLib, 'getEnvVars')
+    spawnStub = sandbox.stub(spawnLib, 'spawn')
     spawnStub.returns({
       on: (): void => { /* Fake the on method */ },
       kill: (): void => { /* Fake the kill method */ }
     })
-    expandEnvsSpy = sinon.spy(expandEnvsLib, 'expandEnvs')
+    expandEnvsSpy = sandbox.spy(expandEnvsLib, 'expandEnvs')
+    sandbox.stub(signalTermLib.TermSignals.prototype, 'handleTermSignals')
+    sandbox.stub(signalTermLib.TermSignals.prototype, 'handleUncaughtExceptions')
   })
 
   after((): void => {
-    sinon.restore()
+    sandbox.restore()
   })
 
   afterEach((): void => {
-    sinon.resetHistory()
+    sandbox.resetHistory()
   })
 
   it('should parse the provided args and execute the EnvCmd', async (): Promise<void> => {

test/signal-termination.spec.ts

@@ -3,6 +3,15 @@ import * as sinon from 'sinon'
 import { TermSignals } from '../src/signal-termination'
 
 describe('signal-termination', (): void => {
+  let sandbox: sinon.SinonSandbox
+  before(() => {
+    sandbox = sinon.createSandbox()
+  })
+
+  after(() => {
+    sandbox.restore()
+  })
+
   describe('TermSignals', (): void => {
     describe('_uncaughtExceptionHandler', (): void => {
       const term = new TermSignals()
@@ -10,12 +19,12 @@ describe('signal-termination', (): void => {
       let processStub: sinon.SinonStub<any, any>
 
       beforeEach((): void => {
-        logStub = sinon.stub(console, 'error')
+        logStub = sandbox.stub(console, 'error')
-        processStub = sinon.stub(process, 'exit')
+        processStub = sandbox.stub(process, 'exit')
       })
 
       afterEach((): void => {
-        sinon.restore()
+        sandbox.restore()
       })
 
       it('should print the error message and exit the process with error code 1 ', (): void => {
@@ -32,11 +41,11 @@ describe('signal-termination', (): void => {
       const term = new TermSignals()
       let removeListenerStub: sinon.SinonStub<any, any>
       before((): void => {
-        removeListenerStub = sinon.stub(process, 'removeListener')
+        removeListenerStub = sandbox.stub(process, 'removeListener')
       })
 
       after((): void => {
-        sinon.restore()
+        sandbox.restore()
       })
 
       it('should remove all listeners from default signals and exit signal', (): void => {
@@ -52,12 +61,12 @@ describe('signal-termination', (): void => {
       let killStub: sinon.SinonStub<any, any>
 
       beforeEach((): void => {
-        exitStub = sinon.stub(process, 'exit')
+        exitStub = sandbox.stub(process, 'exit')
-        killStub = sinon.stub(process, 'kill')
+        killStub = sandbox.stub(process, 'kill')
       })
 
       afterEach((): void => {
-        sinon.restore()
+        sandbox.restore()
       })
 
       it('should call exit method on parent process if no signal provided', (): void => {
@@ -95,12 +104,12 @@ describe('signal-termination', (): void => {
       let _uncaughtExceptionHandlerStub: sinon.SinonStub<any, any>
 
       before((): void => {
-        processOnStub = sinon.stub(process, 'on')
+        processOnStub = sandbox.stub(process, 'on')
-        _uncaughtExceptionHandlerStub = sinon.stub(term, '_uncaughtExceptionHandler')
+        _uncaughtExceptionHandlerStub = sandbox.stub(term, '_uncaughtExceptionHandler')
       })
 
       after((): void => {
-        sinon.restore()
+        sandbox.restore()
       })
 
       it('attach handler to the process `uncaughtException` event', (): void => {
@@ -124,11 +133,11 @@ describe('signal-termination', (): void => {
 
       function setup (verbose: boolean = false): void {
         term = new TermSignals({ verbose })
-        procKillStub = sinon.stub()
+        procKillStub = sandbox.stub()
-        procOnStub = sinon.stub()
+        procOnStub = sandbox.stub()
-        processOnceStub = sinon.stub(process, 'once')
+        processOnceStub = sandbox.stub(process, 'once')
-        _removeProcessListenersStub = sinon.stub(term, '_removeProcessListeners')
+        _removeProcessListenersStub = sandbox.stub(term, '_removeProcessListeners')
-        _terminateProcessStub = sinon.stub(term, '_terminateProcess')
+        _terminateProcessStub = sandbox.stub(term, '_terminateProcess')
         proc = {
           kill: procKillStub,
           on: procOnStub
@@ -140,7 +149,7 @@ describe('signal-termination', (): void => {
       })
 
       afterEach((): void => {
-        sinon.restore()
+        sandbox.restore()
       })
 
       it('should setup 4 listeners for the parent process and 1 listen for the child process', (): void => {
@@ -161,9 +170,9 @@ describe('signal-termination', (): void => {
       })
 
       it('should print child process terminated to info for verbose', (): void => {
-        sinon.restore()
+        sandbox.restore()
         setup(true)
-        logInfoStub = sinon.stub(console, 'info')
+        logInfoStub = sandbox.stub(console, 'info')
         assert.notOk(term._exitCalled)
         term.handleTermSignals(proc)
         processOnceStub.args[0][1]('SIGTERM', 1)
@@ -181,8 +190,40 @@ describe('signal-termination', (): void => {
         assert.equal(procKillStub.callCount, 1)
         assert.equal(_terminateProcessStub.callCount, 1)
         assert.isOk(term._exitCalled)
-      }
+      })
-      )
+
+      it('should convert and use number signal as code', (): void => {
+        assert.notOk(term._exitCalled)
+        term.handleTermSignals(proc)
+        processOnceStub.args[0][1](4, 1)
+        assert.equal(_removeProcessListenersStub.callCount, 1)
+        assert.equal(procKillStub.callCount, 1)
+        assert.equal(procKillStub.args[0][0], 'SIGINT')
+        assert.equal(_terminateProcessStub.callCount, 1)
+        assert.isOk(term._exitCalled)
+      })
+
+      it('should not use signal number as code if value is 0', (): void => {
+        assert.notOk(term._exitCalled)
+        term.handleTermSignals(proc)
+        processOnceStub.args[0][1](0, 1)
+        assert.equal(_removeProcessListenersStub.callCount, 1)
+        assert.equal(procKillStub.callCount, 1)
+        assert.equal(procKillStub.args[0], 1)
+        assert.equal(_terminateProcessStub.callCount, 1)
+        assert.isOk(term._exitCalled)
+      })
+
+      it('should use signal value and default SIGINT signal if code is undefined', (): void => {
+        assert.notOk(term._exitCalled)
+        term.handleTermSignals(proc)
+        processOnceStub.args[0][1](4, undefined)
+        assert.equal(_removeProcessListenersStub.callCount, 1)
+        assert.equal(procKillStub.callCount, 1)
+        assert.equal(procKillStub.args[0][0], 'SIGINT')
+        assert.equal(_terminateProcessStub.callCount, 1)
+        assert.isOk(term._exitCalled)
+      })
 
       it('should terminate parent process if child process terminated', (): void => {
         assert.notOk(term._exitCalled)
@@ -194,9 +235,9 @@ describe('signal-termination', (): void => {
       })
 
       it('should print parent process terminated to info for verbose', (): void => {
-        sinon.restore()
+        sandbox.restore()
         setup(true)
-        logInfoStub = sinon.stub(console, 'info')
+        logInfoStub = sandbox.stub(console, 'info')
         assert.notOk(term._exitCalled)
         term.handleTermSignals(proc)
         procOnStub.args[0][1](1, 'SIGTERM')
@@ -224,6 +265,39 @@ describe('signal-termination', (): void => {
         assert.strictEqual(_terminateProcessStub.firstCall.args[1], undefined)
         assert.isOk(term._exitCalled)
       })
+
+      it('should convert and use number signal as code', (): void => {
+        assert.notOk(term._exitCalled)
+        term.handleTermSignals(proc)
+        procOnStub.args[0][1](1, 4)
+        assert.equal(_removeProcessListenersStub.callCount, 1)
+        assert.equal(_terminateProcessStub.callCount, 1)
+        assert.strictEqual(_terminateProcessStub.firstCall.args[0], 4)
+        assert.strictEqual(_terminateProcessStub.firstCall.args[1], 'SIGINT')
+        assert.isOk(term._exitCalled)
+      })
+
+      it('should not use signal number as code if value is 0', (): void => {
+        assert.notOk(term._exitCalled)
+        term.handleTermSignals(proc)
+        procOnStub.args[0][1](1, 0)
+        assert.equal(_removeProcessListenersStub.callCount, 1)
+        assert.equal(_terminateProcessStub.callCount, 1)
+        assert.strictEqual(_terminateProcessStub.firstCall.args[0], 1)
+        assert.isUndefined(_terminateProcessStub.firstCall.args[1])
+        assert.isOk(term._exitCalled)
+      })
+
+      it('should use signal value and default SIGINT signal if code is undefined', (): void => {
+        assert.notOk(term._exitCalled)
+        term.handleTermSignals(proc)
+        procOnStub.args[0][1](null, 1)
+        assert.equal(_removeProcessListenersStub.callCount, 1)
+        assert.equal(_terminateProcessStub.callCount, 1)
+        assert.strictEqual(_terminateProcessStub.firstCall.args[0], 1)
+        assert.strictEqual(_terminateProcessStub.firstCall.args[1], 'SIGINT')
+        assert.isOk(term._exitCalled)
+      })
     })
   })
 })