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

Commit 2a1f44c

Browse files
kentcdoddskentcdodds
committed
Revert "fix: signal handling (#227)"
This reverts commit 8a9cf0e.
1 parent 8a9cf0e commit 2a1f44c

File tree

2 files changed

+48
-145
lines changed

2 files changed

+48
-145
lines changed

‎src/__tests__/index.js

Lines changed: 35 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -6,44 +6,16 @@ jest.mock('cross-spawn')
66

77
const crossEnv = require('../')
88

9-
/*
10-
Each test should spawn at most once.
11-
*/
12-
const getSpawned = () => {
13-
if (crossSpawnMock.spawn.mock.results.length !== 0) {
14-
return crossSpawnMock.spawn.mock.results[0].value
15-
}
16-
17-
return undefined
18-
}
19-
20-
const getSpawnedOnExitCallBack = () => getSpawned().on.mock.calls[0][1]
9+
const getSpawned = (call = 0) => crossSpawnMock.spawn.mock.results[call].value
2110

22-
// Enforce checks for leaking process.on() listeners in cross-env
23-
process.setMaxListeners(1)
11+
process.setMaxListeners(20)
2412

2513
beforeEach(() => {
2614
jest.spyOn(process, 'exit').mockImplementation(() => {})
27-
jest.spyOn(process, 'kill').mockImplementation(() => {})
28-
crossSpawnMock.spawn.mockReturnValue({
29-
pid: 42,
30-
on: jest.fn(),
31-
kill: jest.fn(),
32-
})
15+
crossSpawnMock.spawn.mockReturnValue({on: jest.fn(), kill: jest.fn()})
3316
})
3417

3518
afterEach(() => {
36-
// stop tests from leaking process.on() listeners in cross-env
37-
const spawned = getSpawned()
38-
if (spawned) {
39-
const spawnExitCallback = getSpawnedOnExitCallBack()
40-
const signal = 'SIGTEST'
41-
const exitCode = null
42-
spawnExitCallback(exitCode, signal)
43-
44-
process.removeAllListeners('exit')
45-
}
46-
4719
jest.clearAllMocks()
4820
process.exit.mockRestore()
4921
})
@@ -158,6 +130,20 @@ test(`does not normalize command arguments on windows`, () => {
158130
)
159131
})
160132

