Merge pull request #6976 from olabie2/6973-print-error-before-plugin-hooks

cmd/docker: print command error before running plugin hooks

cmd/docker/docker.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"os/signal"
@@ -470,6 +471,34 @@ func restoreTerminal(streams command.Streams) {
 	streams.Err().RestoreTerminal()
 }
 
+// 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)}
+}
+
 //nolint:gocyclo
 func runDocker(ctx context.Context, dockerCli *command.DockerCli) error {
 	tcmd := newDockerCommand(dockerCli)
@@ -519,6 +548,7 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error {
 			err := tryPluginRun(ctx, dockerCli, cmd, args[0], envs)
 			if ccmd != nil && dockerCli.Out().IsTerminal() && dockerCli.HooksEnabled() && !errdefs.IsNotFound(err) {
 				errMessage := cmdErrorMessage(err)
+				err = printCommandError(dockerCli.Err(), err)
 				pluginmanager.RunPluginHooks(ctx, dockerCli, cmd, ccmd, args, errMessage)
 			}
 			if err == nil {
@@ -543,9 +573,12 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error {
 	err = cmd.ExecuteContext(ctx)
 
 	// If the command is being executed in an interactive terminal
-	// and hook are enabled, run the plugin hooks.
+	// and hooks are enabled, run the plugin hooks. Print the command's
+	// error first so that hook output is rendered after the error.
 	if subCommand != nil && dockerCli.Out().IsTerminal() && dockerCli.HooksEnabled() {
-		pluginmanager.RunCLICommandHooks(ctx, dockerCli, cmd, subCommand, cmdErrorMessage(err))
+		errMessage := cmdErrorMessage(err)
+		err = printCommandError(dockerCli.Err(), err)
+		pluginmanager.RunCLICommandHooks(ctx, dockerCli, cmd, subCommand, errMessage)
 	}
 
 	return err

cmd/docker/docker_test.go

@@ -163,6 +163,84 @@ func TestGetExitCode(t *testing.T) {
 	})
 }
 
+func TestPrintCommandError(t *testing.T) {
+	t.Run("nil error", 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", 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)
+
+		assert.Equal(t, buf.String(), orig.Error()+"\n")
+
+		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", func(t *testing.T) {
+		var buf bytes.Buffer
+		got := printCommandError(&buf, dockercli.StatusError{
+			Status:     "build failed",
+			StatusCode: 125,
+		})
+
+		assert.Equal(t, buf.String(), "build failed\n")
+
+		var st dockercli.StatusError
+		assert.Assert(t, errors.As(got, &st))
+		assert.Equal(t, st.Status, "")
+		assert.Equal(t, st.StatusCode, 125)
+	})
+
+	t.Run("StatusError without message", func(t *testing.T) {
+		var buf bytes.Buffer
+		got := printCommandError(&buf, dockercli.StatusError{StatusCode: 42})
+
+		assert.Equal(t, buf.String(), "")
+
+		var st dockercli.StatusError
+		assert.Assert(t, errors.As(got, &st))
+		assert.Equal(t, st.Status, "")
+		assert.Equal(t, st.StatusCode, 42)
+	})
+
+	t.Run("canceled error", func(t *testing.T) {
+		var buf bytes.Buffer
+		got := printCommandError(&buf, context.Canceled)
+
+		assert.Equal(t, buf.String(), "")
+		assert.ErrorIs(t, got, context.Canceled)
+	})
+
+	t.Run("wrapped canceled error", 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", 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), "")