diff --git a/cli/command/container/cp.go b/cli/command/container/cp.go index 8193e8b04fe2..12779c8fd0c8 100644 --- a/cli/command/container/cp.go +++ b/cli/command/container/cp.go @@ -16,6 +16,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/streams" + "github.com/docker/cli/internal/hint" "github.com/docker/go-units" "github.com/moby/go-archive" "github.com/moby/moby/client" @@ -242,7 +243,10 @@ func runCopy(ctx context.Context, dockerCli command.Cli, opts copyOptions) error case acrossContainers: return errors.New("copying between containers is not supported") default: - return errors.New("must specify at least one container source") + return hint.Wrap( + errors.New("one argument must reference a container as 'CONTAINER:PATH'"), + "Use 'docker cp CONTAINER:SRC LOCAL' to copy from a container, or 'docker cp LOCAL CONTAINER:DEST' to copy into one.", + ) } } diff --git a/cli/command/container/cp_test.go b/cli/command/container/cp_test.go index e193ebbf411b..35283b0016c9 100644 --- a/cli/command/container/cp_test.go +++ b/cli/command/container/cp_test.go @@ -38,7 +38,7 @@ func TestRunCopyWithInvalidArguments(t *testing.T) { source: "./source", destination: "./dest", }, - expectedErr: "must specify at least one container source", + expectedErr: "one argument must reference a container as 'CONTAINER:PATH'", }, } for _, testcase := range testcases { diff --git a/cli/command/container/create.go b/cli/command/container/create.go index 2598323d7e6e..fa1e59a294bc 100644 --- a/cli/command/container/create.go +++ b/cli/command/container/create.go @@ -20,6 +20,7 @@ import ( "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/cli/config/types" "github.com/docker/cli/cli/streams" + "github.com/docker/cli/internal/hint" "github.com/docker/cli/internal/jsonstream" "github.com/docker/cli/opts" "github.com/moby/moby/api/types/mount" @@ -188,7 +189,10 @@ func (cid *cidFile) Write(id string) error { return nil } if _, err := cid.file.WriteString(id); err != nil { - return fmt.Errorf("failed to write the container ID (%s) to file: %w", id, err) + return hint.Wrap( + fmt.Errorf("container %s was created, but writing its ID to %s failed: %w", id, cid.path, err), + fmt.Sprintf("The container exists on the daemon. Run 'docker rm %s' to remove it, or note the ID for later use.", id), + ) } cid.written = true return nil diff --git a/cli/command/container/exec.go b/cli/command/container/exec.go index bd4f9bc6242c..021b30f2724e 100644 --- a/cli/command/container/exec.go +++ b/cli/command/container/exec.go @@ -10,6 +10,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/hint" "github.com/docker/cli/opts" "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" @@ -115,7 +116,10 @@ func RunExec(ctx context.Context, dockerCLI command.Cli, containerIDorName strin execID := response.ID if execID == "" { - return errors.New("exec ID empty") + return hint.Wrap( + errors.New("the Docker daemon returned an empty response when creating the exec session"), + "This is unexpected — please report it at https://github.com/moby/moby/issues with the daemon version ('docker version') and the command you ran.", + ) } if options.Detach { diff --git a/cli/command/container/exec_test.go b/cli/command/container/exec_test.go index 47b6574ec513..e4cd46f43c8c 100644 --- a/cli/command/container/exec_test.go +++ b/cli/command/container/exec_test.go @@ -195,7 +195,7 @@ func TestRunExec(t *testing.T) { { doc: "missing exec ID", options: NewExecOptions(), - expectedError: "exec ID empty", + expectedError: "the Docker daemon returned an empty response when creating the exec session", client: &fakeClient{}, }, } diff --git a/cli/command/container/export.go b/cli/command/container/export.go index 0c0f3410b5ed..b0bcd135ab95 100644 --- a/cli/command/container/export.go +++ b/cli/command/container/export.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/hint" "github.com/moby/moby/client" "github.com/moby/sys/atomicwriter" "github.com/spf13/cobra" @@ -49,13 +50,16 @@ func runExport(ctx context.Context, dockerCLI command.Cli, opts exportOptions) e var output io.Writer if opts.output == "" { if dockerCLI.Out().IsTerminal() { - return errors.New("cowardly refusing to save to a terminal. Use the -o flag or redirect") + return hint.Wrap( + errors.New("refusing to write a binary tar archive to the terminal"), + "Use '-o FILE' to write to a file, or redirect stdout, e.g. 'docker container export CONTAINER > out.tar'.", + ) } output = dockerCLI.Out() } else { writer, err := atomicwriter.New(opts.output, 0o600) if err != nil { - return fmt.Errorf("failed to export container: %w", err) + return fmt.Errorf("cannot open output file %q: %w", opts.output, err) } defer writer.Close() output = writer diff --git a/cli/command/container/export_test.go b/cli/command/container/export_test.go index 8fe6be06b9ff..048f9ae5fe97 100644 --- a/cli/command/container/export_test.go +++ b/cli/command/container/export_test.go @@ -44,6 +44,6 @@ func TestContainerExportOutputToIrregularFile(t *testing.T) { cmd.SetErr(io.Discard) cmd.SetArgs([]string{"-o", "/dev/random", "container"}) - const expected = `failed to export container: cannot write to a character device file` + const expected = `cannot open output file "/dev/random": cannot write to a character device file` assert.Error(t, cmd.Execute(), expected) } diff --git a/cli/command/container/opts.go b/cli/command/container/opts.go index f9d896882ebf..ae0a4a741960 100644 --- a/cli/command/container/opts.go +++ b/cli/command/container/opts.go @@ -18,6 +18,7 @@ import ( "strings" "time" + "github.com/docker/cli/internal/hint" "github.com/docker/cli/internal/lazyregexp" "github.com/docker/cli/internal/volumespec" "github.com/docker/cli/opts" @@ -517,22 +518,34 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con pidMode := container.PidMode(copts.pidMode) if !pidMode.Valid() { - return nil, errors.New("--pid: invalid PID mode") + return nil, hint.Wrap( + fmt.Errorf("invalid --pid mode %q", copts.pidMode), + "Valid forms are 'host' or 'container:'.", + ) } utsMode := container.UTSMode(copts.utsMode) if !utsMode.Valid() { - return nil, errors.New("--uts: invalid UTS mode") + return nil, hint.Wrap( + fmt.Errorf("invalid --uts mode %q", copts.utsMode), + "The only valid form is 'host'.", + ) } usernsMode := container.UsernsMode(copts.usernsMode) if !usernsMode.Valid() { - return nil, errors.New("--userns: invalid USER mode") + return nil, hint.Wrap( + fmt.Errorf("invalid --userns mode %q", copts.usernsMode), + "The only valid form is 'host'.", + ) } cgroupnsMode := container.CgroupnsMode(copts.cgroupnsMode) if !cgroupnsMode.Valid() { - return nil, errors.New("--cgroupns: invalid CGROUP mode") + return nil, hint.Wrap( + fmt.Errorf("invalid --cgroupns mode %q", copts.cgroupnsMode), + "Valid forms are 'private' or 'host'.", + ) } restartPolicy, err := opts.ParseRestartPolicy(copts.restartPolicy) @@ -920,7 +933,10 @@ func convertToStandardNotation(ports []string) ([]string, error) { func parseLoggingOpts(loggingDriver string, loggingOpts []string) (map[string]string, error) { loggingOptsMap := opts.ConvertKVStringsToMap(loggingOpts) if loggingDriver == "none" && len(loggingOpts) > 0 { - return map[string]string{}, fmt.Errorf("invalid logging opts for driver %s", loggingDriver) + return map[string]string{}, hint.Wrap( + errors.New("log driver \"none\" accepts no --log-opt entries"), + "Remove the --log-opt flag(s) or choose a different --log-driver.", + ) } return loggingOptsMap, nil } @@ -984,7 +1000,7 @@ func parseStorageOpts(storageOpts []string) (map[string]string, error) { for _, option := range storageOpts { k, v, ok := strings.Cut(option, "=") if !ok { - return nil, errors.New("invalid storage option") + return nil, fmt.Errorf("invalid --storage-opt value %q: expected key=value", option) } m[k] = v } @@ -1095,12 +1111,12 @@ func validateLinuxPath(val string, validator func(string) bool) (string, error) var mode string if strings.Count(val, ":") > 2 { - return val, fmt.Errorf("bad format for path: %s", val) + return val, fmt.Errorf("invalid --device %q: too many ':' separators, expected [host-path:]container-path[:mode]", val) } split := strings.SplitN(val, ":", 3) if split[0] == "" { - return val, fmt.Errorf("bad format for path: %s", val) + return val, fmt.Errorf("invalid --device %q: host path before ':' is empty, expected [host-path:]container-path[:mode]", val) } switch len(split) { case 1: diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index 51a0e72b4943..5e6dfc6d26a8 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -767,7 +767,7 @@ func TestParseModes(t *testing.T) { args := []string{"--pid=container:", "img", "cmd"} assert.NilError(t, flags.Parse(args)) _, err := parse(flags, copts, runtime.GOOS) - assert.ErrorContains(t, err, "--pid: invalid PID mode") + assert.ErrorContains(t, err, "invalid --pid mode") // pid ok _, hostconfig, _, err := parseRun([]string{"--pid=host", "img", "cmd"}) @@ -778,7 +778,7 @@ func TestParseModes(t *testing.T) { // uts ko _, _, _, err = parseRun([]string{"--uts=container:", "img", "cmd"}) //nolint:dogsled - assert.ErrorContains(t, err, "--uts: invalid UTS mode") + assert.ErrorContains(t, err, "invalid --uts mode") // uts ok _, hostconfig, _, err = parseRun([]string{"--uts=host", "img", "cmd"}) @@ -923,8 +923,8 @@ func TestParseHealth(t *testing.T) { func TestParseLoggingOpts(t *testing.T) { // logging opts ko - if _, _, _, err := parseRun([]string{"--log-driver=none", "--log-opt=anything", "img", "cmd"}); err == nil || err.Error() != "invalid logging opts for driver none" { - t.Fatalf("Expected an error with message 'invalid logging opts for driver none', got %v", err) + if _, _, _, err := parseRun([]string{"--log-driver=none", "--log-opt=anything", "img", "cmd"}); err == nil || !strings.HasPrefix(err.Error(), "log driver \"none\" accepts no --log-opt entries") { + t.Fatalf("Expected an error stating that 'none' accepts no --log-opt entries, got %v", err) } // logging opts ok _, hostconfig, _, err := parseRun([]string{"--log-driver=syslog", "--log-opt=something", "img", "cmd"}) @@ -1034,22 +1034,24 @@ func TestValidateDevice(t *testing.T) { "/hostPath:/containerPath:rw", "/hostPath:/containerPath:mrw", } + const emptyHostMsg = `: host path before ':' is empty, expected [host-path:]container-path[:mode]` + const tooManyMsg = `: too many ':' separators, expected [host-path:]container-path[:mode]` invalid := map[string]string{ - "": "bad format for path: ", + "": `invalid --device ""` + emptyHostMsg, "./": "./ is not an absolute path", "../": "../ is not an absolute path", "/:../": "../ is not an absolute path", "/:path": "path is not an absolute path", - ":": "bad format for path: :", + ":": `invalid --device ":"` + emptyHostMsg, "/tmp:": " is not an absolute path", - ":test": "bad format for path: :test", - ":/test": "bad format for path: :/test", + ":test": `invalid --device ":test"` + emptyHostMsg, + ":/test": `invalid --device ":/test"` + emptyHostMsg, "tmp:": " is not an absolute path", - ":test:": "bad format for path: :test:", - "::": "bad format for path: ::", - ":::": "bad format for path: :::", - "/tmp:::": "bad format for path: /tmp:::", - ":/tmp::": "bad format for path: :/tmp::", + ":test:": `invalid --device ":test:"` + emptyHostMsg, + "::": `invalid --device "::"` + emptyHostMsg, + ":::": `invalid --device ":::"` + tooManyMsg, + "/tmp:::": `invalid --device "/tmp:::"` + tooManyMsg, + ":/tmp::": `invalid --device ":/tmp::"` + tooManyMsg, "path:ro": "ro is not an absolute path", "path:rr": "rr is not an absolute path", "a:/b:ro": "bad mode specified: ro", diff --git a/cli/command/container/update.go b/cli/command/container/update.go index 3bc11412e19a..ab8c40bf62f8 100644 --- a/cli/command/container/update.go +++ b/cli/command/container/update.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/hint" "github.com/docker/cli/opts" containertypes "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" @@ -92,7 +93,10 @@ func runUpdate(ctx context.Context, dockerCli command.Cli, options *updateOption var err error if options.nFlag == 0 { - return errors.New("you must provide one or more flags when using this command") + return hint.Wrap( + errors.New("no resource flags supplied — nothing to update"), + "Pass at least one tunable flag (for example --memory, --cpus, --restart). Run 'docker container update --help' for the full list.", + ) } var restartPolicy containertypes.RestartPolicy diff --git a/cli/command/context/create.go b/cli/command/context/create.go index f7e62205ba21..cb677864a455 100644 --- a/cli/command/context/create.go +++ b/cli/command/context/create.go @@ -85,7 +85,7 @@ func runCreate(dockerCLI command.Cli, name string, opts createOptions) error { func createNewContext(contextStore store.ReaderWriter, name string, opts createOptions) error { if opts.endpoint == nil { - return errors.New("docker endpoint configuration is required") + return errors.New("no docker endpoint configured: set one with --docker, or copy from an existing context with --from") } dockerEP, dockerTLS, err := getDockerEndpointMetadataAndTLS(contextStore, opts.endpoint) if err != nil { diff --git a/cli/command/context/export.go b/cli/command/context/export.go index 53f7e1fb989a..e70c5809b9b4 100644 --- a/cli/command/context/export.go +++ b/cli/command/context/export.go @@ -9,6 +9,7 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/context/store" + "github.com/docker/cli/internal/hint" "github.com/spf13/cobra" ) @@ -37,7 +38,10 @@ func writeTo(dockerCli command.Cli, reader io.Reader, dest string) error { var printDest bool if dest == "-" { if dockerCli.Out().IsTerminal() { - return errors.New("cowardly refusing to export to a terminal, specify a file path") + return hint.Wrap( + errors.New("exported context is a binary tar stream and would corrupt the terminal"), + "Pass a file path as the second argument, or redirect stdout (e.g. 'docker context export NAME > file.dockercontext').", + ) } writer = dockerCli.Out() } else { diff --git a/cli/command/context/options.go b/cli/command/context/options.go index 429f47cc4139..c8d1491616ad 100644 --- a/cli/command/context/options.go +++ b/cli/command/context/options.go @@ -100,7 +100,7 @@ func getDockerEndpoint(contextStore store.Reader, config map[string]string) (doc if ep, ok := metadata.Endpoints[docker.DockerEndpoint].(docker.EndpointMeta); ok { return docker.Endpoint{EndpointMeta: ep}, nil } - return docker.Endpoint{}, fmt.Errorf("unable to get endpoint from context %q", contextName) + return docker.Endpoint{}, fmt.Errorf("context %q has no docker endpoint configured", contextName) } tlsData, err := context.TLSDataFromFiles(config[keyCA], config[keyCert], config[keyKey]) if err != nil { diff --git a/cli/command/image/build.go b/cli/command/image/build.go index db33e0e7c64d..9ab9568dea4c 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -261,7 +261,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) _, _ = fmt.Fprintln(dockerCli.Err(), progBuff) } default: - return fmt.Errorf("unable to prepare context: path %q not found", options.context) + return fmt.Errorf("build context %q is not a supported form: expected '-' for stdin, an existing directory, a Git URL, or an HTTP(S) URL", options.context) } // read from a directory into tar archive @@ -356,7 +356,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) aux := func(msg jsonstream.JSONMessage) { var result buildtypes.Result if err := json.Unmarshal(*msg.Aux, &result); err != nil { - _, _ = fmt.Fprintf(dockerCli.Err(), "Failed to parse aux message: %s", err) + _, _ = fmt.Fprintf(dockerCli.Err(), "could not read image ID from daemon response; the image was built, but --iidfile and -q output will be empty: %s", err) } else { imageID = result.ID } @@ -387,7 +387,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) if options.imageIDFile != "" { if imageID == "" { - return fmt.Errorf("server did not provide an image ID. Cannot write %s", options.imageIDFile) + return fmt.Errorf("image was built, but --iidfile %q was not written: the daemon did not return an image ID", options.imageIDFile) } if err := os.WriteFile(options.imageIDFile, []byte(imageID), 0o666); err != nil { return err diff --git a/cli/command/image/build/context_detect.go b/cli/command/image/build/context_detect.go index c5edc4ecf161..c84ff227fedf 100644 --- a/cli/command/image/build/context_detect.go +++ b/cli/command/image/build/context_detect.go @@ -29,7 +29,7 @@ func DetectContextType(specifiedContext string) (ContextType, error) { case urlutil.IsURL(specifiedContext): return ContextTypeRemote, nil default: - return "", fmt.Errorf("unable to prepare context: path %q not found", specifiedContext) + return "", fmt.Errorf("build context %q is not a supported form: expected '-' for stdin, an existing directory, a Git URL, or an HTTP(S) URL", specifiedContext) } } diff --git a/cli/command/image/list.go b/cli/command/image/list.go index 4f5da98b6323..7a8f77e09d9e 100644 --- a/cli/command/image/list.go +++ b/cli/command/image/list.go @@ -161,7 +161,7 @@ func shouldUseTree(options imagesOptions) (bool, error) { } if options.showDigests { if options.tree { - return false, errors.New("--show-digest is not yet supported with --tree") + return false, errors.New("--digests is not yet supported with --tree") } return false, nil } diff --git a/cli/command/image/load.go b/cli/command/image/load.go index 4fc939466189..f4de185a168f 100644 --- a/cli/command/image/load.go +++ b/cli/command/image/load.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/hint" "github.com/docker/cli/internal/jsonstream" "github.com/moby/moby/client" "github.com/moby/sys/sequential" @@ -61,7 +62,10 @@ func runLoad(ctx context.Context, dockerCli command.Cli, opts loadOptions) error // To avoid getting stuck, verify that a tar file is given either in // the input flag or through stdin and if not display an error message and exit. if dockerCli.In().IsTerminal() { - return errors.New("requested load from stdin, but stdin is empty") + return hint.Wrap( + errors.New("no input given: stdin is a terminal, not a tar archive"), + "Pipe a tar archive into 'docker image load', or pass one with '-i FILE'.", + ) } default: // We use sequential.Open to use sequential file access on Windows, avoiding diff --git a/cli/command/image/load_test.go b/cli/command/image/load_test.go index 9ca0f321d5cf..efd3d2934b03 100644 --- a/cli/command/image/load_test.go +++ b/cli/command/image/load_test.go @@ -30,7 +30,7 @@ func TestNewLoadCommandErrors(t *testing.T) { name: "input-to-terminal", args: []string{}, isTerminalIn: true, - expectedError: "requested load from stdin, but stdin is empty", + expectedError: "stdin is a terminal, not a tar archive", }, { name: "pull-error", diff --git a/cli/command/image/save.go b/cli/command/image/save.go index f900cb1da88b..21659883fa82 100644 --- a/cli/command/image/save.go +++ b/cli/command/image/save.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/hint" "github.com/moby/moby/client" "github.com/moby/sys/atomicwriter" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -70,13 +71,16 @@ func runSave(ctx context.Context, dockerCLI command.Cli, opts saveOptions) error var output io.Writer if opts.output == "" { if dockerCLI.Out().IsTerminal() { - return errors.New("cowardly refusing to save to a terminal. Use the -o flag or redirect") + return hint.Wrap( + errors.New("refusing to write a binary tar archive to the terminal"), + "Use '-o FILE' to write to a file, or redirect stdout, e.g. 'docker image save IMAGE > image.tar'.", + ) } output = dockerCLI.Out() } else { writer, err := atomicwriter.New(opts.output, 0o600) if err != nil { - return fmt.Errorf("failed to save image: %w", err) + return fmt.Errorf("cannot open output file %q: %w", opts.output, err) } defer writer.Close() output = writer diff --git a/cli/command/image/save_test.go b/cli/command/image/save_test.go index dbdad6c32b82..75f18cc9cde7 100644 --- a/cli/command/image/save_test.go +++ b/cli/command/image/save_test.go @@ -30,7 +30,7 @@ func TestNewSaveCommandErrors(t *testing.T) { name: "output to terminal", args: []string{"output", "file", "arg1"}, isTerminal: true, - expectedError: "cowardly refusing to save to a terminal. Use the -o flag or redirect", + expectedError: "refusing to write a binary tar archive to the terminal", }, { name: "ImageSave fail", @@ -44,12 +44,12 @@ func TestNewSaveCommandErrors(t *testing.T) { { name: "output directory does not exist", args: []string{"-o", "fake-dir/out.tar", "arg1"}, - expectedError: `failed to save image: invalid output path: stat fake-dir: no such file or directory`, + expectedError: `cannot open output file "fake-dir/out.tar": invalid output path: stat fake-dir: no such file or directory`, }, { name: "output file is irregular", args: []string{"-o", "/dev/null", "arg1"}, - expectedError: `failed to save image: cannot write to a character device file`, + expectedError: `cannot open output file "/dev/null": cannot write to a character device file`, }, { name: "invalid platform", diff --git a/cli/command/inspect/inspector.go b/cli/command/inspect/inspector.go index 63e7b597ba53..96f5048f7831 100644 --- a/cli/command/inspect/inspector.go +++ b/cli/command/inspect/inspector.go @@ -12,6 +12,7 @@ import ( "text/template" "github.com/docker/cli/cli" + "github.com/docker/cli/internal/hint" "github.com/docker/cli/templates" "github.com/sirupsen/logrus" ) @@ -112,7 +113,7 @@ func (i *TemplateInspector) Inspect(typedElement any, rawElement []byte) error { buffer := new(bytes.Buffer) if err := i.tmpl.Execute(buffer, typedElement); err != nil { if rawElement == nil { - return fmt.Errorf("template parsing error: %w", err) + return fmt.Errorf("--format template failed during execution: %w", err) } return i.tryRawInspectFallback(rawElement) } @@ -136,7 +137,10 @@ func (i *TemplateInspector) tryRawInspectFallback(rawElement []byte) error { tmplMissingKey := i.tmpl.Option("missingkey=error") if err := tmplMissingKey.Execute(buffer, raw); err != nil { - return fmt.Errorf("template parsing error: %w", err) + return hint.Wrap( + fmt.Errorf("--format template failed during execution: %w", err), + "Check that field names referenced in the template match the JSON output of 'docker inspect' without --format.", + ) } i.buffer.Write(buffer.Bytes()) diff --git a/cli/command/inspect/inspector_test.go b/cli/command/inspect/inspector_test.go index 142d50342ba3..1cff00b573b3 100644 --- a/cli/command/inspect/inspector_test.go +++ b/cli/command/inspect/inspector_test.go @@ -62,7 +62,7 @@ func TestTemplateInspectorTemplateError(t *testing.T) { t.Fatal("Expected error got nil") } - if !strings.HasPrefix(err.Error(), "template parsing error") { + if !strings.HasPrefix(err.Error(), "--format template failed during execution") { t.Fatalf("Expected template error, got %v", err) } } @@ -98,7 +98,7 @@ func TestTemplateInspectorRawFallbackError(t *testing.T) { t.Fatal("Expected error got nil") } - if !strings.HasPrefix(err.Error(), "template parsing error") { + if !strings.HasPrefix(err.Error(), "--format template failed during execution") { t.Fatalf("Expected template error, got %v", err) } } diff --git a/cli/command/manifest/annotate.go b/cli/command/manifest/annotate.go index 9bdb7b1d26cc..87f5bc9cf43d 100644 --- a/cli/command/manifest/annotate.go +++ b/cli/command/manifest/annotate.go @@ -14,6 +14,7 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/manifest/store" + "github.com/docker/cli/internal/hint" "github.com/docker/cli/internal/registryclient" "github.com/moby/moby/api/types/registry" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -162,7 +163,10 @@ func runManifestAnnotate(dockerCLI command.Cli, opts annotateOptions) error { } if !isValidOSArch(imageManifest.Descriptor.Platform.OS, imageManifest.Descriptor.Platform.Architecture) { - return fmt.Errorf("manifest entry for image has unsupported os/arch combination: %s/%s", opts.os, opts.arch) + return hint.Wrap( + fmt.Errorf("unsupported os/arch combination %s/%s for manifest entry %s", imageManifest.Descriptor.Platform.OS, imageManifest.Descriptor.Platform.Architecture, opts.image), + "Set --os and --arch to a supported pair (for example linux/amd64, linux/arm64, or windows/amd64).", + ) } return manifestStore.Save(targetRef, imgRef, imageManifest) } diff --git a/cli/command/manifest/annotate_test.go b/cli/command/manifest/annotate_test.go index f2c924e45ffc..6781ce69e30d 100644 --- a/cli/command/manifest/annotate_test.go +++ b/cli/command/manifest/annotate_test.go @@ -63,7 +63,7 @@ func TestManifestAnnotate(t *testing.T) { cmd.Flags().Set("os-version", "1") cmd.Flags().Set("os-features", "feature1") cmd.Flags().Set("variant", "v7") - expectedError = "manifest entry for image has unsupported os/arch combination" + expectedError = "unsupported os/arch combination" assert.ErrorContains(t, cmd.Execute(), expectedError) cmd.Flags().Set("arch", "arm") diff --git a/cli/command/manifest/push.go b/cli/command/manifest/push.go index 024bf6e025a2..99de20450c18 100644 --- a/cli/command/manifest/push.go +++ b/cli/command/manifest/push.go @@ -281,7 +281,10 @@ func mountBlobs(ctx context.Context, client registryclient.RegistryClient, ref r case nil: case registryclient.ErrBlobCreated: if blob.os != "windows" { - return fmt.Errorf("error mounting %s to %s", blob.canonical, ref) + return fmt.Errorf( + "blob %s was uploaded to %s instead of mounted from the source repository; cross-repository blob mount is required for %s manifests: %w", + blob.canonical, ref, blob.os, err, + ) } default: return err diff --git a/cli/command/network/connect.go b/cli/command/network/connect.go index a629005f786b..1c481fbfec4c 100644 --- a/cli/command/network/connect.go +++ b/cli/command/network/connect.go @@ -2,7 +2,7 @@ package network import ( "context" - "errors" + "fmt" "net" "net/netip" "strings" @@ -92,7 +92,7 @@ func convertDriverOpt(options []string) (map[string]string, error) { // TODO(thaJeztah): we should probably not accept whitespace here (both for key and value). k = strings.TrimSpace(k) if !ok || k == "" { - return nil, errors.New("invalid key/value pair format in driver options") + return nil, fmt.Errorf("invalid --driver-opt value %q: expected key=value", opt) } driverOpt[k] = strings.TrimSpace(v) } diff --git a/cli/command/network/create.go b/cli/command/network/create.go index cf676a4cea9b..fde45dab82e2 100644 --- a/cli/command/network/create.go +++ b/cli/command/network/create.go @@ -154,7 +154,7 @@ func createIPAMConfig(options ipamOptions) (*network.IPAM, error) { return nil, err } if ok1 || ok2 { - return nil, errors.New("multiple overlapping subnet configuration is not supported") + return nil, fmt.Errorf("--subnet %s overlaps with --subnet %s", s, k) } } sn, err := netip.ParsePrefix(s) diff --git a/cli/command/network/create_test.go b/cli/command/network/create_test.go index 7de25edb63fa..b14db7a972c6 100644 --- a/cli/command/network/create_test.go +++ b/cli/command/network/create_test.go @@ -71,7 +71,7 @@ func TestNetworkCreateErrors(t *testing.T) { "gateway": "255.0.255.0", // FIXME(thaJeztah): this used to accept a CIDR ("255.0.0.0/24") "subnet": "10.1.2.0/23,10.1.3.248/30", }, - expectedError: "multiple overlapping subnet configuration is not supported", + expectedError: "overlaps with --subnet", }, { args: []string{"toto"}, diff --git a/cli/command/system/info.go b/cli/command/system/info.go index 5ae650a17c6a..a86d77513d3f 100644 --- a/cli/command/system/info.go +++ b/cli/command/system/info.go @@ -209,7 +209,7 @@ func prettyPrintInfo(streams command.Streams, info dockerInfo) error { } if len(info.ServerErrors) > 0 || len(info.ClientErrors) > 0 { - return errors.New("errors pretty printing info") + return errors.New("one or more errors occurred while collecting system info; see ERROR lines above") } return nil } diff --git a/cli/command/system/info_test.go b/cli/command/system/info_test.go index 56d587a69fff..a4d72b9e67a2 100644 --- a/cli/command/system/info_test.go +++ b/cli/command/system/info_test.go @@ -362,7 +362,7 @@ func TestPrettyPrintInfo(t *testing.T) { prettyGolden: "docker-info-errors", jsonGolden: "docker-info-errors", warningsGolden: "docker-info-errors-stderr", - expectedError: "errors pretty printing info", + expectedError: "one or more errors occurred while collecting system info; see ERROR lines above", }, { doc: "bad security info", @@ -374,7 +374,7 @@ func TestPrettyPrintInfo(t *testing.T) { prettyGolden: "docker-info-badsec", jsonGolden: "docker-info-badsec", warningsGolden: "docker-info-badsec-stderr", - expectedError: "errors pretty printing info", + expectedError: "one or more errors occurred while collecting system info; see ERROR lines above", }, { doc: "info with devices", diff --git a/cli/command/volume/update.go b/cli/command/volume/update.go index 0d986051b8a0..40fb98d10612 100644 --- a/cli/command/volume/update.go +++ b/cli/command/volume/update.go @@ -2,7 +2,7 @@ package volume import ( "context" - "errors" + "fmt" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" @@ -52,7 +52,7 @@ func runUpdate(ctx context.Context, dockerCli command.Cli, volumeID, availabilit } if res.Volume.ClusterVolume == nil { - return errors.New("can only update cluster volumes") + return fmt.Errorf("volume %q is not a cluster volume; only cluster volumes can be updated", volumeID) } if flags.Changed("availability") { diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 0efcc70118fe..36f9373139c7 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -24,6 +24,7 @@ import ( cliflags "github.com/docker/cli/cli/flags" "github.com/docker/cli/cli/version" platformsignals "github.com/docker/cli/cmd/docker/internal/signals" + "github.com/docker/cli/internal/hint" "github.com/moby/moby/client/pkg/versions" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -48,6 +49,9 @@ func main() { if err != nil && !errdefs.IsCanceled(err) { if err.Error() != "" { _, _ = fmt.Fprintln(os.Stderr, err) + if h := hint.Of(err); h != "" { + _, _ = fmt.Fprintln(os.Stderr, "\n"+h) + } } os.Exit(getExitCode(err)) } diff --git a/internal/hint/hint.go b/internal/hint/hint.go new file mode 100644 index 000000000000..6d420460c2b7 --- /dev/null +++ b/internal/hint/hint.go @@ -0,0 +1,51 @@ +// Package hint attaches actionable user guidance to errors. +// +// A hint describes what the user can do about a failure ("Run +// 'docker rm foo' to remove it.") and is rendered separately from the +// error's message by the top-level error handler. It is not part of +// [error.Error], so substring matching on the error message stays +// stable as hints are added or reworded. +// +// Use [Wrap] to attach a hint at the call site, and [Of] (or +// [errors.As] against [Hinter]) to extract it for rendering. +package hint + +import "errors" + +// Hinter is implemented by errors that carry actionable user guidance. +type Hinter interface { + Hint() string +} + +type errWithHint struct { + error + hint string +} + +func (e *errWithHint) Hint() string { return e.hint } +func (e *errWithHint) Unwrap() error { return e.error } + +// Wrap attaches actionable guidance to err. It returns nil if err is +// nil. The hint does not appear in the wrapped error's [error.Error] +// output; it is read out of the chain by the top-level renderer via +// [Of] (or [errors.As] against [Hinter]). +func Wrap(err error, hint string) error { + if err == nil { + return nil + } + return &errWithHint{error: err, hint: hint} +} + +// Of returns the first non-empty hint in the error chain, or "" if +// none of the wrapped errors implement [Hinter]. +func Of(err error) string { + for err != nil { + if h, ok := err.(Hinter); ok { + if msg := h.Hint(); msg != "" { + return msg + } + } + err = errors.Unwrap(err) + } + return "" +} diff --git a/internal/hint/hint_test.go b/internal/hint/hint_test.go new file mode 100644 index 000000000000..fd9df61b2dbc --- /dev/null +++ b/internal/hint/hint_test.go @@ -0,0 +1,52 @@ +package hint + +import ( + "errors" + "fmt" + "testing" + + "gotest.tools/v3/assert" +) + +func TestWrap_NilError(t *testing.T) { + assert.Assert(t, Wrap(nil, "irrelevant") == nil) +} + +func TestWrap_PreservesMessage(t *testing.T) { + err := Wrap(errors.New("bad input"), "Try --help.") + assert.Equal(t, err.Error(), "bad input") +} + +func TestWrap_HintReadable(t *testing.T) { + err := Wrap(errors.New("bad input"), "Try --help.") + + var h Hinter + assert.Assert(t, errors.As(err, &h)) + assert.Equal(t, h.Hint(), "Try --help.") +} + +func TestOf_FindsHint(t *testing.T) { + base := Wrap(errors.New("bad input"), "Try --help.") + wrapped := fmt.Errorf("context: %w", base) + + assert.Equal(t, Of(wrapped), "Try --help.") +} + +func TestOf_NoHint(t *testing.T) { + assert.Equal(t, Of(errors.New("plain")), "") + assert.Equal(t, Of(nil), "") +} + +func TestOf_FirstNonEmpty(t *testing.T) { + // outer wrapper has empty hint; inner has content. Of should + // return the first non-empty hint. + inner := Wrap(errors.New("bad input"), "Try --help.") + outer := Wrap(inner, "") + assert.Equal(t, Of(outer), "Try --help.") +} + +func TestUnwrap(t *testing.T) { + base := errors.New("bad input") + err := Wrap(base, "Try --help.") + assert.Assert(t, errors.Is(err, base)) +}