Skip to content

Commit

Permalink
fix: stream in error without error content handling (#662)
Browse files Browse the repository at this point in the history
Co-authored-by: Sebastien <[email protected]>
  • Loading branch information
gdman and scolladon authored Jul 27, 2023
1 parent 5bfbdac commit 97ccab0
Show file tree
Hide file tree
Showing 5 changed files with 351 additions and 331 deletions.
3 changes: 1 addition & 2 deletions __mocks__/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ childProcess.spawn.mockImplementation(() => {
this.push(output.pop().join(EOL))
}
this.push(null)
mock.emit('close')
},
})
mock.stderr = new Readable({
Expand All @@ -26,9 +25,9 @@ childProcess.spawn.mockImplementation(() => {
this.push('error')
}
this.push(null)
mock.emit('close')
},
})
setTimeout(() => mock.emit('close', error ? 1 : 0), 0)
return mock
})

Expand Down
66 changes: 37 additions & 29 deletions __tests__/unit/lib/utils/childProcessUtils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,30 @@ const {
const { EventEmitter, Readable } = require('stream')
const { sep } = require('path')

const arrangeStream = (data, error, isError) => {
const getReadable = content => {
return new Readable({
read() {
if (content) this.push(content)
this.push(null)
},
})
}
const stream = new EventEmitter()
stream.stdout = getReadable(data)
stream.stderr = getReadable(error)
setTimeout(() => stream.emit('close', isError ? 1 : 0), 0)
return stream
}

describe('childProcessUtils', () => {
describe('getStreamContent', () => {
describe.each([Buffer.from('text'), 'text'])(
'when called with stream of %o',
content => {
it('returns Buffer', async () => {
// Arrange
const stream = new EventEmitter()
stream.stdout = new Readable({
read() {
this.push(content)
this.push(null)
stream.emit('close')
},
})
stream.stderr = new Readable({
read() {
this.push(null)
stream.emit('close')
},
})
const stream = arrangeStream(content, null, false)

// Act
const result = await getStreamContent(stream)
Expand All @@ -40,25 +43,12 @@ describe('childProcessUtils', () => {
}
)

describe('when stream emit error', () => {
describe('when stream emits error', () => {
it('throws the error', async () => {
// Arrange
expect.assertions(1)
const stream = new EventEmitter()
stream.stdout = new Readable({
read() {
this.push(null)
stream.emit('close')
},
})

stream.stderr = new Readable({
read() {
this.push('error')
this.push(null)
stream.emit('close')
},
})
const stream = arrangeStream('irrelevant std out output', 'error', true)

// Act
try {
Expand All @@ -70,6 +60,24 @@ describe('childProcessUtils', () => {
}
})
})

describe('when stream emits error but no stderr output', () => {
it('throws an empty error', async () => {
// Arrange
expect.assertions(1)

const stream = arrangeStream('irrelevant std out output', null, true)

// Act
try {
await getStreamContent(stream)

// Assert
} catch (error) {
expect(error.message).toEqual('')
}
})
})
})
describe('linify', () => {
describe('when called with lines', () => {
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,16 @@
"@stryker-mutator/jest-runner": "^7.1.1",
"@swc/core": "^1.3.71",
"@types/mocha": "^10.0.1",
"@types/node": "^20.4.4",
"@types/node": "^20.4.5",
"@typescript-eslint/parser": "^6.2.0",
"benchmark": "^2.1.4",
"chai": "^4.3.7",
"depcheck": "^1.4.3",
"eslint": "^8.45.0",
"eslint-config-prettier": "^8.8.0",
"eslint-config-prettier": "^8.9.0",
"eslint-plugin-prettier": "^5.0.0",
"husky": "^8.0.3",
"jest": "^29.6.1",
"jest": "^29.6.2",
"lint-staged": "^13.2.3",
"mocha": "^10.2.0",
"nyc": "^15.1.0",
Expand Down
33 changes: 21 additions & 12 deletions src/utils/childProcessUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,27 @@ async function* linify(stream) {
}

const getStreamContent = async stream => {
const content = []
for await (const chunk of stream.stdout) {
content.push(chunk)
}
const error = []
for await (const chunk of stream.stderr) {
error.push(chunk)
}
if (error.length > 0) {
throw new Error(error.join(''))
}
return Buffer.concat(content)
return new Promise((resolve, reject) => {
const content = []
const error = []

stream.stdout.on('data', data => {
content.push(data)
})

stream.stderr.setEncoding('utf8')
stream.stderr.on('data', data => {
error.push(data.toString())
})

stream.on('close', code => {
if (code !== 0) {
reject(new Error(error.join('')))
}

resolve(Buffer.concat(content))
})
})
}

module.exports.EOLRegex = EOLRegex
Expand Down
Loading

0 comments on commit 97ccab0

Please sign in to comment.