diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 0efcc70118fe..38474d534869 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "io" "os" "os/exec" "os/signal" @@ -133,6 +134,34 @@ func cmdErrorMessage(err error) string { return fmt.Sprintf("exited with code %d", getExitCode(err)) } +// printCommandError prints err to stderr before plugin hooks run, so that +// hook output (such as the "What's next:" hint) is rendered after the +// command's own error output instead of before it. +// +// Errors caused by context cancellation, user-initiated signal termination, +// or errors with an empty message are not printed (matching the conditions +// used in [main]). +// +// If err was printed, it is replaced with a status only [cli.StatusError] +// preserving the exit code, so that [main] does not print the same message +// a second time. +func printCommandError(stderr io.Writer, err error) error { + if err == nil { + return nil + } + if errdefs.IsCanceled(err) { + return err + } + if errors.As(err, &errCtxSignalTerminated{}) { + return err + } + if err.Error() == "" { + return err + } + _, _ = fmt.Fprintln(stderr, err) + return cli.StatusError{StatusCode: getExitCode(err)} +} + func newDockerCommand(dockerCli *command.DockerCli) *cli.TopLevelCommand { var ( opts *cliflags.ClientOptions @@ -543,10 +572,19 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error { cmd.SetArgs(args) err = cmd.ExecuteContext(ctx) + // Capture the error message before printCommandError may replace err; + // plugin hooks still need to see the original message. + errMessage := cmdErrorMessage(err) + + // Print the command's error before invoking plugin hooks so that the + // "What's next" hint (and any other hook output) is rendered after + // the command's error output, not before it. + err = printCommandError(dockerCli.Err(), err) + // If the command is being executed in an interactive terminal // and hook are enabled, run the plugin hooks. if subCommand != nil && dockerCli.Out().IsTerminal() && dockerCli.HooksEnabled() { - pluginmanager.RunCLICommandHooks(ctx, dockerCli, cmd, subCommand, cmdErrorMessage(err)) + pluginmanager.RunCLICommandHooks(ctx, dockerCli, cmd, subCommand, errMessage) } return err diff --git a/cmd/docker/docker_test.go b/cmd/docker/docker_test.go index 928fcc6ee201..3995943e9f77 100644 --- a/cmd/docker/docker_test.go +++ b/cmd/docker/docker_test.go @@ -163,6 +163,90 @@ func TestGetExitCode(t *testing.T) { }) } +func TestPrintCommandError(t *testing.T) { + t.Run("nil error returns nil and writes nothing", func(t *testing.T) { + var buf bytes.Buffer + got := printCommandError(&buf, nil) + assert.NilError(t, got) + assert.Equal(t, buf.String(), "") + }) + + t.Run("generic error is printed and replaced with StatusError", func(t *testing.T) { + var buf bytes.Buffer + orig := errors.New("docker: open ./no-such-file: no such file or directory") + got := printCommandError(&buf, orig) + + // The original message is written to stderr before hooks run + assert.Equal(t, buf.String(), orig.Error()+"\n") + + // and the returned error is a status-only StatusError so + // main() does not print the same message a second time. + var st dockercli.StatusError + assert.Assert(t, errors.As(got, &st)) + assert.Equal(t, st.Status, "") + assert.Equal(t, st.StatusCode, 1) + }) + + t.Run("StatusError with message preserves exit code and prints message", func(t *testing.T) { + var buf bytes.Buffer + orig := dockercli.StatusError{Status: "build failed", StatusCode: 125} + got := printCommandError(&buf, orig) + + assert.Equal(t, buf.String(), "build failed\n") + + var st dockercli.StatusError + assert.Assert(t, errors.As(got, &st)) + assert.Equal(t, st.StatusCode, 125) + // The replacement is status-only; the message field is cleared + // because we already printed it ourselves. + assert.Equal(t, st.Status, "") + }) + + t.Run("StatusError with only exit code is not printed", func(t *testing.T) { + var buf bytes.Buffer + got := printCommandError(&buf, dockercli.StatusError{StatusCode: 42}) + + // main() also skips printing exit-code-only StatusErrors, so we + // must not print it here either, and the exit code must propagate. + assert.Equal(t, buf.String(), "") + + var st dockercli.StatusError + assert.Assert(t, errors.As(got, &st)) + assert.Equal(t, st.StatusCode, 42) + assert.Equal(t, st.Status, "") + }) + + t.Run("canceled error is not printed and not replaced", func(t *testing.T) { + var buf bytes.Buffer + got := printCommandError(&buf, context.Canceled) + + assert.Equal(t, buf.String(), "") + // If it had been replaced with a StatusError, errors.Is would + // return false; this asserts the error is propagated as-is. + assert.ErrorIs(t, got, context.Canceled) + }) + + t.Run("wrapped canceled error is not printed and not replaced", func(t *testing.T) { + var buf bytes.Buffer + got := printCommandError(&buf, fmt.Errorf("wrapped: %w", context.Canceled)) + + assert.Equal(t, buf.String(), "") + assert.ErrorIs(t, got, context.Canceled) + }) + + t.Run("signal-terminated error is not printed and not replaced", func(t *testing.T) { + var buf bytes.Buffer + orig := errCtxSignalTerminated{signal: syscall.SIGINT} + got := printCommandError(&buf, orig) + + assert.Equal(t, buf.String(), "") + + var sig errCtxSignalTerminated + assert.Assert(t, errors.As(got, &sig)) + assert.Equal(t, sig, orig) + }) +} + func TestCmdErrorMessage(t *testing.T) { t.Run("nil error returns empty string", func(t *testing.T) { assert.Equal(t, cmdErrorMessage(nil), "")