From f459ec8f85be892a0c7dbde08db9b9798f1ed048 Mon Sep 17 00:00:00 2001 From: Matt Devy Date: Mon, 13 Apr 2026 12:32:12 +0100 Subject: [PATCH] fix: Surface all config loading errors in preAction hook (#97) The preAction hook previously only reported config errors when --config-file or --use-context were explicitly passed, silently discarding failures in the common case. Commands would proceed without config and produce confusing downstream errors with exit 0. Now all config loading failures write to stderr and exit 1. The version command is exempt since it does not require configuration. --- src/cli.ts | 5 ++-- test/cli.test.ts | 60 +++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index f9c1c33d..e7a1a4a9 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -25,7 +25,8 @@ program // Before every sub-command action, load and resolve the config file. // On error, print a structured message and exit -- never let a config failure // silently propagate into the command handler. -program.hook('preAction', async (thisCommand) => { +program.hook('preAction', async (thisCommand, actionCommand) => { + if (actionCommand.name() === 'version') return const { configFile: configPath, useContext: contextName } = thisCommand.opts() const result = await loadConfig({ ...(configPath != null && { configPath }), @@ -33,7 +34,7 @@ program.hook('preAction', async (thisCommand) => { }) if (result.ok) { setResolvedConfig(result.value) - } else if (configPath != null || contextName != null) { + } else { process.stderr.write(`Error: ${result.error.message}\n`) process.exit(1) } diff --git a/test/cli.test.ts b/test/cli.test.ts index c8405dc2..fae71efe 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -5,6 +5,10 @@ import { describe, it } from 'node:test' import assert from 'node:assert/strict' +import { spawn } from 'node:child_process' +import { mkdtemp, rm } from 'node:fs/promises' +import { join } from 'node:path' +import { tmpdir } from 'node:os' import { Command } from 'commander' /** @@ -86,21 +90,55 @@ describe('elastic CLI -- global flags', () => { }) }) +function runCli (args: string[], opts: { cwd?: string, env?: Record } = {}): Promise<{ code: number | null, stdout: string, stderr: string }> { + return new Promise((resolve) => { + const child = spawn(process.execPath, [join(process.cwd(), 'dist', 'cli.js'), ...args], { + cwd: opts.cwd, + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env, ...opts.env } + }) + child.stdin.end('') + let stdout = '', stderr = '' + child.stdout.on('data', (d: Buffer) => { stdout += d }) + child.stderr.on('data', (d: Buffer) => { stderr += d }) + child.on('close', (code: number | null) => resolve({ code, stdout, stderr })) + }) +} + +describe('elastic CLI -- preAction config error handling', () => { + it('exits with error when no config file is found', async () => { + const dir = await mkdtemp(join(tmpdir(), 'elastic-cli-noconfig-')) + try { + const { code, stderr } = await runCli(['es', 'info'], { cwd: dir, env: { HOME: dir, XDG_CONFIG_HOME: dir } }) + assert.equal(code, 1, `expected exit code 1, got ${code}`) + assert.ok(stderr.includes('Error:'), `expected stderr to contain "Error:", got: ${stderr}`) + assert.ok(stderr.includes('No configuration file found'), `expected config error message, got: ${stderr}`) + } finally { + await rm(dir, { recursive: true }) + } + }) + + it('exits with error when --config-file points to a nonexistent file', async () => { + const dir = await mkdtemp(join(tmpdir(), 'elastic-cli-badconfig-')) + try { + const { code, stderr } = await runCli( + ['es', 'info', '--config-file', '/nonexistent/path.yml'], + { cwd: dir, env: { HOME: dir, XDG_CONFIG_HOME: dir } } + ) + assert.equal(code, 1, `expected exit code 1, got ${code}`) + assert.ok(stderr.includes('Error:'), `expected stderr to contain "Error:", got: ${stderr}`) + } finally { + await rm(dir, { recursive: true }) + } + }) +}) + describe('elastic CLI -- config-free commands', () => { it('`elastic version` succeeds without a config file', async () => { - const { execFile } = await import('node:child_process') - const { promisify } = await import('node:util') - const { mkdtemp, rm } = await import('node:fs/promises') - const { join } = await import('node:path') - const { tmpdir } = await import('node:os') - const exec = promisify(execFile) const dir = await mkdtemp(join(tmpdir(), 'elastic-cli-noconfig-')) try { - const { stdout } = await exec( - process.execPath, - [join(process.cwd(), 'dist', 'cli.js'), 'version', '--json'], - { cwd: dir, env: { ...process.env, HOME: dir } } - ) + const { code, stdout } = await runCli(['version', '--json'], { cwd: dir, env: { HOME: dir } }) + assert.equal(code, 0, `expected exit code 0, got ${code}`) const parsed = JSON.parse(stdout) assert.ok('version' in parsed) } finally {