From 0b9c02e9171509189acf712a7776611692a632b5 Mon Sep 17 00:00:00 2001 From: Thanh Lee Date: Wed, 5 Aug 2026 16:15:58 +0000 Subject: [PATCH 1/2] fix(cli): print each error once Cobra printed the "Error: ..." line when a command returned and main printed the same message again, so a single failure was reported twice. Silence cobra's reporting and keep main as the only printer. Main runs after the background update check has finished writing to stderr, so the error stays the last line the user sees. It keeps the "Error: " prefix, which separates a failure from the progress and warning output atb also writes to stderr. Extract run() from main so the exit path can be tested. --- cmd/atb/main.go | 16 +++++++++-- cmd/atb/main_test.go | 67 ++++++++++++++++++++++++++++++++++++++++++++ internal/cli/root.go | 19 ++++++++----- 3 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 cmd/atb/main_test.go diff --git a/cmd/atb/main.go b/cmd/atb/main.go index 664b78e..f1aec81 100644 --- a/cmd/atb/main.go +++ b/cmd/atb/main.go @@ -2,8 +2,11 @@ package main import ( "fmt" + "io" "os" + "github.com/spf13/cobra" + "github.com/allthebacteria/atb-cli/internal/cli" ) @@ -12,7 +15,13 @@ var version = "dev" func main() { cli.RootCmd.Version = version - err := cli.RootCmd.Execute() + os.Exit(run(cli.RootCmd, os.Stderr)) +} + +// run executes root and returns the process exit code, reporting any error on +// stderr. +func run(root *cobra.Command, stderr io.Writer) int { + err := root.Execute() // Let the background update check finish saving state (up to 2s). if cli.WaitForUpdateCheck != nil { @@ -20,7 +29,8 @@ func main() { } if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) + fmt.Fprintln(stderr, "Error:", err) + return 1 } + return 0 } diff --git a/cmd/atb/main_test.go b/cmd/atb/main_test.go new file mode 100644 index 0000000..1474b49 --- /dev/null +++ b/cmd/atb/main_test.go @@ -0,0 +1,67 @@ +package main + +import ( + "bytes" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/allthebacteria/atb-cli/internal/cli" +) + +// failingRoot builds a real command tree and an argument list that always +// fails during flag parsing, so the test does not depend on any data files. +func failingRoot() (*cobra.Command, *bytes.Buffer) { + root := cli.NewRootCmd("test") + root.SetArgs([]string{"query", "--no-such-flag"}) + + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + return root, &out +} + +// Cobra reports the error when Execute returns and main reported it a second +// time, so the same sentence reached the user twice. +func TestRunReportsAnErrorOnce(t *testing.T) { + root, out := failingRoot() + + code := run(root, out) + + if code != 1 { + t.Errorf("exit code: got %d, want 1", code) + } + if got := strings.Count(out.String(), "unknown flag: --no-such-flag"); got != 1 { + t.Errorf("error reported %d time(s), want 1:\n%s", got, out.String()) + } +} + +// atb writes progress and warnings to stderr as well, so the error keeps the +// conventional prefix that separates it from that output. +func TestRunPrefixesTheError(t *testing.T) { + root, out := failingRoot() + + run(root, out) + + line := strings.TrimSpace(out.String()) + if !strings.HasPrefix(line, "Error: ") { + t.Errorf("error line should start with %q, got:\n%s", "Error: ", line) + } +} + +func TestRunReportsNothingOnSuccess(t *testing.T) { + root := cli.NewRootCmd("test") + root.SetArgs([]string{"--help"}) + + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + + if code := run(root, &out); code != 0 { + t.Errorf("exit code: got %d, want 0", code) + } + if strings.Contains(out.String(), "Error:") { + t.Errorf("--help should not report an error, got:\n%s", out.String()) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 517f93f..f16835c 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -26,10 +26,14 @@ var ( ) // RootCmd is the base command for atb. +// +// SilenceErrors leaves reporting to main, which prints the error after the +// background update check has finished writing to stderr. var RootCmd = &cobra.Command{ - Use: "atb", - Short: "Query and download AllTheBacteria genomes", - SilenceUsage: true, + Use: "atb", + Short: "Query and download AllTheBacteria genomes", + SilenceUsage: true, + SilenceErrors: true, } func init() { @@ -69,10 +73,11 @@ func NewRootCmd(version string) *cobra.Command { var localCfgFile, localDataDir string root := &cobra.Command{ - Use: "atb", - Short: "Query and download AllTheBacteria genomes", - SilenceUsage: true, - Version: version, + Use: "atb", + Short: "Query and download AllTheBacteria genomes", + SilenceUsage: true, + SilenceErrors: true, + Version: version, } root.PersistentFlags().StringVar(&localCfgFile, "config", "", "config file (default $HOME/.atb/config.toml)") From 782b0f37ea6d6bf49abad38ee125b9058a0a1abb Mon Sep 17 00:00:00 2001 From: Thanh Lee Date: Wed, 5 Aug 2026 16:15:58 +0000 Subject: [PATCH 2/2] docs(changelog): record the duplicate error fix --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58a9f6c..941416d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ All notable changes to `atb-cli` are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/). +## [Unreleased] + +### Fixed + +- Every error was printed twice. Cobra wrote the `Error: ...` line when a + command returned and `main` wrote the same sentence again, so one failure + looked like two. The error is now reported once, keeping the `Error: ` prefix + that separates it from the progress and warning output `atb` also writes to + stderr. Exit codes are unchanged. + ## [v0.21.0](https://github.com/allthebacteria/atb-cli/releases/tag/v0.21.0) - 2026-08-05 ### Added