Skip to content

Commit 921fa36

Browse files
committed
Handle non-zero exit status when formatting using shfmt
Non-zero exist statuses from `shfmt` weren't checked for or handled. As a result the (empty) output from `shfmt` was returned as the formatted document. An exception is now thrown when a non-zero exit status is encountered. Fixes #1162
1 parent 2d51e28 commit 921fa36

File tree

2 files changed

+14
-6
lines changed

2 files changed

+14
-6
lines changed

server/src/shfmt/__tests__/index.test.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe('formatter', () => {
3838
expect(new Formatter({ executablePath: 'foo' }).canFormat).toBe(true)
3939
})
4040

41-
it('should set canFormat to false when formatting fails', async () => {
41+
it('should set canFormat to false when the executable cannot be found', async () => {
4242
const [result, formatter] = await getFormattingResult({
4343
document: textToDoc(''),
4444
executablePath: 'foo',
@@ -54,6 +54,14 @@ describe('formatter', () => {
5454
)
5555
})
5656

57+
it('should throw when formatting fails', async () => {
58+
expect(async () => {
59+
await getFormattingResult({ document: FIXTURE_DOCUMENT.PARSE_PROBLEMS })
60+
}).rejects.toThrow(
61+
'Shfmt: exited with status 1: <standard input>:10:1: > must be followed by a word',
62+
)
63+
})
64+
5765
it('should format when shfmt is present', async () => {
5866
const [result] = await getFormattingResult({ document: FIXTURE_DOCUMENT.SHFMT })
5967
expect(result).toMatchInlineSnapshot(`

server/src/shfmt/index.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,10 @@ export class Formatter {
9191
proc.stdin.end(documentText)
9292
})
9393

94-
// NOTE: do we care about exit code? 0 means "ok", 1 possibly means "errors",
95-
// but the presence of parseable errors in the output is also sufficient to
96-
// distinguish.
9794
let exit
9895
try {
9996
exit = await proc
10097
} catch (e) {
101-
// TODO: we could do this up front?
10298
if ((e as any).code === 'ENOENT') {
10399
// shfmt path wasn't found, don't try to format any more:
104100
logger.warn(
@@ -107,7 +103,11 @@ export class Formatter {
107103
this._canFormat = false
108104
return ''
109105
}
110-
throw new Error(`Shfmt: failed with code ${exit}: ${e}\nout:\n${out}\nerr:\n${err}`)
106+
throw new Error(`Shfmt: child process error: ${e}`)
107+
}
108+
109+
if (exit != 0) {
110+
throw new Error(`Shfmt: exited with status ${exit}: ${err}`)
111111
}
112112

113113
return out

0 commit comments

Comments
 (0)