From d5ec5b1aff47a77668d9602f9ed4f1435d1c74fc Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Wed, 28 Oct 2026 09:59:41 +0100 Subject: [PATCH] fix: print CLI errors once, without a stack trace, and honor exitCode citty's runMain swallows every error itself: for a non-citty error it prints the error with a stack trace, prints the message again, and always exits 1. So every CliError printed twice with a stack, its exitCode was ignored, and the .catch after runMain never ran. runCli hands --help/--version to runMain and runs everything else through runCommand: CliError -> one "Error: ..." line on stderr + its exitCode; citty usage errors (missing arg, unknown command) are replayed through runMain for its usage rendering (they occur before any command body runs, so no side effects are replayed); unexpected errors -> one line, exit 1. Co-Authored-By: Claude Sonnet 5.5 --- src/index.ts | 14 ++----- src/lib/run.ts | 47 ++++++++++++++++++++++++ tests/lib/run.test.ts | 85 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 11 deletions(-) create mode 100644 src/lib/run.ts create mode 100644 tests/lib/run.test.ts diff --git a/src/index.ts b/src/index.ts index a2695e3..9187483 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,6 @@ -import { defineCommand, runMain } from "citty"; -import { CliError } from "./lib/errors.js"; +import { defineCommand } from "citty"; +import { runCli } from "./lib/run.js"; import { VERSION } from "./lib/client.js"; -import { stderr } from "./lib/output.js"; const main = defineCommand({ meta: { @@ -24,11 +23,4 @@ const main = defineCommand({ }, }); -runMain(main).catch((err: unknown) => { - if (err instanceof CliError) { - stderr(`Error: ${err.message}`); - process.exit(err.exitCode); - } - stderr(`Error: ${err instanceof Error ? err.message : String(err)}`); - process.exit(1); -}); +void runCli(main); diff --git a/src/lib/run.ts b/src/lib/run.ts new file mode 100644 index 0000000..406df4b --- /dev/null +++ b/src/lib/run.ts @@ -0,0 +1,47 @@ +import { runCommand, runMain, type CommandDef } from "citty"; +import { CliError } from "./errors.js"; +import { stderr } from "./output.js"; + +/** + * Entry point wrapper around citty. + * + * citty's `runMain` swallows every error itself: for anything that isn't its own + * usage error it prints the error WITH a stack trace, prints the message a second + * time, and always exits 1 — so a `CliError` printed twice with a stack and its + * `exitCode` was ignored (and the `.catch` after `runMain` never ran). We hand + * `--help` / `--version` to `runMain` (it renders those well) and run everything + * else through `runCommand` so we own error output. + */ +export async function runCli( + main: CommandDef, + rawArgs: string[] = process.argv.slice(2), + exit: (code: number) => void = (code) => process.exit(code) +): Promise { + const wantsMeta = + rawArgs.includes("--help") || + rawArgs.includes("-h") || + (rawArgs.length === 1 && rawArgs[0] === "--version"); + if (wantsMeta) { + await runMain(main, { rawArgs }); + return; + } + + try { + await runCommand(main, { rawArgs }); + } catch (err) { + if (err instanceof CliError) { + stderr(`Error: ${err.message}`); + exit(err.exitCode); + return; + } + // citty's own usage errors (missing argument, unknown command) surface before + // any command body runs, so replaying through runMain is side-effect free and + // gets citty's usage rendering. + if (err instanceof Error && err.name === "CLIError") { + await runMain(main, { rawArgs }); + return; + } + stderr(`Error: ${err instanceof Error ? err.message : String(err)}`); + exit(1); + } +} diff --git a/tests/lib/run.test.ts b/tests/lib/run.test.ts new file mode 100644 index 0000000..dc9ddb9 --- /dev/null +++ b/tests/lib/run.test.ts @@ -0,0 +1,85 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { defineCommand } from "citty"; +import { runCli } from "../../src/lib/run.js"; +import { CliError } from "../../src/lib/errors.js"; + +const makeMain = (run: () => unknown) => + defineCommand({ + meta: { name: "t", version: "1.2.3", description: "t" }, + subCommands: { + boom: defineCommand({ meta: { name: "boom" }, run }), + needs: defineCommand({ + meta: { name: "needs" }, + args: { plan: { type: "positional", required: true } }, + run() {}, + }), + }, + }); + +describe("runCli", () => { + let err = ""; + let out = ""; + + beforeEach(() => { + err = ""; + out = ""; + vi.spyOn(process.stderr, "write").mockImplementation((s: any) => { + err += s; + return true; + }); + vi.spyOn(process.stdout, "write").mockImplementation((s: any) => { + out += s; + return true; + }); + }); + afterEach(() => vi.restoreAllMocks()); + + it("prints a CliError once, without a stack trace, and exits with its exitCode", async () => { + const exit = vi.fn(); + await runCli( + makeMain(() => { + throw new CliError("nope", 3); + }), + ["boom"], + exit + ); + expect(err).toBe("Error: nope\n"); + expect(err).not.toContain(" at "); + expect(exit).toHaveBeenCalledOnce(); + expect(exit).toHaveBeenCalledWith(3); + }); + + it("reports an unexpected error as one line and exits 1", async () => { + const exit = vi.fn(); + await runCli( + makeMain(() => { + throw new TypeError("kaput"); + }), + ["boom"], + exit + ); + expect(err).toBe("Error: kaput\n"); + expect(exit).toHaveBeenCalledWith(1); + }); + + it("does not exit on success", async () => { + const exit = vi.fn(); + await runCli(makeMain(() => {}), ["boom"], exit); + expect(exit).not.toHaveBeenCalled(); + expect(err).toBe(""); + }); + + it("does not re-run the command when it fails (no side-effect replay)", async () => { + const run = vi.fn(() => { + throw new CliError("x", 1); + }); + await runCli(makeMain(run), ["boom"], vi.fn()); + expect(run).toHaveBeenCalledTimes(1); + }); + + it("leaves citty's usage rendering for a missing argument", async () => { + const exit = vi.spyOn(process, "exit").mockImplementation((() => undefined) as never); + await runCli(makeMain(() => {}), ["needs"], vi.fn()); + expect(exit).toHaveBeenCalledWith(1); + }); +});