diff --git a/cmd/summer/runtime.go b/cmd/summer/runtime.go index 29d550f7..81615696 100644 --- a/cmd/summer/runtime.go +++ b/cmd/summer/runtime.go @@ -90,7 +90,7 @@ func delegateCommand(name, description, hint string) bonfire.Command { long := description + ".\n\nsummer runs bin/ " + name + " in the application directory, building the binary first when it is missing. Every flag and argument after " + name + ", including --help, is passed to the app binary unchanged, and so are the global options (-v, -q, --ansi, --no-ansi, -n, --env). " + - "summer fails when the app binary fails." + "When the app binary fails, summer exits with its exit code." if hint != "" { long += "\n\nCommon flags: " + hint + "." } @@ -141,12 +141,26 @@ func runAppBinary(ctx context.Context, argv []string, out bonfire.Output) error return fmt.Errorf("summer: refusing to recurse into %s", bin) } } - cmd := newAppCmd(ctx, bin, appDir, argv, out) + return runAppCmd(newAppCmd(ctx, bin, appDir, argv, out), argv) +} + +// runAppCmd runs cmd, the app binary started with argv, forwarding SIGINT +// and SIGTERM to it. When the binary exits with a code, the error carries +// that code (bonfire.WithExitCode) and is marked as already reported. +func runAppCmd(cmd *exec.Cmd, argv []string) error { sigs := make(chan os.Signal, 2) signal.Notify(sigs, os.Interrupt, syscall.SIGTERM) defer signal.Stop(sigs) if err := runChild(cmd, sigs); err != nil { - return fmt.Errorf("summer: %s: %w", strings.Join(argv, " "), err) + err = fmt.Errorf("summer: %s: %w", strings.Join(argv, " "), err) + var exit *exec.ExitError + if errors.As(err, &exit) && exit.Exited() { + // The binary printed its own error; summer exits with its + // code and prints nothing more. A binary ended by a signal + // printed nothing, so it gets summer's error block. + return bonfire.Reported(bonfire.WithExitCode(err, exit.ExitCode())) + } + return err } return nil } diff --git a/cmd/summer/runtime_test.go b/cmd/summer/runtime_test.go index dd7c335e..a2e74680 100644 --- a/cmd/summer/runtime_test.go +++ b/cmd/summer/runtime_test.go @@ -288,3 +288,75 @@ func TestDelegateExamples(t *testing.T) { } } } + +// TestExitHelperProcess is not a real test: TestDelegateExitCodePassthrough +// re-executes the test binary with SUMMER_EXIT_HELPER set to get a child +// that prints an error on stderr and exits 2, or that is killed by SIGKILL. +func TestExitHelperProcess(t *testing.T) { + switch os.Getenv("SUMMER_EXIT_HELPER") { + case "exit2": + fmt.Fprintln(os.Stderr, "ERROR the app binary failed") + os.Exit(2) + case "sigkill": + p, _ := os.FindProcess(os.Getpid()) + _ = p.Signal(syscall.SIGKILL) + time.Sleep(10 * time.Second) + os.Exit(3) + } +} + +// exitHelperRunner replaces appRunner with one that runs the exit helper +// as the app binary. +func exitHelperRunner(t *testing.T, mode string) { + t.Helper() + prev := appRunner + appRunner = func(ctx context.Context, argv []string, out bonfire.Output) error { + cmd := newAppCmd(ctx, os.Args[0], ".", []string{"-test.run=^TestExitHelperProcess$"}, out) + cmd.Env = append(os.Environ(), "SUMMER_EXIT_HELPER="+mode) + return runAppCmd(cmd, argv) + } + t.Cleanup(func() { appRunner = prev }) +} + +// TestDelegateExitCodePassthrough (CR-02): summer exits with the app +// binary's exit code and prints no error block of its own; a binary killed +// by a signal gets summer's error block and exit 1. +func TestDelegateExitCodePassthrough(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("SIGKILL is POSIX-only") + } + run := func(t *testing.T, mode string) (string, string, int) { + t.Helper() + exitHelperRunner(t, mode) + var out, errW syncBuffer + root, err := bonfire.NewRootIO("summer", toolCommands(), strings.NewReader(""), &out, &errW) + if err != nil { + t.Fatal(err) + } + code := bonfire.Execute(context.Background(), root, []string{"migrate", "--bogus"}) + return out.String(), errW.String(), code + } + + t.Run("exit code", func(t *testing.T) { + out, errOut, code := run(t, "exit2") + if code != 2 { + t.Errorf("exit %d, want the binary's 2", code) + } + for _, s := range []string{out, errOut} { + if strings.Contains(s, " ERROR ") || strings.Contains(s, "summer: migrate") { + t.Errorf("summer printed an error block of its own:\nstdout %q\nstderr %q", out, errOut) + break + } + } + }) + + t.Run("killed by a signal", func(t *testing.T) { + out, errOut, code := run(t, "sigkill") + if code != bonfire.ExitFailure { + t.Errorf("exit %d, want 1", code) + } + if n := strings.Count(errOut, " ERROR "); n != 1 || !strings.Contains(errOut, "summer: migrate --bogus: signal: killed") { + t.Errorf("want one error block naming the signal, got %d:\nstdout %q\nstderr %q", n, out, errOut) + } + }) +} diff --git a/docs/console/introduction.md b/docs/console/introduction.md index fd69e7b3..eaab6c80 100644 --- a/docs/console/introduction.md +++ b/docs/console/introduction.md @@ -141,6 +141,8 @@ summer -v migrate --pretend summer admin:token:create jane --scope journal.posts --json ``` +When the binary fails, `summer` exits with the binary's exit code (a usage error stays `2`) and prints nothing more: the binary has already printed its error. A binary killed by a signal gets an error block from `summer` and exit code `1`. + When `summer` receives SIGINT or SIGTERM while the binary runs, it passes the signal on and waits for the binary to shut down, so stopping `summer serve` with Ctrl+C or `kill` frees its port. `summer` does not rebuild an existing binary before it delegates. Run `summer build` after you change code, or keep `summer dev` running. Every other runtime command, such as `deferred:purge` or the commands your plugins add, is run on the binary directly. diff --git a/docs/setup/upgrading.md b/docs/setup/upgrading.md index a1e5f2aa..7ce43d22 100644 --- a/docs/setup/upgrading.md +++ b/docs/setup/upgrading.md @@ -43,6 +43,7 @@ Applications pin exact versions, so a new release never reaches an application u - **Exit codes.** A failed command exits 1 and a usage error (an unknown command or flag, a wrong number of arguments) exits 2. Errors print as a block on stderr with a hint where the framework knows the fix, and `-v` shows the wrapped errors. - **`-q` keeps data.** `-q` hides information and success lines, spinners and progress bars, but still prints what a command exists to produce: the token of `admin:token:create`, the key of `key:generate` and every `--json` output. - **Production confirmation.** `migrate`, `migrate:rollback`, `deferred:purge` and `queue:clear` ask before they run when the environment is `production` and they run on an interactive terminal; `--force` skips the question. Off a terminal, as in deploy scripts over SSH, nothing changes. +- **`summer` delegates keep the binary's exit code.** Since 0.4.2 `summer migrate`, `summer serve` and the other delegated commands exit with the app binary's exit code and print only the binary's error block; 0.4.0 and 0.4.1 printed a second block and exited 1. - **`summer dev` runs `serve`.** It used to start the binary without a command, which printed the help and stopped; it now runs `bin/ serve` with the global options you give `summer dev`. - **Result lines.** `summer build` ends with `Built bin/ in