From c373c618dd6a9fde057e109410115853705dcef5 Mon Sep 17 00:00:00 2001 From: Mohammed Olabie Date: Mon, 11 May 2026 18:33:45 +0300 Subject: [PATCH 1/2] cmd/docker: print command error before running plugin hooks Plugin hook output (such as Gordon's "What's next:" hint) was rendered before the command's own error message because hooks were invoked inside runDocker while the error was only printed in main() after runDocker returned. Print the error to stderr before invoking the hooks, and replace the error with a status-only StatusError so main() does not print the same message a second time. The original error message is captured up-front and still passed to the plugin hooks. Closes #6973 Signed-off-by: Mohammed Olabie --- cmd/docker/docker.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 0efcc70118fe..ccd522f6d781 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -543,10 +543,24 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error { cmd.SetArgs(args) err = cmd.ExecuteContext(ctx) + // Capture the error message before we may consume the error below; + // 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. + if err != nil && !errdefs.IsCanceled(err) && !errors.As(err, &errCtxSignalTerminated{}) && err.Error() != "" { + _, _ = fmt.Fprintln(dockerCli.Err(), err) + // Replace the error with a status-only one so that main() + // does not print the same message a second time. + err = cli.StatusError{StatusCode: getExitCode(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 From 81915c59da3bd056934b800103bbbdfcada9c0f6 Mon Sep 17 00:00:00 2001 From: Mohammed Olabie Date: Thu, 14 May 2026 22:25:07 +0300 Subject: [PATCH 2/2] cmd/docker: extract printCommandError helper and add tests The inline error-printing block in runDocker is extracted into a printCommandError helper so the new behavior can be unit-tested directly without spinning up runDocker. TestPrintCommandError covers each branch of the helper: nil error, generic error (printed and replaced with StatusError), StatusError with/without message, context.Canceled (raw and wrapped), and errCtxSignalTerminated. Signed-off-by: Mohammed Olabie --- cmd/docker/docker.go | 38 ++++++++++++++---- cmd/docker/docker_test.go | 84 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 7 deletions(-) diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index ccd522f6d781..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,19 +572,14 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error { cmd.SetArgs(args) err = cmd.ExecuteContext(ctx) - // Capture the error message before we may consume the error below; + // 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. - if err != nil && !errdefs.IsCanceled(err) && !errors.As(err, &errCtxSignalTerminated{}) && err.Error() != "" { - _, _ = fmt.Fprintln(dockerCli.Err(), err) - // Replace the error with a status-only one so that main() - // does not print the same message a second time. - err = cli.StatusError{StatusCode: getExitCode(err)} - } + err = printCommandError(dockerCli.Err(), err) // If the command is being executed in an interactive terminal // and hook are enabled, run the plugin hooks. 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), "")