Skip to content
This repository has been archived by the owner on Jan 6, 2021. It is now read-only.

Commit

Permalink
Revert "fix: signal handling (#227)"
Browse files Browse the repository at this point in the history 
This reverts commit 8a9cf0e.
  • Loading branch information
@kentcdodds
kentcdodds committed Mar 5, 2020
1 parent 8a9cf0e commit 2a1f44c
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 145 deletions.
128 changes: 35 additions & 93 deletions src/__tests__/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,44 +6,16 @@ jest.mock('cross-spawn')

const crossEnv = require('../')

/*
Each test should spawn at most once.
*/
const getSpawned = () => {
if (crossSpawnMock.spawn.mock.results.length !== 0) {
return crossSpawnMock.spawn.mock.results[0].value
}

return undefined
}

const getSpawnedOnExitCallBack = () => getSpawned().on.mock.calls[0][1]
const getSpawned = (call = 0) => crossSpawnMock.spawn.mock.results[call].value

// Enforce checks for leaking process.on() listeners in cross-env
process.setMaxListeners(1)
process.setMaxListeners(20)

beforeEach(() => {
jest.spyOn(process, 'exit').mockImplementation(() => {})
jest.spyOn(process, 'kill').mockImplementation(() => {})
crossSpawnMock.spawn.mockReturnValue({
pid: 42,
on: jest.fn(),
kill: jest.fn(),
})
crossSpawnMock.spawn.mockReturnValue({on: jest.fn(), kill: jest.fn()})
})

afterEach(() => {
// stop tests from leaking process.on() listeners in cross-env
const spawned = getSpawned()
if (spawned) {
const spawnExitCallback = getSpawnedOnExitCallBack()
const signal = 'SIGTEST'
const exitCode = null
spawnExitCallback(exitCode, signal)

process.removeAllListeners('exit')
}

jest.clearAllMocks()
process.exit.mockRestore()
})
Expand Down Expand Up @@ -158,6 +130,20 @@ test(`does not normalize command arguments on windows`, () => {
)
})

test(`propagates kill signals`, () => {
testEnvSetting({FOO_ENV: 'foo=bar'}, 'FOO_ENV="foo=bar"')

process.emit('SIGTERM')
process.emit('SIGINT')
process.emit('SIGHUP')
process.emit('SIGBREAK')
const spawned = getSpawned()
expect(spawned.kill).toHaveBeenCalledWith('SIGTERM')
expect(spawned.kill).toHaveBeenCalledWith('SIGINT')
expect(spawned.kill).toHaveBeenCalledWith('SIGHUP')
expect(spawned.kill).toHaveBeenCalledWith('SIGBREAK')
})

