diff --git a/weed/command/command.go b/weed/command/command.go index daa3823a1..bc40e4c29 100644 --- a/weed/command/command.go +++ b/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 +} diff --git a/weed/command/shell.go b/weed/command/shell.go index 2645f4932..0162d1501 100644 --- a/weed/command/shell.go +++ b/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 diff --git a/weed/shell/shell_liner.go b/weed/shell/shell_liner.go index 9ea8ca873..c3e0d86d7 100644 --- a/weed/shell/shell_liner.go +++ b/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 { diff --git a/weed/shell/shell_liner_test.go b/weed/shell/shell_liner_test.go new file mode 100644 index 000000000..6d9bda9e7 --- /dev/null +++ b/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) + } +} diff --git a/weed/weed.go b/weed/weed.go index f83777bf5..17f6a44e1 100644 --- a/weed/weed.go +++ b/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) }