cli/command: ctx cancel should not print or produce a non zero exit code

The user might kill the CLI through a SIGINT/SIGTERM which cancels the main context we pass around. Currently the context cancel error is printed alongside any other wrapped error with a generic exit code (125). This patch improves on this behavior and prevents any error from being printed when they match `context.Cancelled`. The `cli.StatusError` error would wrap errors but not provide a way to unwrap them. This would lead to situations where `errors.Is` would not match the underlying error. Signed-off-by: Alano Terblanche <[email protected]>
docker · Jan 7, 2025 · 3c2e27d · 3c2e27d
1 parent 5f22178
commit 3c2e27d
Show file tree

Hide file tree

Showing 22 changed files with 101 additions and 59 deletions.
diff --git a/cli/cobra.go b/cli/cobra.go
@@ -10,6 +10,7 @@ import (
 	pluginmanager "github.com/docker/cli/cli-plugins/manager"
 	"github.com/docker/cli/cli/command"
 	cliflags "github.com/docker/cli/cli/flags"
+	"github.com/docker/cli/internal"
 	"github.com/docker/docker/pkg/homedir"
 	"github.com/docker/docker/registry"
 	"github.com/fvbommel/sortorder"
@@ -92,8 +93,8 @@ func FlagErrorFunc(cmd *cobra.Command, err error) error {
 		return nil
 	}
 
-	return StatusError{
-		Status:     fmt.Sprintf("%s\n\nUsage:  %s\n\nRun '%s --help' for more information", err, cmd.UseLine(), cmd.CommandPath()),
+	return internal.StatusError{
+		Cause:      fmt.Errorf("%w\n\nUsage:  %s\n\nRun '%s --help' for more information", err, cmd.UseLine(), cmd.CommandPath()),
 		StatusCode: 125,
 	}
 }

diff --git a/cli/command/config/inspect.go b/cli/command/config/inspect.go
@@ -12,6 +12,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/formatter"
 	flagsHelper "github.com/docker/cli/cli/flags"
+	"github.com/docker/cli/internal"
 	"github.com/spf13/cobra"
 )
 
@@ -67,7 +68,7 @@ func RunConfigInspect(ctx context.Context, dockerCli command.Cli, opts InspectOp
 	}
 
 	if err := InspectFormatWrite(configCtx, opts.Names, getRef); err != nil {
-		return cli.StatusError{StatusCode: 1, Status: err.Error()}
+		return internal.StatusError{StatusCode: 1, Cause: err}
 	}
 	return nil
 }
diff --git a/cli/command/container/attach.go b/cli/command/container/attach.go
@@ -7,6 +7,7 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
+	"github.com/docker/cli/internal"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/client"
 	"github.com/moby/sys/signal"
@@ -161,7 +162,7 @@ func getExitStatus(errC <-chan error, resultC <-chan container.WaitResponse) err
 			return errors.New(result.Error.Message)
 		}
 		if result.StatusCode != 0 {
-			return cli.StatusError{StatusCode: int(result.StatusCode)}
+			return internal.StatusError{StatusCode: int(result.StatusCode)}
 		}
 	case err := <-errC:
 		return err

diff --git a/cli/command/container/attach_test.go b/cli/command/container/attach_test.go
@@ -4,7 +4,7 @@ import (
 	"io"
 	"testing"
 
-	"github.com/docker/cli/cli"
+	"github.com/docker/cli/internal"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/docker/api/types/container"
 	"github.com/pkg/errors"
@@ -110,7 +110,7 @@ func TestGetExitStatus(t *testing.T) {
 			result: &container.WaitResponse{
 				StatusCode: 15,
 			},
-			expectedError: cli.StatusError{StatusCode: 15},
+			expectedError: internal.StatusError{StatusCode: 15},
 		},
 	}
 

