From 084def1501bbd08dc6276f8b7704f6d25d0fd869 Mon Sep 17 00:00:00 2001 From: CKodidela Date: Sat, 2 May 2026 14:15:51 +0000 Subject: [PATCH 1/4] refactor: normalize runCommandWithOutput result type to handle spawn errors --- src/cli/commands/info.ts | 11 +++----- src/cli/commands/update.ts | 4 +++ src/cli/utils/process.ts | 50 ++++++++++++++++++++++++++++-------- test/unit/cli/info.spec.ts | 25 +++++++++++++----- test/unit/cli/update.spec.ts | 2 ++ 5 files changed, 67 insertions(+), 25 deletions(-) diff --git a/src/cli/commands/info.ts b/src/cli/commands/info.ts index 3c9450c2..9b162ff8 100644 --- a/src/cli/commands/info.ts +++ b/src/cli/commands/info.ts @@ -56,13 +56,8 @@ const getEventCount = async (): Promise => { } const getRelayUptimeSeconds = async (): Promise => { - let idResult: { code: number; stdout: string; stderr: string } - try { - idResult = await runCommandWithOutput('docker', ['compose', 'ps', '-q', 'nostream'], { timeoutMs: 1000 }) - } catch { - return null - } - if (idResult.code !== 0) { + const idResult = await runCommandWithOutput('docker', ['compose', 'ps', '-q', 'nostream'], { timeoutMs: 1000 }) + if (!idResult.ok || idResult.code !== 0) { return null } @@ -74,7 +69,7 @@ const getRelayUptimeSeconds = async (): Promise => { const startedAtResult = await runCommandWithOutput('docker', ['inspect', '--format', '{{.State.StartedAt}}', containerId], { timeoutMs: 1000, }) - if (startedAtResult.code !== 0) { + if (!startedAtResult.ok || startedAtResult.code !== 0) { return null } diff --git a/src/cli/commands/update.ts b/src/cli/commands/update.ts index 6fb3026d..0c6859da 100644 --- a/src/cli/commands/update.ts +++ b/src/cli/commands/update.ts @@ -18,6 +18,10 @@ export const runUpdate = async (passthrough: string[]): Promise => { } const stashResult = await runCommandWithOutput('git', ['stash', 'push', '-u', '-m', 'nostream-cli-update']) + if (!stashResult.ok) { + spinner.fail(stashResult.ok === false && stashResult.reason === 'not-found' ? 'Update failed: git is not installed' : 'Update failed while stashing local changes') + return 1 + } if (stashResult.code !== 0) { spinner.fail('Update failed while stashing local changes') return stashResult.code diff --git a/src/cli/utils/process.ts b/src/cli/utils/process.ts index a574de08..c0d49d0c 100644 --- a/src/cli/utils/process.ts +++ b/src/cli/utils/process.ts @@ -7,6 +7,10 @@ export type RunOptions = { timeoutMs?: number } +export type CommandResult = + | { ok: true; code: number; stdout: string; stderr: string } + | { ok: false; reason: 'not-found' | 'permission-denied' | 'spawn-error' | 'timeout' | 'signal'; stdout: string; stderr: string } + export const runCommand = (command: string, args: string[], options: RunOptions = {}): Promise => { return new Promise((resolve, reject) => { const child = spawn(command, args, { @@ -38,10 +42,19 @@ export const runCommandWithOutput = ( command: string, args: string[], options: RunOptions = {}, -): Promise<{ code: number; stdout: string; stderr: string }> => { - return new Promise((resolve, reject) => { +): Promise => { + return new Promise((resolve) => { let stdout = '' let stderr = '' + let timedOut = false + let settled = false + + const settle = (result: CommandResult) => { + if (!settled) { + settled = true + resolve(result) + } + } const child = spawn(command, args, { cwd: options.cwd, @@ -53,6 +66,7 @@ export const runCommandWithOutput = ( const timer = typeof options.timeoutMs === 'number' ? setTimeout(() => { + timedOut = true child.kill('SIGTERM') }, options.timeoutMs) : undefined @@ -65,17 +79,31 @@ export const runCommandWithOutput = ( stderr += chunk.toString() }) - child.on('error', reject) - child.on('close', (code) => { - if (timer) { - clearTimeout(timer) + child.on('error', (err: NodeJS.ErrnoException) => { + if (timer) clearTimeout(timer) + if (err.code === 'ENOENT') { + settle({ ok: false, reason: 'not-found', stdout, stderr }) + } else if (err.code === 'EACCES') { + settle({ ok: false, reason: 'permission-denied', stdout, stderr }) + } else { + settle({ ok: false, reason: 'spawn-error', stdout, stderr }) + } + }) + + child.on('close', (code, signal) => { + if (timer) clearTimeout(timer) + + if (timedOut) { + settle({ ok: false, reason: 'timeout', stdout, stderr }) + return + } + + if (signal !== null && code === null) { + settle({ ok: false, reason: 'signal', stdout, stderr }) + return } - resolve({ - code: code ?? 1, - stdout, - stderr, - }) + settle({ ok: true, code: code ?? 1, stdout, stderr }) }) }) } diff --git a/test/unit/cli/info.spec.ts b/test/unit/cli/info.spec.ts index ffe0bd83..7bf9b892 100644 --- a/test/unit/cli/info.spec.ts +++ b/test/unit/cli/info.spec.ts @@ -31,14 +31,27 @@ describe('runInfo', () => { sinon.restore() }) + it('outputs valid JSON when docker is not installed (ENOENT)', async () => { + sinon.stub(fs, 'existsSync').returns(false) + sinon.stub(processUtils, 'runCommandWithOutput').resolves({ ok: false, reason: 'not-found', stdout: '', stderr: '' }) + + const code = await infoCommand.runInfo({ json: true }) + + expect(code).to.equal(0) + const parsed = JSON.parse(stdout) + expect(parsed).to.have.nested.property('runtime.uptimeSeconds', null) + expect(stderr).to.equal('') + }) + it('prints detected I2P hostnames as JSON', async () => { sinon.stub(fs, 'existsSync').callsFake((target) => String(target).endsWith('nostream.dat')) sinon .stub(processUtils, 'runCommandWithOutput') .onFirstCall() - .resolves({ code: 1, stdout: '', stderr: '' }) + .resolves({ ok: true, code: 1, stdout: '', stderr: '' }) .onSecondCall() .resolves({ + ok: true, code: 0, stdout: 'alphaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.b32.i2p\n', stderr: 'betabbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.b32.i2p\n', @@ -58,7 +71,7 @@ describe('runInfo', () => { it('prints a JSON error when I2P keys are missing', async () => { sinon.stub(fs, 'existsSync').returns(false) - sinon.stub(processUtils, 'runCommandWithOutput').resolves({ code: 1, stdout: '', stderr: '' }) + sinon.stub(processUtils, 'runCommandWithOutput').resolves({ ok: true, code: 1, stdout: '', stderr: '' }) const code = await infoCommand.runInfo({ i2pHostname: true, json: true }) @@ -77,9 +90,9 @@ describe('runInfo', () => { sinon .stub(processUtils, 'runCommandWithOutput') .onFirstCall() - .resolves({ code: 1, stdout: '', stderr: '' }) + .resolves({ ok: true, code: 1, stdout: '', stderr: '' }) .onSecondCall() - .resolves({ code: 0, stdout: '', stderr: '' }) + .resolves({ ok: true, code: 0, stdout: '', stderr: '' }) const code = await infoCommand.runInfo({ i2pHostname: true, json: true }) @@ -101,9 +114,9 @@ describe('runInfo', () => { sinon .stub(processUtils, 'runCommandWithOutput') .onFirstCall() - .resolves({ code: 1, stdout: '', stderr: '' }) + .resolves({ ok: true, code: 1, stdout: '', stderr: '' }) .onSecondCall() - .resolves({ code: 0, stdout: '', stderr: '' }) + .resolves({ ok: true, code: 0, stdout: '', stderr: '' }) const code = await infoCommand.runInfo({ i2pHostname: true }) diff --git a/test/unit/cli/update.spec.ts b/test/unit/cli/update.spec.ts index 6c5d9308..7f216b71 100644 --- a/test/unit/cli/update.spec.ts +++ b/test/unit/cli/update.spec.ts @@ -15,6 +15,7 @@ describe('runUpdate', () => { sinon.stub(stopCommand, 'runStop').resolves(0) const runStartStub = sinon.stub(startCommand, 'runStart').resolves(0) sinon.stub(processUtils, 'runCommandWithOutput').resolves({ + ok: true, code: 0, stdout: 'Saved working directory and index state WIP on main: abc123', stderr: '', @@ -38,6 +39,7 @@ describe('runUpdate', () => { sinon.stub(stopCommand, 'runStop').resolves(0) const runStartStub = sinon.stub(startCommand, 'runStart').resolves(0) sinon.stub(processUtils, 'runCommandWithOutput').resolves({ + ok: true, code: 0, stdout: 'Saved working directory and index state WIP on main: abc123', stderr: '', From 3bfc101ba18b66113fcb95e1970282bbb4bcf47f Mon Sep 17 00:00:00 2001 From: CKodidela Date: Sat, 2 May 2026 14:18:47 +0000 Subject: [PATCH 2/4] fix: add braces to single-line if statements in process.ts --- src/cli/utils/process.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/utils/process.ts b/src/cli/utils/process.ts index c0d49d0c..51185144 100644 --- a/src/cli/utils/process.ts +++ b/src/cli/utils/process.ts @@ -80,7 +80,7 @@ export const runCommandWithOutput = ( }) child.on('error', (err: NodeJS.ErrnoException) => { - if (timer) clearTimeout(timer) + if (timer) { clearTimeout(timer) } if (err.code === 'ENOENT') { settle({ ok: false, reason: 'not-found', stdout, stderr }) } else if (err.code === 'EACCES') { @@ -91,7 +91,7 @@ export const runCommandWithOutput = ( }) child.on('close', (code, signal) => { - if (timer) clearTimeout(timer) + if (timer) { clearTimeout(timer) } if (timedOut) { settle({ ok: false, reason: 'timeout', stdout, stderr }) From 6e5ce6087d54de0756dc15bb6314040a8bdb275e Mon Sep 17 00:00:00 2001 From: CKodidela Date: Sat, 2 May 2026 14:23:15 +0000 Subject: [PATCH 3/4] chore: add changeset for runCommandWithOutput normalization --- .changeset/normalize-run-command-with-output.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/normalize-run-command-with-output.md diff --git a/.changeset/normalize-run-command-with-output.md b/.changeset/normalize-run-command-with-output.md new file mode 100644 index 00000000..d086daa9 --- /dev/null +++ b/.changeset/normalize-run-command-with-output.md @@ -0,0 +1,5 @@ +--- +"nostream": patch +--- + +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. From bbf199a9ef213e0b5a7217d586a554b11fd4e816 Mon Sep 17 00:00:00 2001 From: CKodidela Date: Sat, 2 May 2026 14:41:04 +0000 Subject: [PATCH 4/4] test: add unit tests for runCommandWithOutput covering all result paths --- test/unit/cli/run-command.spec.ts | 44 +++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 test/unit/cli/run-command.spec.ts diff --git a/test/unit/cli/run-command.spec.ts b/test/unit/cli/run-command.spec.ts new file mode 100644 index 00000000..98e6d190 --- /dev/null +++ b/test/unit/cli/run-command.spec.ts @@ -0,0 +1,44 @@ +const { expect } = require('chai') + +const { runCommandWithOutput } = require('../../../dist/src/cli/utils/process.js') + +describe('runCommandWithOutput', () => { + it('resolves ok:true with captured stdout, stderr and exit code 0', async () => { + const result = await runCommandWithOutput('sh', ['-c', 'echo out; echo err >&2']) + + expect(result).to.deep.equal({ ok: true, code: 0, stdout: 'out\n', stderr: 'err\n' }) + }) + + it('resolves ok:true with non-zero exit code', async () => { + const result = await runCommandWithOutput('sh', ['-c', 'exit 2']) + + expect(result.ok).to.equal(true) + expect(result.code).to.equal(2) + }) + + it('resolves ok:false reason:not-found when command does not exist (ENOENT)', async () => { + const result = await runCommandWithOutput('__nostream_nonexistent_cmd__', []) + + expect(result).to.deep.equal({ ok: false, reason: 'not-found', stdout: '', stderr: '' }) + }) + + it('resolves ok:false reason:timeout when the process exceeds timeoutMs', async () => { + const result = await runCommandWithOutput('sleep', ['10'], { timeoutMs: 100 }) + + expect(result).to.deep.equal({ ok: false, reason: 'timeout', stdout: '', stderr: '' }) + }) + + it('resolves ok:false reason:signal when the process is killed by a signal', async () => { + const result = await runCommandWithOutput('sh', ['-c', 'kill -9 $$']) + + expect(result.ok).to.equal(false) + expect(result.reason).to.equal('signal') + }) + + it('does not double-settle when ENOENT fires both error and close', async () => { + const result = await runCommandWithOutput('__nostream_nonexistent_cmd__', []) + + expect(result.ok).to.equal(false) + expect(result.reason).to.equal('not-found') + }) +})