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 <olabiedev@gmail.com>

Author: olabie2

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.

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), "")