diff --git a/cmd/summer/runtime.go b/cmd/summer/runtime.go index 33b4fe0a..ec410930 100644 --- a/cmd/summer/runtime.go +++ b/cmd/summer/runtime.go @@ -162,22 +162,26 @@ func appStreams(out bonfire.Output, terminal bool) (stdout, stderr io.Writer) { // 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. The -// error names the command through argmask.Join, with the values of -// secret-looking flags masked, on every branch: a binary that fails to -// start or is ended by a signal gets summer's error block, and a signal -// death keeps exit code 1. +// that code (bonfire.WithExitCode) and is marked as already reported. A +// binary ended by a signal makes summer exit 128 plus the signal number +// (signalExit). The error names the command through argmask.Join, with the +// values of secret-looking flags masked, on every branch that prints it: a +// binary that fails to start gets summer's error block and exit code 1. 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 { - err = fmt.Errorf("summer: %s: %w", argmask.Join(argv), err) var exit *exec.ExitError + if errors.As(err, &exit) { + if ws, ok := exit.Sys().(syscall.WaitStatus); ok && ws.Signaled() { + return signalExit(argv, err, ws.Signal()) + } + } + err = fmt.Errorf("summer: %s: %w", argmask.Join(argv), err) 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. + // code and prints nothing more. return bonfire.Reported(bonfire.WithExitCode(err, exit.ExitCode())) } return err @@ -185,6 +189,21 @@ func runAppCmd(cmd *exec.Cmd, argv []string) error { return nil } +// signalExit is the error for an app binary, started with argv, that died +// by sig: it carries exit code 128 plus the signal number, as a shell +// reports such a death (130 for Ctrl+C, 143 for SIGTERM, 137 for SIGKILL). +// SIGINT and SIGTERM are what stopping summer or the binary sends, so they +// count as already reported and print nothing more; any other signal gets +// summer's error block, naming the command through argmask.Join with the +// values of secret-looking flags masked. +func signalExit(argv []string, err error, sig syscall.Signal) error { + err = bonfire.WithExitCode(fmt.Errorf("summer: %s: %w", argmask.Join(argv), err), 128+int(sig)) + if sig == syscall.SIGINT || sig == syscall.SIGTERM { + return bonfire.Reported(err) + } + return err +} + // appShutdownGrace is how long a cancelled app binary may take to exit after // SIGTERM before it is killed: the grace summer dev gives serve as well, // longer than the 10-second shutdown timeout of serve and the workers. diff --git a/cmd/summer/runtime_test.go b/cmd/summer/runtime_test.go index 8b637454..e4f5334a 100644 --- a/cmd/summer/runtime_test.go +++ b/cmd/summer/runtime_test.go @@ -293,7 +293,8 @@ 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. +// that prints an error on stderr and exits 2, or that is killed by SIGKILL +// or SIGTERM. func TestExitHelperProcess(t *testing.T) { switch os.Getenv("SUMMER_EXIT_HELPER") { case "exit2": @@ -304,6 +305,12 @@ func TestExitHelperProcess(t *testing.T) { _ = p.Signal(syscall.SIGKILL) time.Sleep(10 * time.Second) os.Exit(3) + case "sigterm": + signal.Reset(syscall.SIGTERM) + p, _ := os.FindProcess(os.Getpid()) + _ = p.Signal(syscall.SIGTERM) + time.Sleep(10 * time.Second) + os.Exit(3) } } @@ -323,8 +330,8 @@ func exitHelperRunner(t *testing.T, mode string) { // TestDelegateExitCodePassthrough (CR-02, WR-10): 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, with secret flag values -// masked, and exit 1. +// killed by a signal exits 128 plus the signal number: SIGKILL gets +// summer's error block, with secret flag values masked, SIGTERM none. func TestDelegateExitCodePassthrough(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("SIGKILL is POSIX-only") @@ -361,8 +368,8 @@ func TestDelegateExitCodePassthrough(t *testing.T) { t.Run("killed by a signal", func(t *testing.T) { out, errOut, code := run(t, "sigkill", "migrate", "--bogus") - if code != bonfire.ExitFailure { - t.Errorf("exit %d, want 1", code) + if code != 137 { + t.Errorf("exit %d, want 137 (128+SIGKILL)", 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) @@ -371,8 +378,8 @@ func TestDelegateExitCodePassthrough(t *testing.T) { t.Run("killed by a signal masks secrets", func(t *testing.T) { out, errOut, code := run(t, "sigkill", "migrate", "--password", "hunter2") - if code != bonfire.ExitFailure { - t.Errorf("exit %d, want 1", code) + if code != 137 { + t.Errorf("exit %d, want 137 (128+SIGKILL)", code) } if n := strings.Count(errOut, " ERROR "); n != 1 || !strings.Contains(errOut, "--password ***") { t.Errorf("want one error block with the password masked, got %d:\nstdout %q\nstderr %q", n, out, errOut) @@ -381,6 +388,56 @@ func TestDelegateExitCodePassthrough(t *testing.T) { t.Errorf("the password reached the output:\nstdout %q\nstderr %q", out, errOut) } }) + + t.Run("terminated by SIGTERM", func(t *testing.T) { + out, errOut, code := run(t, "sigterm", "migrate", "--password", "hunter2") + if code != 143 { + t.Errorf("exit %d, want 143 (128+SIGTERM)", code) + } + if strings.Contains(out+errOut, " ERROR ") || strings.Contains(out+errOut, "hunter2") { + t.Errorf("summer printed a block for SIGTERM:\nstdout %q\nstderr %q", out, errOut) + } + }) +} + +// TestDelegateSignalExitCodes (WR-10 follow-up): a delegate that died by a +// signal makes summer exit 128 plus the signal number. SIGINT (130) and +// SIGTERM (143) count as already reported and print nothing; any other +// signal (SIGKILL, 137) gets summer's error block with secret flag values +// masked. The mapping is driven directly, so no signal is delivered. +func TestDelegateSignalExitCodes(t *testing.T) { + argv := []string{"migrate", "--password", "hunter2"} + cases := []struct { + sig syscall.Signal + code int + block bool + }{ + {syscall.SIGINT, 130, false}, + {syscall.SIGTERM, 143, false}, + {syscall.SIGKILL, 137, true}, + } + for _, tc := range cases { + t.Run(tc.sig.String(), func(t *testing.T) { + cmd := bonfire.Command{Name: "migrate", Passthrough: true, Run: func(context.Context, bonfire.Input, bonfire.Output) error { + return signalExit(argv, errors.New("signal: "+tc.sig.String()), tc.sig) + }} + var out, errW syncBuffer + root, err := bonfire.NewRootIO("summer", []bonfire.Command{cmd}, strings.NewReader(""), &out, &errW) + if err != nil { + t.Fatal(err) + } + if code := bonfire.Execute(context.Background(), root, argv); code != tc.code { + t.Errorf("exit %d, want %d", code, tc.code) + } + got := errW.String() + if !tc.block && got != "" { + t.Errorf("printed a block:\n%s", got) + } + if tc.block && (strings.Count(got, " ERROR ") != 1 || !strings.Contains(got, "summer: migrate --password ***") || strings.Contains(got, "hunter2")) { + t.Errorf("want one block with the argv masked:\n%s", got) + } + }) + } } // TestRunAppCmdMasksSecrets (WR-10): the error of an app binary killed by a diff --git a/docs/console/introduction.md b/docs/console/introduction.md index 60212251..c4a2f22c 100644 --- a/docs/console/introduction.md +++ b/docs/console/introduction.md @@ -143,7 +143,7 @@ summer admin:token:create jane --scope journal.posts --json The binary writes to your terminal directly, so it asks the production question, prints its styled startup summary and animates its spinners as if you had run it yourself. Its error stream stays separate from its output: `summer route:list --json > routes.json` captures only the JSON, while warnings and log lines still reach the terminal on stderr. -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`; the block names the command with the values of secret-looking flags (`--password`, `--api-token=...`) masked as `***`. +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 that dies by a signal makes `summer` exit with 128 plus the signal number, as a shell reports it: `130` for Ctrl+C (SIGINT), `143` for SIGTERM, `137` for SIGKILL. After SIGINT or SIGTERM `summer` prints nothing more; any other signal gets an error block from `summer` that names the command with the values of secret-looking flags (`--password`, `--api-token=...`) masked as `***`. 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. diff --git a/scripts/check-phase19.8.sh b/scripts/check-phase19.8.sh index 9545f558..ee765a8d 100755 --- a/scripts/check-phase19.8.sh +++ b/scripts/check-phase19.8.sh @@ -192,6 +192,8 @@ SUMMER_TESTS=( TestDelegateExitCodePassthrough TestAppStreams TestDelegateStreams # quick 261022-4z8 (re-review fixes): WR-10. TestRunAppCmdMasksSecrets + # quick 261028-rqd: WR-10 follow-up (exit 128+n). + TestDelegateSignalExitCodes ) # hello stage.