133+
test(`propagates kill signals`, () => {
134+
testEnvSetting({FOO_ENV: 'foo=bar'}, 'FOO_ENV="foo=bar"')
135+
136+
process.emit('SIGTERM')
137+
process.emit('SIGINT')
138+
process.emit('SIGHUP')
139+
process.emit('SIGBREAK')
140+
const spawned = getSpawned()
141+
expect(spawned.kill).toHaveBeenCalledWith('SIGTERM')
142+
expect(spawned.kill).toHaveBeenCalledWith('SIGINT')
143+
expect(spawned.kill).toHaveBeenCalledWith('SIGHUP')
144+
expect(spawned.kill).toHaveBeenCalledWith('SIGBREAK')
145+
})
146+
161147
test(`keeps backslashes`, () => {
162148
isWindowsMock.mockReturnValue(true)
163149
crossEnv(['echo', '\\\\\\\\someshare\\\\somefolder'])
@@ -171,73 +157,29 @@ test(`keeps backslashes`, () => {
171157
)
172158
})
173159

174-
describe(`cross-env delegates signals to spawn`, () => {
175-
test(`SIGINT is not delegated`, () => {
176-
const signal = 'SIGINT'
177-
178-
crossEnv(['echo', 'hello world'])
179-
const spawnExitCallback = getSpawnedOnExitCallBack()
180-
const exitCode = null
181-
const parentProcessId = expect.any(Number)
182-
183-
// Parent receives signal
184-
// SIGINT is sent to all processes in group, no need to delegated.
185-
process.emit(signal)
186-
expect(process.kill).not.toHaveBeenCalled()
187-
// child handles signal and 'exits'
188-
spawnExitCallback(exitCode, signal)
189-
expect(process.kill).toHaveBeenCalledTimes(1)
190-
expect(process.kill).toHaveBeenCalledWith(parentProcessId, signal)
191-
})
192-
193-
test.each(['SIGTERM', 'SIGBREAK', 'SIGHUP', 'SIGQUIT'])(
194-
`delegates %s`,
195-
signal => {
196-
crossEnv(['echo', 'hello world'])
197-
const spawnExitCallback = getSpawnedOnExitCallBack()
198-
const exitCode = null
199-
const parentProcessId = expect.any(Number)
200-
201-
// Parent receives signal
202-
process.emit(signal)
203-
expect(process.kill).toHaveBeenCalledTimes(1)
204-
expect(process.kill).toHaveBeenCalledWith(42, signal)
205-
// Parent delegates signal to child, child handles signal and 'exits'
206-
spawnExitCallback(exitCode, signal)
207-
expect(process.kill).toHaveBeenCalledTimes(2)
208-
expect(process.kill).toHaveBeenCalledWith(parentProcessId, signal)
209-
},
210-
)
160+
test(`propagates unhandled exit signal`, () => {
161+
const {spawned} = testEnvSetting({FOO_ENV: 'foo=bar'}, 'FOO_ENV="foo=bar"')
162+
const spawnExitCallback = spawned.on.mock.calls[0][1]
163+
const spawnExitCode = null
164+
spawnExitCallback(spawnExitCode)
165+
expect(process.exit).toHaveBeenCalledWith(1)
211166
})
212167

213-
describe(`spawn received signal and exits`, () => {
214-
test.each(['SIGTERM', 'SIGINT', 'SIGBREAK', 'SIGHUP', 'SIGQUIT'])(
215-
`delegates %s`,
216-
signal => {
217-
crossEnv(['echo', 'hello world'])
218-
219-
const spawnExitCallback = getSpawnedOnExitCallBack()
220-
const exitCode = null
221-
const parentProcessId = expect.any(Number)
222-
223-
// cross-env child.on('exit') re-raises signal, now with no signal handlers
224-
spawnExitCallback(exitCode, signal)
225-
process.emit('exit', exitCode, signal)
226-
expect(process.kill).toHaveBeenCalledTimes(1)
227-
expect(process.kill).toHaveBeenCalledWith(parentProcessId, signal)
228-
},
229-
)
168+
test(`exits cleanly with SIGINT with a null exit code`, () => {
169+
const {spawned} = testEnvSetting({FOO_ENV: 'foo=bar'}, 'FOO_ENV="foo=bar"')
170+
const spawnExitCallback = spawned.on.mock.calls[0][1]
171+
const spawnExitCode = null
172+
const spawnExitSignal = 'SIGINT'
173+
spawnExitCallback(spawnExitCode, spawnExitSignal)
174+
expect(process.exit).toHaveBeenCalledWith(0)
230175
})
231176

232-
test(`spawn regular exit code`, () => {
233-
crossEnv(['echo', 'hello world'])
234-
235-
const spawnExitCallback = getSpawnedOnExitCallBack()
177+
test(`propagates regular exit code`, () => {
178+
const {spawned} = testEnvSetting({FOO_ENV: 'foo=bar'}, 'FOO_ENV="foo=bar"')
179+
const spawnExitCallback = spawned.on.mock.calls[0][1]
236180
const spawnExitCode = 0
237-
const spawnExitSignal = null
238-
spawnExitCallback(spawnExitCode, spawnExitSignal)
239-
expect(process.exit).not.toHaveBeenCalled()
240-
expect(process.exitCode).toBe(0)
181+
spawnExitCallback(spawnExitCode)
182+
expect(process.exit).toHaveBeenCalledWith(0)
241183
})
242184

243185
function testEnvSetting(expected, ...envSettings) {

‎src/index.js

Lines changed: 13 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ function crossEnv(args, options = {}) {
1010
const [envSetters, command, commandArgs] = parseCommand(args)
1111
const env = getEnvVars(envSetters)
1212
if (command) {
13-
let child = spawn(
13+
const proc = spawn(
1414
// run `path.normalize` for command(on windows)
1515
commandConvert(command, env, true),
1616
// by default normalize is `false`, so not run for cmd args
@@ -21,59 +21,20 @@ function crossEnv(args, options = {}) {
2121
env,
2222
},
2323
)
24-
25-
// See https://github.com/jtlapp/node-cleanup
26-
//
27-
// > When you hit Ctrl-C, you send a SIGINT signal to each process in the
28-
// > current process group. A process group is set of processes that are
29-
// > all supposed to end together as a group instead of persisting
30-
// > independently. However, some programs, such as Emacs, intercept and
31-
// > repurpose SIGINT so that it does not end the process. In such cases,
32-
// > SIGINT should not end any processes of the group.
33-
//
34-
// Delegate decision to terminate to child process, if the child exits on
35-
// SIGINT then the `child.on('exit')` callback will be invoked, re-raising
36-
// the signal to the parent process
37-
const delegateSignalToChild = signal => () => {
38-
// SIGINT is sent to all processes in group, no need to delegate.
39-
if (child && signal !== 'SIGINT') {
40-
process.kill(child.pid, signal)
41-
}
42-
}
43-
44-
const sigtermHandler = delegateSignalToChild('SIGTERM')
45-
const sigintHandler = delegateSignalToChild('SIGINT')
46-
const sigbreakHandler = delegateSignalToChild('SIGBREAK')
47-
const sighupHandler = delegateSignalToChild('SIGHUP')
48-
const sigquitHandler = delegateSignalToChild('SIGQUIT')
49-
50-
process.on('SIGTERM', sigtermHandler)
51-
process.on('SIGINT', sigintHandler)
52-
process.on('SIGBREAK', sigbreakHandler)
53-
process.on('SIGHUP', sighupHandler)
54-
process.on('SIGQUIT', sigquitHandler)
55-
56-
child.on('exit', (exitCode, signal) => {
57-
// Child has decided to exit.
58-
child = null
59-
process.removeListener('SIGTERM', sigtermHandler)
60-
process.removeListener('SIGINT', sigintHandler)
61-
process.removeListener('SIGBREAK', sigbreakHandler)
62-
process.removeListener('SIGHUP', sighupHandler)
63-
process.removeListener('SIGQUIT', sigquitHandler)
64-
65-
if (exitCode !== null) {
66-
// Calling process.exit is not necessary most of the time,
67-
// see https://nodejs.org/api/process.html#process_process_exit_code
68-
process.exitCode = exitCode
69-
}
70-
if (signal !== null) {
71-
// Pass through child's signal to parent.
72-
// SIGINT should not be transformed into a 0 exit code
73-
process.kill(process.pid, signal)
24+
process.on('SIGTERM', () => proc.kill('SIGTERM'))
25+
process.on('SIGINT', () => proc.kill('SIGINT'))
26+
process.on('SIGBREAK', () => proc.kill('SIGBREAK'))
27+
process.on('SIGHUP', () => proc.kill('SIGHUP'))
28+
proc.on('exit', (code, signal) => {
29+
let crossEnvExitCode = code
30+
// exit code could be null when OS kills the process(out of memory, etc) or due to node handling it
31+
// but if the signal is SIGINT the user exited the process so we want exit code 0
32+
if (crossEnvExitCode === null) {
33+
crossEnvExitCode = signal === 'SIGINT' ? 0 : 1
7434
}
35+
process.exit(crossEnvExitCode) //eslint-disable-line no-process-exit
7536
})
76-
return child
37+
return proc
7738
}
7839
return null
7940
}

0 commit comments

Comments
 (0)