Skip to content

Commit 1f0f097

Browse files
dvdksnclaude
andcommitted
cli/command: rewrite user-facing error messages flagged by the audit
Applies the rewrites produced by an end-to-end review of CLI error quality (audit/{inventory,review,rewrites,editorial}.json). 31 messages across 11 command areas, organised around the patterns that surfaced most often: Factual / wrong-noun fixes — messages that misdirected users to the wrong layer or named things that didn't exist: - image/list: --show-digest -> --digests (the actual flag name) - image/load: stdin-state assertion now matches IsTerminal() guard - inspect: template execution failures no longer mislabelled as parsing errors - system/info: blames the actual cause (info collection) not pretty-printing - image/save, container/export: -o open failures name the file, not the operation - image/build: build-context errors enumerate supported forms instead of misnaming everything as a missing path - manifest/annotate: prints the os/arch pair actually validated - context/options: describes the missing-endpoint state rather than implying a fetch failure - container/cp: corrects the misleading 'container source' wording Partial success surfaced — operations that succeeded with a follow-up failure now say so, preventing wasted retries: - container/create: cidfile write failure (container exists) - image/build: aux-message decode failure (build succeeded) - image/build: missing image ID for --iidfile (build succeeded) Refusal reason explained — TTY-refusal messages for binary tar output now state why instead of using 'cowardly': - container/export, context/export, image/save Echo-the-value / name-the-flag — validation errors now include what the user typed and the expected form: - container/opts: --pid, --uts, --userns, --cgroupns, --storage-opt, --device (×2 branches), --log-driver - network/connect: --driver-opt - network/create: --subnet (names both overlapping subnets) - volume/update, container/update, context/create Internal jargon replaced — code-level field names removed from user-facing wording: - container/exec: 'exec ID empty' replaced with daemon-anomaly framing Cause preserved — manifest/push: ErrBlobCreated is now wrapped via %w and the wording describes what actually happened. Errors that have an actionable hint use internal/hint.Wrap so the hint is rendered separately from the message at the top-level error handler. Errors that benefit from a sibling-site fix (image/build's context detection in image/build/context_detect.go) carry that fix to keep the same condition producing the same wording regardless of entry point. Tests updated to match the new wordings; substring-based assertions stay stable because hints are no longer part of err.Error(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
1 parent d096b7d commit 1f0f097

31 files changed

Lines changed: 119 additions & 61 deletions

cli/command/container/cp.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/docker/cli/cli"
1717
"github.com/docker/cli/cli/command"
1818
"github.com/docker/cli/cli/streams"
19+
"github.com/docker/cli/internal/hint"
1920
"github.com/docker/go-units"
2021
"github.com/moby/go-archive"
2122
"github.com/moby/moby/client"
@@ -242,7 +243,10 @@ func runCopy(ctx context.Context, dockerCli command.Cli, opts copyOptions) error
242243
case acrossContainers:
243244
return errors.New("copying between containers is not supported")
244245
default:
245-
return errors.New("must specify at least one container source")
246+
return hint.Wrap(
247+
errors.New("one argument must reference a container as 'CONTAINER:PATH'"),
248+
"Use 'docker cp CONTAINER:SRC LOCAL' to copy from a container, or 'docker cp LOCAL CONTAINER:DEST' to copy into one.",
249+
)
246250
}
247251
}
248252

