Skip to content

Commit df6d580

Browse files
fix: Set up listeners first, in case of long write to stdin. (#520)
Co-authored-by: Jordan Rickman <jordan@alloy.com> Co-authored-by: Jordan Rickman <jordan.rickman.42@gmail.com>
1 parent c959558 commit df6d580

File tree

2 files changed

+45
-8
lines changed

2 files changed

+45
-8
lines changed

src/exec.js

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,26 @@ const exec = (command, args, stdin, cwd) => {
1212

1313
const process = childProcess.spawn(command, args, spawnOptions)
1414

15+
// All of these handlers can close the Promise, so guard against rejecting it twice.
16+
let promiseAlreadyRejected = false
17+
process.on('close', code => {
18+
if (!promiseAlreadyRejected) {
19+
promiseAlreadyRejected = true
20+
if (code !== 0) {
21+
return reject(Error(stderr))
22+
} else {
23+
return resolve(stripFinalNewline(stdout))
24+
}
25+
}
26+
})
27+
1528
if (stdin) {
29+
process.stdin.on('error', err => {
30+
if (!promiseAlreadyRejected) {
31+
promiseAlreadyRejected = true
32+
return reject(err)
33+
}
34+
})
1635
process.stdin.setEncoding('utf-8')
1736
process.stdin.write(stdin)
1837
process.stdin.end()
@@ -26,14 +45,6 @@ const exec = (command, args, stdin, cwd) => {
2645
process.stderr.on('data', data => {
2746
stderr += data
2847
})
29-
30-
process.on('close', code => {
31-
if (code !== 0) {
32-
return reject(Error(stderr))
33-
} else {
34-
return resolve(stripFinalNewline(stdout))
35-
}
36-
})
3748
})
3849
}
3950

src/jq.test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { expect } from 'chai'
22
import path from 'path'
3+
import { readFileSync } from 'fs'
34

45
import { run } from './jq'
56
import { FILTER_UNDEFINED_ERROR } from './command'
@@ -74,6 +75,31 @@ describe('jq core', () => {
7475
})
7576
})
7677

78+
it('should catch errors from child process stdin', done => {
79+
// This is a very specific case of error, only triggered if:
80+
// 1) The jq process exits early (e.g. due to an invalid filter)
81+
// AND
82+
// 2) We are trying to send a large amount of data over stdin,
83+
// (which will take longer to do than jq takes to exit).
84+
const largeJsonString = JSON.parse(readFileSync(PATH_LARGE_JSON_FIXTURE))
85+
run(FILTER_INVALID, largeJsonString, { input: 'json' })
86+
.then(result => {
87+
done('Expected an error to be thrown from child process stdin')
88+
})
89+
.catch(error => {
90+
expect(error).to.be.an.instanceof(Error)
91+
// On Mac/Linux, the error code is "EPIPE".
92+
// On Windows, the equivalent code is "EOF".
93+
expect(error.message).to.be.oneOf(['write EPIPE', 'write EOF'])
94+
expect(error.code).to.be.oneOf(['EPIPE', 'EOF'])
95+
expect(error.syscall).to.equal('write')
96+
done()
97+
})
98+
.catch(error => {
99+
done(error)
100+
})
101+
})
102+
77103
it('should fail on an undefined filter', done => {
78104
run(undefined, PATH_JSON_FIXTURE)
79105
.catch(error => {

0 commit comments

Comments
 (0)