shell: exit non-zero when a piped command fails (#11117)

* shell: non-interactive mode exits non-zero when a command fails

A failed command in a piped weed shell run printed 'error: ...' but the process
still exited 0, so a CronJob wrapping e.g.

  echo 's3.lifecycle.run-shard -shards 0-15' | weed shell -master=...

reported green while the run aborted partway (shards N+1..15 unwalked). An
unknown command likewise exited 0.

RunShell now returns the last command failure from the non-interactive stdin
path (unknown commands included), and the shell command exits 2 on it.
Interactive sessions are unchanged: errors are shown to the operator and the
session continues, exiting 0 as before.

* shell: route the piped-failure exit through main's shutdown path

Review follow-up: os.Exit(2) inside the shell command skipped main's shutdown
work. The command now records the status (SetCommandExitStatus) and returns
normally; main applies it via setExitStatus before exit(). exit() itself now
flushes sentry before os.Exit -- main's deferred sentry.Flush never ran on this
path (os.Exit skips defers), so the existing 'flush buffered events before the
program terminates' intent only worked for the autocomplete early-return.
Exit status 2 on a failed piped run is preserved (verified: piped success
exits 0, piped failing command exits 2).

* shell: test the registered-command failure path

Review follow-up: the error-propagation test only covered unknown commands.
A fake registered command now drives processEachCmd's real dispatch path:
a failing Do surfaces its exact error (errors.Is) and a succeeding one
returns nil. The non-interactive exit status itself is main-level plumbing,
verified end to end against the reproduction (piped failure exits 2).

* shell: trim the comments added with the exit status

Keep the non-obvious why -- why a piped run has to fail its wrapper, why the
status is recorded instead of os.Exit'ed -- and drop the narration.

Claude-Session: https://claude.ai/code/session_018DWwctzD4T2DmnczPRM47t

* shell: fail a piped run with the status weed already uses for that

weed.go spends 1 on a command that failed and 2 on a usage or syntax error, and
runShell returns true precisely so the usage dump is skipped. Exiting 2 there
told a wrapper the command line was wrong.

Claude-Session: https://claude.ai/code/session_018DWwctzD4T2DmnczPRM47t

---------

Co-authored-by: Carlos Leyva <carlos.leyva@idener.es>

weed/command/command.go

@@ -97,3 +97,17 @@ func (c *Command) Usage() {
 func (c *Command) Runnable() bool {
 	return c.Run != nil
 }
+
+// Recorded, not os.Exit'ed, so the failure still travels through main's
+// shutdown path. The highest requested status wins.
+var commandExitStatus int
+
+func SetCommandExitStatus(n int) {
+	if n > commandExitStatus {
+		commandExitStatus = n
+	}
+}
+
+func CommandExitStatus() int {
+	return commandExitStatus
+}

weed/command/shell.go

@@ -68,7 +68,11 @@ func runShell(command *Command, args []string) bool {
 		fmt.Fprintf(os.Stderr, "master: %s filer: %s\n", *shellOptions.Masters, shellOptions.FilerAddress)
 	}
 
-	shell.RunShell(shellOptions)
+	if err := shell.RunShell(shellOptions); err != nil {
+		// The command already printed the error; a piped run has to fail the
+		// script wrapping it too.
+		SetCommandExitStatus(1)
+	}
 
 	return true
 

weed/shell/shell_liner.go

@@ -24,7 +24,9 @@ import (
 
 var historyPath = path.Join(os.TempDir(), "weed-shell")
 
-func RunShell(options ShellOptions) {
+// Piped stdin returns the last command failure; an interactive session shows
+// the error to the operator and keeps going.
+func RunShell(options ShellOptions) error {
 	slices.SortFunc(Commands, func(a, b command) int {
 		return strings.Compare(a.Name(), b.Name())
 	})
@@ -94,7 +96,7 @@ func RunShell(options ShellOptions) {
 				if err != io.EOF {
 					fmt.Fprintf(os.Stderr, "%v\n", err)
 				}
-				return
+				return nil
 			}
 
 			if strings.TrimSpace(cmd) != "" {
@@ -102,32 +104,39 @@ func RunShell(options ShellOptions) {
 			}
 
 			for _, c := range util.StringSplit(cmd, ";") {
-				if processEachCmd(c, commandEnv) {
-					return
+				if exit, _ := processEachCmd(c, commandEnv); exit {
+					return nil
 				}
 			}
 		}
 	} else {
+		var lastErr error
 		scanner := bufio.NewScanner(os.Stdin)
 		for scanner.Scan() {
 			cmd := scanner.Text()
 			for _, c := range util.StringSplit(cmd, ";") {
-				if processEachCmd(c, commandEnv) {
-					return
+				exit, err := processEachCmd(c, commandEnv)
+				if err != nil {
+					lastErr = err
+				}
+				if exit {
+					return lastErr
 				}
 			}
 		}
 		if err := scanner.Err(); err != nil {
 			fmt.Fprintf(os.Stderr, "error reading stdin: %v\n", err)
+			lastErr = err
 		}
+		return lastErr
 	}
 }
 
-func processEachCmd(cmd string, commandEnv *CommandEnv) bool {
+func processEachCmd(cmd string, commandEnv *CommandEnv) (exit bool, cmdErr error) {
 	cmds := splitCommandLine(cmd)
 
 	if len(cmds) == 0 {
-		return false
+		return false, nil
 	} else {
 
 		args := cmds[1:]
@@ -136,7 +145,7 @@ func processEachCmd(cmd string, commandEnv *CommandEnv) bool {
 		if cmd == "help" || cmd == "?" {
 			printHelp(cmds)
 		} else if cmd == "exit" || cmd == "quit" {
-			return true
+			return true, nil
 		} else {
 			foundCommand := false
 			for _, c := range Commands {
@@ -149,17 +158,19 @@ func processEachCmd(cmd string, commandEnv *CommandEnv) bool {
 					commandEnv.SetNoLock(false)
 					if err := c.Do(args, commandEnv, os.Stdout); err != nil {
 						fmt.Fprintf(os.Stderr, "error: %v\n", err)
+						cmdErr = err
 					}
 					foundCommand = true
 				}
 			}
 			if !foundCommand {
 				fmt.Fprintf(os.Stderr, "unknown command: %v\n", cmd)
+				cmdErr = fmt.Errorf("unknown command: %v", cmd)
 			}
 		}
 
 	}
-	return false
+	return false, cmdErr
 }
 
 func splitCommandLine(line string) []string {

weed/shell/shell_liner_test.go

@@ -0,0 +1,66 @@
+package shell
+
+import (
+	"errors"
+	"io"
+	"strings"
+	"testing"
+)
+
+type fakeCommand struct {
+	name string
+	err  error
+}
+
+func (c *fakeCommand) Name() string                              { return c.name }
+func (c *fakeCommand) Help() string                              { return "test helper" }
+func (c *fakeCommand) HasTag(CommandTag) bool                    { return false }
+func (c *fakeCommand) Do([]string, *CommandEnv, io.Writer) error { return c.err }
+
+func TestProcessEachCmdReturnsErrors(t *testing.T) {
+
+	exit, err := processEachCmd("definitely.not.a.command", nil)
+	if exit {
+		t.Errorf("unknown command must not request exit")
+	}
+	if err == nil {
+		t.Errorf("unknown command must return an error")
+	}
+	if err != nil && !strings.Contains(err.Error(), "unknown command") {
+		t.Errorf("unexpected error for unknown command: %v", err)
+	}
+
+	exit, err = processEachCmd("exit", nil)
+	if !exit {
+		t.Errorf("exit must request exit")
+	}
+	if err != nil {
+		t.Errorf("exit must not return an error, got: %v", err)
+	}
+
+	exit, err = processEachCmd("   ", nil)
+	if exit || err != nil {
+		t.Errorf("blank input must be a no-op, got exit=%v err=%v", exit, err)
+	}
+}
+
+func TestProcessEachCmdPreservesRegisteredCommandError(t *testing.T) {
+	doErr := errors.New("daily_run: shard=3: recovery walk failed")
+	failing := &fakeCommand{name: "test.failing.command", err: doErr}
+	succeeding := &fakeCommand{name: "test.succeeding.command"}
+	Commands = append(Commands, failing, succeeding)
+	t.Cleanup(func() { Commands = Commands[:len(Commands)-2] })
+
+	exit, err := processEachCmd("test.failing.command -some -args", nil)
+	if exit {
+		t.Errorf("a failing command must not request exit")
+	}
+	if !errors.Is(err, doErr) {
+		t.Errorf("the command's own error must be preserved, got: %v", err)
+	}
+
+	exit, err = processEachCmd("test.succeeding.command", nil)
+	if exit || err != nil {
+		t.Errorf("a succeeding command must return no error, got exit=%v err=%v", exit, err)
+	}
+}

weed/weed.go

@@ -102,6 +102,8 @@ func main() {
 				// Command execution failed - general error
 				setExitStatus(1)
 			}
+			// A command can also record a failure without returning false.
+			setExitStatus(command.CommandExitStatus())
 			exit()
 			return
 		}
@@ -200,5 +202,7 @@ func exit() {
 	for _, f := range atexitFuncs {
 		f()
 	}
+	// os.Exit below skips main's deferred flush.
+	sentry.Flush(2 * time.Second)
 	os.Exit(exitStatus)
 }