cli/command/container/cp_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TestRunCopyWithInvalidArguments(t *testing.T) {
3838
source: "./source",
3939
destination: "./dest",
4040
},
41-
expectedErr: "must specify at least one container source",
41+
expectedErr: "one argument must reference a container as 'CONTAINER:PATH'",
4242
},
4343
}
4444
for _, testcase := range testcases {

cli/command/container/create.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/docker/cli/cli/config/configfile"
2121
"github.com/docker/cli/cli/config/types"
2222
"github.com/docker/cli/cli/streams"
23+
"github.com/docker/cli/internal/hint"
2324
"github.com/docker/cli/internal/jsonstream"
2425
"github.com/docker/cli/opts"
2526
"github.com/moby/moby/api/types/mount"
@@ -188,7 +189,10 @@ func (cid *cidFile) Write(id string) error {
188189
return nil
189190
}
190191
if _, err := cid.file.WriteString(id); err != nil {
191-
return fmt.Errorf("failed to write the container ID (%s) to file: %w", id, err)
192+
return hint.Wrap(
193+
fmt.Errorf("container %s was created, but writing its ID to %s failed: %w", id, cid.path, err),
194+
fmt.Sprintf("The container exists on the daemon. Run 'docker rm %s' to remove it, or note the ID for later use.", id),
195+
)
192196
}
193197
cid.written = true
194198
return nil

cli/command/container/exec.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/docker/cli/cli/command"
1111
"github.com/docker/cli/cli/command/completion"
1212
"github.com/docker/cli/cli/config/configfile"
13+
"github.com/docker/cli/internal/hint"
1314
"github.com/docker/cli/opts"
1415
"github.com/moby/moby/api/types/container"
1516
"github.com/moby/moby/client"
@@ -115,7 +116,10 @@ func RunExec(ctx context.Context, dockerCLI command.Cli, containerIDorName strin
115116

116117
execID := response.ID
117118
if execID == "" {
118-
return errors.New("exec ID empty")
119+
return hint.Wrap(
120+
errors.New("the Docker daemon returned an empty response when creating the exec session"),
121+
"This is unexpected — please report it at https://github.com/moby/moby/issues with the daemon version ('docker version') and the command you ran.",
122+
)
119123
}
120124

121125
if options.Detach {

cli/command/container/exec_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func TestRunExec(t *testing.T) {
195195
{
196196
doc: "missing exec ID",
197197
options: NewExecOptions(),
198-
expectedError: "exec ID empty",
198+
expectedError: "the Docker daemon returned an empty response when creating the exec session",
199199
client: &fakeClient{},
200200
},
201201
}

cli/command/container/export.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/docker/cli/cli"
1010
"github.com/docker/cli/cli/command"
1111
"github.com/docker/cli/cli/command/completion"
12+
"github.com/docker/cli/internal/hint"
1213
"github.com/moby/moby/client"
1314
"github.com/moby/sys/atomicwriter"
1415
"github.com/spf13/cobra"
@@ -49,13 +50,16 @@ func runExport(ctx context.Context, dockerCLI command.Cli, opts exportOptions) e
4950
var output io.Writer
5051
if opts.output == "" {
5152
if dockerCLI.Out().IsTerminal() {
52-
return errors.New("cowardly refusing to save to a terminal. Use the -o flag or redirect")
53+
return hint.Wrap(
54+
errors.New("refusing to write a binary tar archive to the terminal"),
55+
"Use '-o FILE' to write to a file, or redirect stdout, e.g. 'docker container export CONTAINER > out.tar'.",
56+
)
5357
}
5458
output = dockerCLI.Out()
5559
} else {
5660
writer, err := atomicwriter.New(opts.output, 0o600)
5761
if err != nil {
58-
return fmt.Errorf("failed to export container: %w", err)
62+
return fmt.Errorf("cannot open output file %q: %w", opts.output, err)
5963
}
6064
defer writer.Close()
6165
output = writer

cli/command/container/export_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,6 @@ func TestContainerExportOutputToIrregularFile(t *testing.T) {
4444
cmd.SetErr(io.Discard)
4545
cmd.SetArgs([]string{"-o", "/dev/random", "container"})
4646

47-
const expected = `failed to export container: cannot write to a character device file`
47+
const expected = `cannot open output file "/dev/random": cannot write to a character device file`
4848
assert.Error(t, cmd.Execute(), expected)
4949
}

cli/command/container/opts.go

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"strings"
1919
"time"
2020

21+
"github.com/docker/cli/internal/hint"
2122
"github.com/docker/cli/internal/lazyregexp"
2223
"github.com/docker/cli/internal/volumespec"
2324
"github.com/docker/cli/opts"
@@ -517,22 +518,34 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
517518

518519
pidMode := container.PidMode(copts.pidMode)
519520
if !pidMode.Valid() {
520-
return nil, errors.New("--pid: invalid PID mode")
521+
return nil, hint.Wrap(
522+
fmt.Errorf("invalid --pid mode %q", copts.pidMode),
523+
"Valid forms are 'host' or 'container:<name|id>'.",
524+
)
521525
}
522526

523527
utsMode := container.UTSMode(copts.utsMode)
524528
if !utsMode.Valid() {
525-
return nil, errors.New("--uts: invalid UTS mode")
529+
return nil, hint.Wrap(
530+
fmt.Errorf("invalid --uts mode %q", copts.utsMode),
531+
"The only valid form is 'host'.",
532+
)
526533
}
527534

528535
usernsMode := container.UsernsMode(copts.usernsMode)
529536
if !usernsMode.Valid() {
530-
return nil, errors.New("--userns: invalid USER mode")
537+
return nil, hint.Wrap(
538+
fmt.Errorf("invalid --userns mode %q", copts.usernsMode),
539+
"The only valid form is 'host'.",
540+
)
531541
}
532542

533543
cgroupnsMode := container.CgroupnsMode(copts.cgroupnsMode)
534544
if !cgroupnsMode.Valid() {
535-
return nil, errors.New("--cgroupns: invalid CGROUP mode")
545+
return nil, hint.Wrap(
546+
fmt.Errorf("invalid --cgroupns mode %q", copts.cgroupnsMode),
547+
"Valid forms are 'private' or 'host'.",
548+
)
536549
}
537550

538551
restartPolicy, err := opts.ParseRestartPolicy(copts.restartPolicy)
@@ -920,7 +933,10 @@ func convertToStandardNotation(ports []string) ([]string, error) {
920933
func parseLoggingOpts(loggingDriver string, loggingOpts []string) (map[string]string, error) {
921934
loggingOptsMap := opts.ConvertKVStringsToMap(loggingOpts)
922935
if loggingDriver == "none" && len(loggingOpts) > 0 {
923-
return map[string]string{}, fmt.Errorf("invalid logging opts for driver %s", loggingDriver)
936+
return map[string]string{}, hint.Wrap(
937+
errors.New("log driver \"none\" accepts no --log-opt entries"),
938+
"Remove the --log-opt flag(s) or choose a different --log-driver.",
939+
)
924940
}
925941
return loggingOptsMap, nil
926942
}
@@ -984,7 +1000,7 @@ func parseStorageOpts(storageOpts []string) (map[string]string, error) {
9841000
for _, option := range storageOpts {
9851001
k, v, ok := strings.Cut(option, "=")
9861002
if !ok {
987-
return nil, errors.New("invalid storage option")
1003+
return nil, fmt.Errorf("invalid --storage-opt value %q: expected key=value", option)
9881004
}
9891005
m[k] = v
9901006
}
@@ -1095,12 +1111,12 @@ func validateLinuxPath(val string, validator func(string) bool) (string, error)
10951111
var mode string
10961112

10971113
if strings.Count(val, ":") > 2 {
1098-
return val, fmt.Errorf("bad format for path: %s", val)
1114+
return val, fmt.Errorf("invalid --device %q: too many ':' separators, expected [host-path:]container-path[:mode]", val)
10991115
}
11001116

11011117
split := strings.SplitN(val, ":", 3)
11021118
if split[0] == "" {
1103-
return val, fmt.Errorf("bad format for path: %s", val)
1119+
return val, fmt.Errorf("invalid --device %q: host path before ':' is empty, expected [host-path:]container-path[:mode]", val)
11041120
}
11051121
switch len(split) {
11061122
case 1:

cli/command/container/opts_test.go

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ func TestParseModes(t *testing.T) {
767767
args := []string{"--pid=container:", "img", "cmd"}
768768
assert.NilError(t, flags.Parse(args))
769769
_, err := parse(flags, copts, runtime.GOOS)
770-
assert.ErrorContains(t, err, "--pid: invalid PID mode")
770+
assert.ErrorContains(t, err, "invalid --pid mode")
771771

772772
// pid ok
773773
_, hostconfig, _, err := parseRun([]string{"--pid=host", "img", "cmd"})
@@ -778,7 +778,7 @@ func TestParseModes(t *testing.T) {
778778

779779
// uts ko
780780
_, _, _, err = parseRun([]string{"--uts=container:", "img", "cmd"}) //nolint:dogsled
781-
assert.ErrorContains(t, err, "--uts: invalid UTS mode")
781+
assert.ErrorContains(t, err, "invalid --uts mode")
782782

783783
// uts ok
784784
_, hostconfig, _, err = parseRun([]string{"--uts=host", "img", "cmd"})
@@ -923,8 +923,8 @@ func TestParseHealth(t *testing.T) {
923923

924924
func TestParseLoggingOpts(t *testing.T) {
925925
// logging opts ko
926-
if _, _, _, err := parseRun([]string{"--log-driver=none", "--log-opt=anything", "img", "cmd"}); err == nil || err.Error() != "invalid logging opts for driver none" {
927-
t.Fatalf("Expected an error with message 'invalid logging opts for driver none', got %v", err)
926+
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") {
927+
t.Fatalf("Expected an error stating that 'none' accepts no --log-opt entries, got %v", err)
928928
}
929929
// logging opts ok
930930
_, hostconfig, _, err := parseRun([]string{"--log-driver=syslog", "--log-opt=something", "img", "cmd"})
@@ -1034,22 +1034,24 @@ func TestValidateDevice(t *testing.T) {
10341034
"/hostPath:/containerPath:rw",
10351035
"/hostPath:/containerPath:mrw",
10361036
}
1037+
const emptyHostMsg = `: host path before ':' is empty, expected [host-path:]container-path[:mode]`
1038+
const tooManyMsg = `: too many ':' separators, expected [host-path:]container-path[:mode]`
10371039
invalid := map[string]string{
1038-
"": "bad format for path: ",
1040+
"": `invalid --device ""` + emptyHostMsg,
10391041
"./": "./ is not an absolute path",
10401042
"../": "../ is not an absolute path",
10411043
"/:../": "../ is not an absolute path",
10421044
"/:path": "path is not an absolute path",
1043-
":": "bad format for path: :",
1045+
":": `invalid --device ":"` + emptyHostMsg,
10441046
"/tmp:": " is not an absolute path",
1045-
":test": "bad format for path: :test",
1046-
":/test": "bad format for path: :/test",
1047+
":test": `invalid --device ":test"` + emptyHostMsg,
1048+
":/test": `invalid --device ":/test"` + emptyHostMsg,
10471049
"tmp:": " is not an absolute path",
1048-
":test:": "bad format for path: :test:",
1049-
"::": "bad format for path: ::",
1050-
":::": "bad format for path: :::",
1051-
"/tmp:::": "bad format for path: /tmp:::",
1052-
":/tmp::": "bad format for path: :/tmp::",
1050+
":test:": `invalid --device ":test:"` + emptyHostMsg,
1051+
"::": `invalid --device "::"` + emptyHostMsg,
1052+
":::": `invalid --device ":::"` + tooManyMsg,
1053+
"/tmp:::": `invalid --device "/tmp:::"` + tooManyMsg,
1054+
":/tmp::": `invalid --device ":/tmp::"` + tooManyMsg,
10531055
"path:ro": "ro is not an absolute path",
10541056
"path:rr": "rr is not an absolute path",
10551057
"a:/b:ro": "bad mode specified: ro",

cli/command/container/update.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/docker/cli/cli"
1010
"github.com/docker/cli/cli/command"
1111
"github.com/docker/cli/cli/command/completion"
12+
"github.com/docker/cli/internal/hint"
1213
"github.com/docker/cli/opts"
1314
containertypes "github.com/moby/moby/api/types/container"
1415
"github.com/moby/moby/client"
@@ -92,7 +93,10 @@ func runUpdate(ctx context.Context, dockerCli command.Cli, options *updateOption
9293
var err error
9394

9495
if options.nFlag == 0 {
95-
return errors.New("you must provide one or more flags when using this command")
96+
return hint.Wrap(
97+
errors.New("no resource flags supplied — nothing to update"),
98+
"Pass at least one tunable flag (for example --memory, --cpus, --restart). Run 'docker container update --help' for the full list.",
99+
)
96100
}
97101

98102
var restartPolicy containertypes.RestartPolicy

0 commit comments

Comments
 (0)