mirror of
https://github.com/docker/cli.git
synced 2026-09-27 01:40:31 -04:00
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. 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> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
+35
-2
@@ -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
|
||||
|
||||
@@ -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), "")
|
||||
|
||||
Reference in New Issue
Block a user