diff --git a/cli/command/container/create.go b/cli/command/container/create.go
@@ -14,6 +14,7 @@ import (
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/cli/cli/command/image"
 	"github.com/docker/cli/cli/streams"
+	"github.com/docker/cli/internal"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types/container"
 	imagetypes "github.com/docker/docker/api/types/image"
@@ -92,8 +93,8 @@ func NewCreateCommand(dockerCli command.Cli) *cobra.Command {
 
 func runCreate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, options *createOptions, copts *containerOptions) error {
 	if err := validatePullOpt(options.pull); err != nil {
-		return cli.StatusError{
-			Status:     withHelp(err, "create").Error(),
+		return internal.StatusError{
+			Cause:      withHelp(err, "create"),
 			StatusCode: 125,
 		}
 	}
@@ -109,14 +110,14 @@ func runCreate(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet,
 	copts.env = *opts.NewListOptsRef(&newEnv, nil)
 	containerCfg, err := parse(flags, copts, dockerCli.ServerInfo().OSType)
 	if err != nil {
-		return cli.StatusError{
-			Status:     withHelp(err, "create").Error(),
+		return internal.StatusError{
+			Cause:      withHelp(err, "create"),
 			StatusCode: 125,
 		}
 	}
 	if err = validateAPIVersion(containerCfg, dockerCli.Client().ClientVersion()); err != nil {
-		return cli.StatusError{
-			Status:     withHelp(err, "create").Error(),
+		return internal.StatusError{
+			Cause:      withHelp(err, "create"),
 			StatusCode: 125,
 		}
 	}

diff --git a/cli/command/container/create_test.go b/cli/command/container/create_test.go
@@ -10,8 +10,8 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/config/configfile"
+	"github.com/docker/cli/internal"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/cli/internal/test/notary"
 	"github.com/docker/docker/api/types/container"
@@ -185,7 +185,7 @@ func TestCreateContainerImagePullPolicyInvalid(t *testing.T) {
 				&containerOptions{},
 			)
 
-			statusErr := cli.StatusError{}
+			statusErr := internal.StatusError{}
 			assert.Check(t, errors.As(err, &statusErr))
 			assert.Check(t, is.Equal(statusErr.StatusCode, 125))
 			assert.Check(t, is.ErrorContains(err, tc.ExpectedErrMsg))

diff --git a/cli/command/container/exec.go b/cli/command/container/exec.go
@@ -9,6 +9,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/cli/cli/config/configfile"
+	"github.com/docker/cli/internal"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/client"
@@ -206,11 +207,11 @@ func getExecExitStatus(ctx context.Context, apiClient client.ContainerAPIClient,
 		if !client.IsErrConnectionFailed(err) {
 			return err
 		}
-		return cli.StatusError{StatusCode: -1}
+		return internal.StatusError{StatusCode: -1}
 	}
 	status := resp.ExitCode
 	if status != 0 {
-		return cli.StatusError{StatusCode: status}
+		return internal.StatusError{StatusCode: status}
 	}
 	return nil
 }

diff --git a/cli/command/container/exec_test.go b/cli/command/container/exec_test.go
@@ -6,8 +6,8 @@ import (
 	"os"
 	"testing"
 
-	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/config/configfile"
+	"github.com/docker/cli/internal"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types"
@@ -231,7 +231,7 @@ func TestGetExecExitStatus(t *testing.T) {
 		},
 		{
 			exitCode:      15,
-			expectedError: cli.StatusError{StatusCode: 15},
+			expectedError: internal.StatusError{StatusCode: 15},
 		},
 	}
 

diff --git a/cli/command/container/run.go b/cli/command/container/run.go
@@ -10,6 +10,7 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
+	"github.com/docker/cli/internal"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api/types/container"
 	"github.com/moby/sys/signal"
@@ -84,8 +85,8 @@ func NewRunCommand(dockerCli command.Cli) *cobra.Command {
 
 func runRun(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, ropts *runOptions, copts *containerOptions) error {
 	if err := validatePullOpt(ropts.pull); err != nil {
-		return cli.StatusError{
-			Status:     withHelp(err, "run").Error(),
+		return internal.StatusError{
+			Cause:      withHelp(err, "run"),
 			StatusCode: 125,
 		}
 	}
@@ -102,14 +103,14 @@ func runRun(ctx context.Context, dockerCli command.Cli, flags *pflag.FlagSet, ro
 	containerCfg, err := parse(flags, copts, dockerCli.ServerInfo().OSType)
 	// just in case the parse does not exit
 	if err != nil {
-		return cli.StatusError{
-			Status:     withHelp(err, "run").Error(),
+		return internal.StatusError{
+			Cause:      withHelp(err, "run"),
 			StatusCode: 125,
 		}
 	}
 	if err = validateAPIVersion(containerCfg, dockerCli.CurrentVersion()); err != nil {
-		return cli.StatusError{
-			Status:     withHelp(err, "run").Error(),
+		return internal.StatusError{
+			Cause:      withHelp(err, "run"),
 			StatusCode: 125,
 		}
 	}
@@ -242,7 +243,7 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption
 
 	status := <-statusChan
 	if status != 0 {
-		return cli.StatusError{StatusCode: status}
+		return internal.StatusError{StatusCode: status}
 	}
 	return nil
 }
@@ -302,7 +303,7 @@ func withHelp(err error, commandName string) error {
 
 // toStatusError attempts to detect specific error-conditions to assign
 // an appropriate exit-code for situations where the problem originates
-// from the container. It returns [cli.StatusError] with the original
+// from the container. It returns [internal.StatusError] with the original
 // error message and the Status field set as follows:
 //
 // - 125: for generic failures sent back from the daemon
@@ -314,21 +315,21 @@ func toStatusError(err error) error {
 	errMsg := err.Error()
 
 	if strings.Contains(errMsg, "executable file not found") || strings.Contains(errMsg, "no such file or directory") || strings.Contains(errMsg, "system cannot find the file specified") {
-		return cli.StatusError{
-			Status:     withHelp(err, "run").Error(),
+		return internal.StatusError{
+			Cause:      withHelp(err, "run"),
 			StatusCode: 127,
 		}
 	}
 
 	if strings.Contains(errMsg, syscall.EACCES.Error()) || strings.Contains(errMsg, syscall.EISDIR.Error()) {
-		return cli.StatusError{
-			Status:     withHelp(err, "run").Error(),
+		return internal.StatusError{
+			Cause:      withHelp(err, "run"),
 			StatusCode: 126,
 		}
 	}
 
-	return cli.StatusError{
-		Status:     withHelp(err, "run").Error(),
+	return internal.StatusError{
+		Cause:      withHelp(err, "run"),
 		StatusCode: 125,
 	}
 }
diff --git a/cli/command/container/run_test.go b/cli/command/container/run_test.go
@@ -12,8 +12,8 @@ import (
 	"time"
 
 	"github.com/creack/pty"
-	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/streams"
+	"github.com/docker/cli/internal"
 	"github.com/docker/cli/internal/test"
 	"github.com/docker/cli/internal/test/notary"
 	"github.com/docker/docker/api/types"
@@ -131,7 +131,7 @@ func TestRunAttach(t *testing.T) {
 
 	select {
 	case cmdErr := <-cmdErrC:
-		assert.Equal(t, cmdErr, cli.StatusError{
+		assert.Equal(t, cmdErr, internal.StatusError{
 			StatusCode: 33,
 		})
 	case <-time.After(2 * time.Second):
@@ -213,7 +213,7 @@ func TestRunAttachTermination(t *testing.T) {
 
 	select {
 	case cmdErr := <-cmdErrC:
-		assert.Equal(t, cmdErr, cli.StatusError{
+		assert.Equal(t, cmdErr, internal.StatusError{
 			StatusCode: 130,
 		})
 	case <-time.After(2 * time.Second):
@@ -294,10 +294,10 @@ func TestRunPullTermination(t *testing.T) {
 
 	select {
 	case cmdErr := <-cmdErrC:
-		assert.Equal(t, cmdErr, cli.StatusError{
-			StatusCode: 125,
-			Status:     "docker: context canceled\n\nRun 'docker run --help' for more information",
-		})
+		assert.ErrorIs(t, cmdErr, context.Canceled)
+		v, ok := cmdErr.(internal.StatusError)
+		assert.Check(t, ok)
+		assert.Check(t, is.Equal(v.StatusCode, 125))
 	case <-time.After(10 * time.Second):
 		t.Fatal("cmd did not return before the timeout")
 	}
@@ -347,7 +347,7 @@ func TestRunCommandWithContentTrustErrors(t *testing.T) {
 			cmd.SetOut(io.Discard)
 			cmd.SetErr(io.Discard)
 			err := cmd.Execute()
-			statusErr := cli.StatusError{}
+			statusErr := internal.StatusError{}
 			assert.Check(t, errors.As(err, &statusErr))
 			assert.Check(t, is.Equal(statusErr.StatusCode, 125))
 			assert.Check(t, is.ErrorContains(err, tc.expectedError))
@@ -380,7 +380,7 @@ func TestRunContainerImagePullPolicyInvalid(t *testing.T) {
 				&containerOptions{},
 			)
 
-			statusErr := cli.StatusError{}
+			statusErr := internal.StatusError{}
 			assert.Check(t, errors.As(err, &statusErr))
 			assert.Check(t, is.Equal(statusErr.StatusCode, 125))
 			assert.Check(t, is.ErrorContains(err, tc.ExpectedErrMsg))

diff --git a/cli/command/container/start.go b/cli/command/container/start.go
@@ -9,6 +9,7 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
+	"github.com/docker/cli/internal"
 	"github.com/docker/docker/api/types/container"
 	"github.com/moby/sys/signal"
 	"github.com/moby/term"
@@ -172,7 +173,7 @@ func RunStart(ctx context.Context, dockerCli command.Cli, opts *StartOptions) er
 		}
 
 		if status := <-statusChan; status != 0 {
-			return cli.StatusError{StatusCode: status}
+			return internal.StatusError{StatusCode: status}
 		}
 		return nil
 	case opts.Checkpoint != "":

diff --git a/cli/command/image/build.go b/cli/command/image/build.go
@@ -20,6 +20,7 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/cli/cli/command/image/build"
+	"github.com/docker/cli/internal"
 	"github.com/docker/cli/opts"
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/api/types"
@@ -371,7 +372,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
 			if options.quiet {
 				fmt.Fprintf(dockerCli.Err(), "%s%s", progBuff, buildBuff)
 			}
-			return cli.StatusError{Status: jerr.Message, StatusCode: jerr.Code}
+			return internal.StatusError{Cause: jerr, StatusCode: jerr.Code}
 		}
 		return err
 	}

diff --git a/cli/command/inspect/inspector.go b/cli/command/inspect/inspector.go
@@ -10,7 +10,7 @@ import (
 	"strings"
 	"text/template"
 
-	"github.com/docker/cli/cli"
+	"github.com/docker/cli/internal"
 	"github.com/docker/cli/templates"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -67,7 +67,7 @@ type GetRefFunc func(ref string) (any, []byte, error)
 func Inspect(out io.Writer, references []string, tmplStr string, getRef GetRefFunc) error {
 	inspector, err := NewTemplateInspectorFromString(out, tmplStr)
 	if err != nil {
-		return cli.StatusError{StatusCode: 64, Status: err.Error()}
+		return internal.StatusError{StatusCode: 64, Cause: err}
 	}
 
 	var inspectErrs []string
@@ -88,9 +88,9 @@ func Inspect(out io.Writer, references []string, tmplStr string, getRef GetRefFu
 	}
 
 	if len(inspectErrs) != 0 {
-		return cli.StatusError{
+		return internal.StatusError{
 			StatusCode: 1,
-			Status:     strings.Join(inspectErrs, "\n"),
+			Cause:      errors.New(strings.Join(inspectErrs, "\n")),
 		}
 	}
 	return nil

diff --git a/cli/command/network/remove.go b/cli/command/network/remove.go
@@ -7,6 +7,7 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
+	"github.com/docker/cli/internal"
 	"github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/errdefs"
 	"github.com/spf13/cobra"
@@ -68,7 +69,7 @@ func runRemove(ctx context.Context, dockerCli command.Cli, networks []string, op
 	}
 
 	if status != 0 {
-		return cli.StatusError{StatusCode: status}
+		return internal.StatusError{StatusCode: status}
 	}
 	return nil
 }