test(`keeps backslashes`, () => {
isWindowsMock.mockReturnValue(true)
crossEnv(['echo', '\\\\\\\\someshare\\\\somefolder'])
Expand All @@ -171,73 +157,29 @@ test(`keeps backslashes`, () => {
)
})

describe(`cross-env delegates signals to spawn`, () => {
test(`SIGINT is not delegated`, () => {
const signal = 'SIGINT'

crossEnv(['echo', 'hello world'])
const spawnExitCallback = getSpawnedOnExitCallBack()
const exitCode = null
const parentProcessId = expect.any(Number)

// Parent receives signal
// SIGINT is sent to all processes in group, no need to delegated.
process.emit(signal)
expect(process.kill).not.toHaveBeenCalled()
// child handles signal and 'exits'
spawnExitCallback(exitCode, signal)
expect(process.kill).toHaveBeenCalledTimes(1)
expect(process.kill).toHaveBeenCalledWith(parentProcessId, signal)
})

test.each(['SIGTERM', 'SIGBREAK', 'SIGHUP', 'SIGQUIT'])(
`delegates %s`,
signal => {
crossEnv(['echo', 'hello world'])
const spawnExitCallback = getSpawnedOnExitCallBack()
const exitCode = null
const parentProcessId = expect.any(Number)

// Parent receives signal
process.emit(signal)
expect(process.kill).toHaveBeenCalledTimes(1)
expect(process.kill).toHaveBeenCalledWith(42, signal)
// Parent delegates signal to child, child handles signal and 'exits'
spawnExitCallback(exitCode, signal)
expect(process.kill).toHaveBeenCalledTimes(2)
expect(process.kill).toHaveBeenCalledWith(parentProcessId, signal)
},
)
test(`propagates unhandled exit signal`, () => {
const {spawned} = testEnvSetting({FOO_ENV: 'foo=bar'}, 'FOO_ENV="foo=bar"')
const spawnExitCallback = spawned.on.mock.calls[0][1]
const spawnExitCode = null
spawnExitCallback(spawnExitCode)
expect(process.exit).toHaveBeenCalledWith(1)
})

describe(`spawn received signal and exits`, () => {
test.each(['SIGTERM', 'SIGINT', 'SIGBREAK', 'SIGHUP', 'SIGQUIT'])(
`delegates %s`,
signal => {
crossEnv(['echo', 'hello world'])

const spawnExitCallback = getSpawnedOnExitCallBack()
const exitCode = null
const parentProcessId = expect.any(Number)

// cross-env child.on('exit') re-raises signal, now with no signal handlers
spawnExitCallback(exitCode, signal)
process.emit('exit', exitCode, signal)
expect(process.kill).toHaveBeenCalledTimes(1)
expect(process.kill).toHaveBeenCalledWith(parentProcessId, signal)
},
)
test(`exits cleanly with SIGINT with a null exit code`, () => {
const {spawned} = testEnvSetting({FOO_ENV: 'foo=bar'}, 'FOO_ENV="foo=bar"')
const spawnExitCallback = spawned.on.mock.calls[0][1]
const spawnExitCode = null
const spawnExitSignal = 'SIGINT'
spawnExitCallback(spawnExitCode, spawnExitSignal)
expect(process.exit).toHaveBeenCalledWith(0)
})

test(`spawn regular exit code`, () => {
crossEnv(['echo', 'hello world'])

const spawnExitCallback = getSpawnedOnExitCallBack()
test(`propagates regular exit code`, () => {
const {spawned} = testEnvSetting({FOO_ENV: 'foo=bar'}, 'FOO_ENV="foo=bar"')
const spawnExitCallback = spawned.on.mock.calls[0][1]
const spawnExitCode = 0
const spawnExitSignal = null
spawnExitCallback(spawnExitCode, spawnExitSignal)
expect(process.exit).not.toHaveBeenCalled()
expect(process.exitCode).toBe(0)
spawnExitCallback(spawnExitCode)
expect(process.exit).toHaveBeenCalledWith(0)
})

function testEnvSetting(expected, ...envSettings) {
Expand Down
65 changes: 13 additions & 52 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function crossEnv(args, options = {}) {
const [envSetters, command, commandArgs] = parseCommand(args)
const env = getEnvVars(envSetters)
if (command) {
let child = spawn(
const proc = spawn(
// run `path.normalize` for command(on windows)
commandConvert(command, env, true),
// by default normalize is `false`, so not run for cmd args
Expand All @@ -21,59 +21,20 @@ function crossEnv(args, options = {}) {
env,
},
)

// See https://github.com/jtlapp/node-cleanup
//
// > When you hit Ctrl-C, you send a SIGINT signal to each process in the
// > current process group. A process group is set of processes that are
// > all supposed to end together as a group instead of persisting
// > independently. However, some programs, such as Emacs, intercept and
// > repurpose SIGINT so that it does not end the process. In such cases,
// > SIGINT should not end any processes of the group.
//
// Delegate decision to terminate to child process, if the child exits on
// SIGINT then the `child.on('exit')` callback will be invoked, re-raising
// the signal to the parent process
const delegateSignalToChild = signal => () => {
// SIGINT is sent to all processes in group, no need to delegate.
if (child && signal !== 'SIGINT') {
process.kill(child.pid, signal)
}
}

const sigtermHandler = delegateSignalToChild('SIGTERM')
const sigintHandler = delegateSignalToChild('SIGINT')
const sigbreakHandler = delegateSignalToChild('SIGBREAK')
const sighupHandler = delegateSignalToChild('SIGHUP')
const sigquitHandler = delegateSignalToChild('SIGQUIT')

process.on('SIGTERM', sigtermHandler)
process.on('SIGINT', sigintHandler)
process.on('SIGBREAK', sigbreakHandler)
process.on('SIGHUP', sighupHandler)
process.on('SIGQUIT', sigquitHandler)

child.on('exit', (exitCode, signal) => {
// Child has decided to exit.
child = null
process.removeListener('SIGTERM', sigtermHandler)
process.removeListener('SIGINT', sigintHandler)
process.removeListener('SIGBREAK', sigbreakHandler)
process.removeListener('SIGHUP', sighupHandler)
process.removeListener('SIGQUIT', sigquitHandler)

if (exitCode !== null) {
// Calling process.exit is not necessary most of the time,
// see https://nodejs.org/api/process.html#process_process_exit_code
process.exitCode = exitCode
}
if (signal !== null) {
// Pass through child's signal to parent.
// SIGINT should not be transformed into a 0 exit code
process.kill(process.pid, signal)
process.on('SIGTERM', () => proc.kill('SIGTERM'))
process.on('SIGINT', () => proc.kill('SIGINT'))
process.on('SIGBREAK', () => proc.kill('SIGBREAK'))
process.on('SIGHUP', () => proc.kill('SIGHUP'))
proc.on('exit', (code, signal) => {
let crossEnvExitCode = code
// exit code could be null when OS kills the process(out of memory, etc) or due to node handling it
// but if the signal is SIGINT the user exited the process so we want exit code 0
if (crossEnvExitCode === null) {
crossEnvExitCode = signal === 'SIGINT' ? 0 : 1
}
process.exit(crossEnvExitCode) //eslint-disable-line no-process-exit
})
return child
return proc
}
return null
}
Expand Down

0 comments on commit 2a1f44c

Please sign in to comment.