fix(cli): return correct exit codes and report errors on stderr

cmd/base.go

@@ -1,7 +1,7 @@
 package cmd
 
 import (
-	"os"
+	"fmt"
 
 	"github.com/fatih/color"
 	"github.com/flywp/server-cli/internal/docker"
@@ -18,50 +18,51 @@ var baseCmd = &cobra.Command{
 var baseStartCmd = &cobra.Command{
 	Use:   "start",
 	Short: "Start base services",
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		if err := docker.RunCompose(baseCompose, "up", "-d"); err != nil {
-			color.Red("Error starting base services: %v", err)
-			os.Exit(1)
-			return
+			return fmt.Errorf("starting base services: %w", err)
 		}
 
 		if err := docker.RunCompose(baseCompose, "ps"); err != nil {
-			color.Red("Error checking status of base services: %v", err)
+			return fmt.Errorf("checking status of base services: %w", err)
 		}
 
 		color.Green("Base services started successfully")
+		return nil
 	},
 }
 
 var baseStopCmd = &cobra.Command{
 	Use:   "stop",
 	Short: "Stop base services",
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		if err := docker.RunCompose(baseCompose, "down"); err != nil {
-			color.Red("Error stopping base services:", err)
+			return fmt.Errorf("stopping base services: %w", err)
 		}
 
 		color.Green("Base services stopped successfully")
+		return nil
 	},
 }
 
 var baseRestartCmd = &cobra.Command{
 	Use:   "restart",
 	Short: "Restart base services",
-	Run: func(cmd *cobra.Command, args []string) {
+	RunE: func(cmd *cobra.Command, args []string) error {
 		if err := docker.RunCompose(baseCompose, "down"); err != nil {
-			color.Red("Error stopping base services:", err)
+			return fmt.Errorf("stopping base services: %w", err)
 		}
 
 		if err := docker.RunCompose(baseCompose, "up", "-d"); err != nil {
-			color.Red("Error starting base services:", err)
+			return fmt.Errorf("starting base services: %w", err)
 		}
 
 		if err := docker.RunCompose(baseCompose, "ps"); err != nil {
-			color.Red("Error checking status of base services:", err)
+			return fmt.Errorf("checking status of base services: %w", err)
 		}
 
 		color.Green("Base services restarted successfully")
+		return nil
 	},
 }
 

cmd/global.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -84,90 +85,68 @@ var sitesCmd = &cobra.Command{
 var sitesStartCmd = &cobra.Command{
 	Use:   "start",
 	Short: "Start all sites",
-	Run: func(cmd *cobra.Command, args []string) {
-		startAllSites()
+	RunE: func(cmd *cobra.Command, args []string) error {
+		return forEachSite("Starting", "up", "-d")
 	},
 }
 
 var sitesStopCmd = &cobra.Command{
 	Use:   "stop",
 	Short: "Stop all sites",
-	Run: func(cmd *cobra.Command, args []string) {
-		stopAllSites()
+	RunE: func(cmd *cobra.Command, args []string) error {
+		return forEachSite("Stopping", "down")
 	},
 }
 
 var restartSitesCmd = &cobra.Command{
 	Use:   "restart",
 	Short: "Restart all sites",
-	Run: func(cmd *cobra.Command, args []string) {
-		stopAllSites()
-		startAllSites()
+	RunE: func(cmd *cobra.Command, args []string) error {
+		// Start the sites even if some of them failed to stop.
+		stopErr := forEachSite("Stopping", "down")
+		startErr := forEachSite("Starting", "up", "-d")
+		return errors.Join(stopErr, startErr)
 	},
 }
 
-func startAllSites() {
-	sitesDir := "/home/fly"
-	foundSite := false
+// sitesDir holds one directory per site, each with its own docker-compose.yml.
+var sitesDir = "/home/fly"
 
+// forEachSite runs docker compose with args for every site in sitesDir.
+// It continues after a failed site and returns all failures together.
+func forEachSite(verb string, args ...string) error {
 	entries, err := os.ReadDir(sitesDir)
 	if err != nil {
-		color.Red("Error reading directory %s: %v\n", sitesDir, err)
-		return
+		return fmt.Errorf("reading sites directory: %w", err)
 	}
 
+	var errs []error
+	foundSite := false
 	for _, entry := range entries {
-		if entry.IsDir() {
-			// Skip hidden directories
-			if strings.HasPrefix(entry.Name(), ".") {
-				continue
-			}
-
-			path := filepath.Join(sitesDir, entry.Name())
-			composePath := filepath.Join(path, "docker-compose.yml")
-			if _, err := os.Stat(composePath); err == nil {
-				color.Yellow("Starting site in %s\n", filepath.Base(path))
-				docker.RunCompose(composePath, "up", "-d")
-				foundSite = true
-			}
+		// Skip files and hidden directories
+		if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".") {
+			continue
 		}
-	}
-
-	if !foundSite {
-		fmt.Println("No sites found to start.")
-	}
-}
-
-func stopAllSites() {
-	sitesDir := "/home/fly"
-	foundSite := false
 
-	entries, err := os.ReadDir(sitesDir)
-	if err != nil {
-		color.Red("Error reading directory %s: %v\n", sitesDir, err)
-		return
-	}
+		composePath := filepath.Join(sitesDir, entry.Name(), "docker-compose.yml")
+		if _, err := os.Stat(composePath); err != nil {
+			continue
+		}
 
-	for _, entry := range entries {
-		if entry.IsDir() {
-			// Skip hidden directories
-			if strings.HasPrefix(entry.Name(), ".") {
-				continue
-			}
-
-			path := filepath.Join(sitesDir, entry.Name())
-			composePath := filepath.Join(path, "docker-compose.yml")
-			if _, err := os.Stat(composePath); err == nil {
-				color.Yellow("Stopping site in %s\n", filepath.Base(path))
-				docker.RunCompose(composePath, "down")
-				foundSite = true
-			}
+		foundSite = true
+		color.Yellow("%s site in %s", verb, entry.Name())
+		if err := docker.RunCompose(composePath, args...); err != nil {
+			// %v, not %w: a failed child process must not hide this summary.
+			// The verb tells restart's stop failures from its start failures.
+			errs = append(errs, fmt.Errorf("%s %s: %v", strings.ToLower(verb), entry.Name(), err))
 		}
 	}
 
 	if !foundSite {
-		fmt.Println("No sites found to stop.")
+		fmt.Println("No sites found.")
 	}
+
+	return errors.Join(errs...)
 }
 
 func init() {

cmd/global_test.go

@@ -0,0 +1,84 @@
+package cmd
+
+import (
+	"errors"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/flywp/server-cli/internal/testutil"
+)
+
+func TestForEachSiteContinuesAfterFailure(t *testing.T) {
+	fake := testutil.NewFakeDocker(t)
+	fake.Install(t)
+	t.Setenv(testutil.EnvExit, "3")
+
+	root := testutil.TempDir(t)
+	testutil.WriteSite(t, filepath.Join(root, "a.example.com"), "php")
+	testutil.WriteSite(t, filepath.Join(root, "b.example.com"), "php")
+	testutil.WriteSite(t, filepath.Join(root, ".fly"), "mysql") // hidden: skipped
+
+	old := sitesDir
+	sitesDir = root
+	t.Cleanup(func() { sitesDir = old })
+
+	err := forEachSite("Starting", "up", "-d")
+	if err == nil {
+		t.Fatal("forEachSite() = nil, want an error for the failed sites")
+	}
+
+	// The summary must be printed, so it must not unwrap to the child's exit status.
+	var exitErr *exec.ExitError
+	if errors.As(err, &exitErr) {
+		t.Errorf("forEachSite() error unwraps to *exec.ExitError, want a plain summary: %v", err)
+	}
+	for _, site := range []string{"a.example.com", "b.example.com"} {
+		if !strings.Contains(err.Error(), "starting "+site) {
+			t.Errorf("error %q does not name the phase and site %s", err, site)
+		}
+	}
+
+	if calls := fake.Calls(t); len(calls) != 2 {
+		t.Errorf("docker calls = %q, want one call per site", calls)
+	}
+}
+
+func TestSitesRestartNamesThePhaseOfEachFailure(t *testing.T) {
+	fake := testutil.NewFakeDocker(t)
+	fake.Install(t)
+	t.Setenv(testutil.EnvExit, "3")
+
+	root := testutil.TempDir(t)
+	testutil.WriteSite(t, filepath.Join(root, "broken.example.com"), "php")
+
+	old := sitesDir
+	sitesDir = root
+	t.Cleanup(func() { sitesDir = old })
+
+	// The site fails to stop and then fails to start: two different failures.
+	err := restartSitesCmd.RunE(restartSitesCmd, nil)
+	if err == nil {
+		t.Fatal("sites restart = nil, want an error for the failed site")
+	}
+
+	got := strings.Split(err.Error(), "\n")
+	want := []string{
+		"stopping broken.example.com: exit status 3",
+		"starting broken.example.com: exit status 3",
+	}
+	if strings.Join(got, "\n") != strings.Join(want, "\n") {
+		t.Errorf("error lines = %q, want %q", got, want)
+	}
+}
+
+func TestForEachSiteMissingDirectory(t *testing.T) {
+	old := sitesDir
+	sitesDir = filepath.Join(t.TempDir(), "missing")
+	t.Cleanup(func() { sitesDir = old })
+
+	if err := forEachSite("Starting", "up", "-d"); err == nil {
+		t.Fatal("forEachSite() = nil, want an error for a missing sites directory")
+	}
+}

cmd/root.go

@@ -1,16 +1,24 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
+	"io"
 	"os"
+	"os/exec"
 
+	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 )
 
 var rootCmd = &cobra.Command{
 	Use:   "fly",
 	Short: "Fly CLI for managing WordPress sites",
 	Long:  `A CLI tool for managing WordPress sites using Docker and custom commands.`,
+	// Execute reports errors itself, so that errors from child processes
+	// are not printed twice and usage is not printed for runtime errors.
+	SilenceErrors: true,
+	SilenceUsage:  true,
 	PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
 		if cmd.Name() != "update" && os.Geteuid() == 0 {
 			return fmt.Errorf("you should not run this command as root")
@@ -20,22 +28,33 @@ var rootCmd = &cobra.Command{
 	},
 }
 
-// Execute the root command.
+// Execute runs the root command and exits with the resulting status code.
 func Execute() {
-	err := rootCmd.Execute()
-	if err != nil {
-		os.Exit(1)
-	}
+	os.Exit(exitCode(rootCmd.Execute(), os.Stderr))
 }
 
-func init() {
-	// Here you will define your flags and configuration settings.
-	// Cobra supports persistent flags, which, if defined here,
-	// will be global for your application.
+// exitCode reports err on stderr and returns the exit status for it.
+func exitCode(err error, stderr io.Writer) int {
+	if err == nil {
+		return 0
+	}
+
+	// A child process (docker compose, wp-cli) has already reported its own
+	// error, so pass its exit status through without printing anything.
+	var exitErr *exec.ExitError
+	if errors.As(err, &exitErr) {
+		if code := exitErr.ExitCode(); code > 0 {
+			return code
+		}
+		return 1
+	}
 
-	// rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is $HOME/.server-cli.yaml)")
+	_, _ = color.New(color.FgRed).Fprintf(stderr, "Error: %v\n", err)
+	return 1
+}
 
-	// Cobra also supports local flags, which will only run
-	// when this action is called directly.
-	// rootCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle")
+func init() {
+	rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error {
+		return fmt.Errorf("%w\nRun '%s --help' for usage", err, cmd.CommandPath())
+	})
 }

cmd/root_test.go

@@ -0,0 +1,38 @@
+package cmd
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"os/exec"
+	"strings"
+	"testing"
+)
+
+func TestExitCode(t *testing.T) {
+	childErr := exec.Command("sh", "-c", "exit 7").Run()
+
+	tests := []struct {
+		name       string
+		err        error
+		wantCode   int
+		wantStderr string
+	}{
+		{name: "success", err: nil, wantCode: 0},
+		{name: "child exit status passes through silently", err: childErr, wantCode: 7},
+		{name: "wrapped child exit status", err: fmt.Errorf("starting site: %w", childErr), wantCode: 7},
+		{name: "other error is printed", err: errors.New("boom"), wantCode: 1, wantStderr: "Error: boom\n"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var stderr bytes.Buffer
+			if got := exitCode(tt.err, &stderr); got != tt.wantCode {
+				t.Errorf("exitCode() = %d, want %d", got, tt.wantCode)
+			}
+			if got := stderr.String(); !strings.Contains(got, tt.wantStderr) || (tt.wantStderr == "" && got != "") {
+				t.Errorf("stderr = %q, want %q", got, tt.wantStderr)
+			}
+		})
+	}
+}