Skip to content

Commit 250c767

Browse files
CKodidelacameri
andauthored
fix: docker availability precheck (#596)
* refactor: normalize runCommandWithOutput result type to handle spawn errors * fix: add braces to single-line if statements in process.ts * chore: add changeset for runCommandWithOutput normalization * test: add unit tests for runCommandWithOutput covering all result paths --------- Co-authored-by: Ricardo Cabral <me@ricardocabral.io>
1 parent f599b1d commit 250c767

7 files changed

Lines changed: 116 additions & 25 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"nostream": patch
3+
---
4+
5+
Normalize runCommandWithOutput to return a CommandResult discriminated union instead of rejecting on spawn errors, fixing a crash in `info --json` when Docker is not installed.

src/cli/commands/info.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,8 @@ const getEventCount = async (): Promise<number | null> => {
5656
}
5757

5858
const getRelayUptimeSeconds = async (): Promise<number | null> => {
59-
let idResult: { code: number; stdout: string; stderr: string }
60-
try {
61-
idResult = await runCommandWithOutput('docker', ['compose', 'ps', '-q', 'nostream'], { timeoutMs: 1000 })
62-
} catch {
63-
return null
64-
}
65-
if (idResult.code !== 0) {
59+
const idResult = await runCommandWithOutput('docker', ['compose', 'ps', '-q', 'nostream'], { timeoutMs: 1000 })
60+
if (!idResult.ok || idResult.code !== 0) {
6661
return null
6762
}
6863

@@ -74,7 +69,7 @@ const getRelayUptimeSeconds = async (): Promise<number | null> => {
7469
const startedAtResult = await runCommandWithOutput('docker', ['inspect', '--format', '{{.State.StartedAt}}', containerId], {
7570
timeoutMs: 1000,
7671
})
77-
if (startedAtResult.code !== 0) {
72+
if (!startedAtResult.ok || startedAtResult.code !== 0) {
7873
return null
7974
}
8075

src/cli/commands/update.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ export const runUpdate = async (passthrough: string[]): Promise<number> => {
1818
}
1919

2020
const stashResult = await runCommandWithOutput('git', ['stash', 'push', '-u', '-m', 'nostream-cli-update'])
21+
if (!stashResult.ok) {
22+
spinner.fail(stashResult.ok === false && stashResult.reason === 'not-found' ? 'Update failed: git is not installed' : 'Update failed while stashing local changes')
23+
return 1
24+
}
2125
if (stashResult.code !== 0) {
2226
spinner.fail('Update failed while stashing local changes')
2327
return stashResult.code

src/cli/utils/process.ts

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ export type RunOptions = {
77
timeoutMs?: number
88
}
99

10+
export type CommandResult =
11+
| { ok: true; code: number; stdout: string; stderr: string }
12+
| { ok: false; reason: 'not-found' | 'permission-denied' | 'spawn-error' | 'timeout' | 'signal'; stdout: string; stderr: string }
13+
1014
export const runCommand = (command: string, args: string[], options: RunOptions = {}): Promise<number> => {
1115
return new Promise((resolve, reject) => {
1216
const child = spawn(command, args, {
@@ -38,10 +42,19 @@ export const runCommandWithOutput = (
3842
command: string,
3943
args: string[],
4044
options: RunOptions = {},
41-
): Promise<{ code: number; stdout: string; stderr: string }> => {
42-
return new Promise((resolve, reject) => {
45+
): Promise<CommandResult> => {
46+
return new Promise((resolve) => {
4347
let stdout = ''
4448
let stderr = ''
49+
let timedOut = false
50+
let settled = false
51+
52+
const settle = (result: CommandResult) => {
53+
if (!settled) {
54+
settled = true
55+
resolve(result)
56+
}
57+
}
4558

4659
const child = spawn(command, args, {
4760
cwd: options.cwd,
@@ -53,6 +66,7 @@ export const runCommandWithOutput = (
5366
const timer =
5467
typeof options.timeoutMs === 'number'
5568
? setTimeout(() => {
69+
timedOut = true
5670
child.kill('SIGTERM')
5771
}, options.timeoutMs)
5872
: undefined
@@ -65,17 +79,31 @@ export const runCommandWithOutput = (
6579
stderr += chunk.toString()
6680
})
6781

68-
child.on('error', reject)
69-
child.on('close', (code) => {
70-
if (timer) {
71-
clearTimeout(timer)
82+
child.on('error', (err: NodeJS.ErrnoException) => {
83+
if (timer) { clearTimeout(timer) }
84+
if (err.code === 'ENOENT') {
85+
settle({ ok: false, reason: 'not-found', stdout, stderr })
86+
} else if (err.code === 'EACCES') {
87+
settle({ ok: false, reason: 'permission-denied', stdout, stderr })
88+
} else {
89+
settle({ ok: false, reason: 'spawn-error', stdout, stderr })
90+
}
91+
})
92+
93+
child.on('close', (code, signal) => {
94+
if (timer) { clearTimeout(timer) }
95+
96+
if (timedOut) {
97+
settle({ ok: false, reason: 'timeout', stdout, stderr })
98+
return
99+
}
100+
101+
if (signal !== null && code === null) {
102+
settle({ ok: false, reason: 'signal', stdout, stderr })
103+
return
72104
}
73105

74-
resolve({
75-
code: code ?? 1,
76-
stdout,
77-
stderr,
78-
})
106+
settle({ ok: true, code: code ?? 1, stdout, stderr })
79107
})
80108
})
81109
}

test/unit/cli/info.spec.ts

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,27 @@ describe('runInfo', () => {
3131
sinon.restore()
3232
})
3333

34+
it('outputs valid JSON when docker is not installed (ENOENT)', async () => {
35+
sinon.stub(fs, 'existsSync').returns(false)
36+
sinon.stub(processUtils, 'runCommandWithOutput').resolves({ ok: false, reason: 'not-found', stdout: '', stderr: '' })
37+
38+
const code = await infoCommand.runInfo({ json: true })
39+
40+
expect(code).to.equal(0)
41+
const parsed = JSON.parse(stdout)
42+
expect(parsed).to.have.nested.property('runtime.uptimeSeconds', null)
43+
expect(stderr).to.equal('')
44+
})
45+
3446
it('prints detected I2P hostnames as JSON', async () => {
3547
sinon.stub(fs, 'existsSync').callsFake((target) => String(target).endsWith('nostream.dat'))
3648
sinon
3749
.stub(processUtils, 'runCommandWithOutput')
3850
.onFirstCall()
39-
.resolves({ code: 1, stdout: '', stderr: '' })
51+
.resolves({ ok: true, code: 1, stdout: '', stderr: '' })
4052
.onSecondCall()
4153
.resolves({
54+
ok: true,
4255
code: 0,
4356
stdout: 'alphaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.b32.i2p\n',
4457
stderr: 'betabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.b32.i2p\n',
@@ -58,7 +71,7 @@ describe('runInfo', () => {
5871

5972
it('prints a JSON error when I2P keys are missing', async () => {
6073
sinon.stub(fs, 'existsSync').returns(false)
61-
sinon.stub(processUtils, 'runCommandWithOutput').resolves({ code: 1, stdout: '', stderr: '' })
74+
sinon.stub(processUtils, 'runCommandWithOutput').resolves({ ok: true, code: 1, stdout: '', stderr: '' })
6275

6376
const code = await infoCommand.runInfo({ i2pHostname: true, json: true })
6477

@@ -77,9 +90,9 @@ describe('runInfo', () => {
7790
sinon
7891
.stub(processUtils, 'runCommandWithOutput')
7992
.onFirstCall()
80-
.resolves({ code: 1, stdout: '', stderr: '' })
93+
.resolves({ ok: true, code: 1, stdout: '', stderr: '' })
8194
.onSecondCall()
82-
.resolves({ code: 0, stdout: '', stderr: '' })
95+
.resolves({ ok: true, code: 0, stdout: '', stderr: '' })
8396

8497
const code = await infoCommand.runInfo({ i2pHostname: true, json: true })
8598

@@ -101,9 +114,9 @@ describe('runInfo', () => {
101114
sinon
102115
.stub(processUtils, 'runCommandWithOutput')
103116
.onFirstCall()
104-
.resolves({ code: 1, stdout: '', stderr: '' })
117+
.resolves({ ok: true, code: 1, stdout: '', stderr: '' })
105118
.onSecondCall()
106-
.resolves({ code: 0, stdout: '', stderr: '' })
119+
.resolves({ ok: true, code: 0, stdout: '', stderr: '' })
107120

108121
const code = await infoCommand.runInfo({ i2pHostname: true })
109122

test/unit/cli/run-command.spec.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
const { expect } = require('chai')
2+
3+
const { runCommandWithOutput } = require('../../../dist/src/cli/utils/process.js')
4+
5+
describe('runCommandWithOutput', () => {
6+
it('resolves ok:true with captured stdout, stderr and exit code 0', async () => {
7+
const result = await runCommandWithOutput('sh', ['-c', 'echo out; echo err >&2'])
8+
9+
expect(result).to.deep.equal({ ok: true, code: 0, stdout: 'out\n', stderr: 'err\n' })
10+
})
11+
12+
it('resolves ok:true with non-zero exit code', async () => {
13+
const result = await runCommandWithOutput('sh', ['-c', 'exit 2'])
14+
15+
expect(result.ok).to.equal(true)
16+
expect(result.code).to.equal(2)
17+
})
18+
19+
it('resolves ok:false reason:not-found when command does not exist (ENOENT)', async () => {
20+
const result = await runCommandWithOutput('__nostream_nonexistent_cmd__', [])
21+
22+
expect(result).to.deep.equal({ ok: false, reason: 'not-found', stdout: '', stderr: '' })
23+
})
24+
25+
it('resolves ok:false reason:timeout when the process exceeds timeoutMs', async () => {
26+
const result = await runCommandWithOutput('sleep', ['10'], { timeoutMs: 100 })
27+
28+
expect(result).to.deep.equal({ ok: false, reason: 'timeout', stdout: '', stderr: '' })
29+
})
30+
31+
it('resolves ok:false reason:signal when the process is killed by a signal', async () => {
32+
const result = await runCommandWithOutput('sh', ['-c', 'kill -9 $$'])
33+
34+
expect(result.ok).to.equal(false)
35+
expect(result.reason).to.equal('signal')
36+
})
37+
38+
it('does not double-settle when ENOENT fires both error and close', async () => {
39+
const result = await runCommandWithOutput('__nostream_nonexistent_cmd__', [])
40+
41+
expect(result.ok).to.equal(false)
42+
expect(result.reason).to.equal('not-found')
43+
})
44+
})

test/unit/cli/update.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ describe('runUpdate', () => {
1515
sinon.stub(stopCommand, 'runStop').resolves(0)
1616
const runStartStub = sinon.stub(startCommand, 'runStart').resolves(0)
1717
sinon.stub(processUtils, 'runCommandWithOutput').resolves({
18+
ok: true,
1819
code: 0,
1920
stdout: 'Saved working directory and index state WIP on main: abc123',
2021
stderr: '',
@@ -38,6 +39,7 @@ describe('runUpdate', () => {
3839
sinon.stub(stopCommand, 'runStop').resolves(0)
3940
const runStartStub = sinon.stub(startCommand, 'runStart').resolves(0)
4041
sinon.stub(processUtils, 'runCommandWithOutput').resolves({
42+
ok: true,
4143
code: 0,
4244
stdout: 'Saved working directory and index state WIP on main: abc123',
4345
stderr: '',

0 commit comments

Comments
 (0)