don't print "context canceled" errors when canceling an action (CTRL-C)

thaJeztah commented 4 days ago

Description

Noticed this on docker 27.4.0-rc.3; when canceling the CLI, we print the context canceled message; this is unlikely informative to the user, and we might as well

docker pull golang
Using default tag: latest
latest: Pulling from library/golang
b2b31b28ee3c: Downloading [>                                                  ]  507.1kB/49.58MB
c3cc7b6f0473: Downloading [>                                                  ]    245kB/24.06MB
2112e5e7c3ff: Downloading [>                                                  ]  526.6kB/64.39MB
b59585393ce6: Waiting
c79bddf330f7: Waiting
fe407d04300b: Waiting
4f4fb700ef54: Waiting
^Ccontext canceled

docker run -it --rm golang
Unable to find image 'golang:latest' locally
latest: Pulling from library/golang
b2b31b28ee3c: Downloading [>                                                  ]  507.2kB/49.58MB
c3cc7b6f0473: Downloading [================>                                  ]  8.141MB/24.06MB
2112e5e7c3ff: Downloading [=======>                                           ]  9.095MB/64.39MB
b59585393ce6: Waiting
c79bddf330f7: Waiting
fe407d04300b: Waiting
4f4fb700ef54: Waiting
^Cdocker: context canceled.
See 'docker run --help'.

echo $?
125

I also noticed that the exit-code is non-zero in these situations, but I'm not sure what the correct thing to do is there. For example, tail also exits with a non-zero exit-code, although it uses a specific code for it (130);

docker logs -f foo
...
94.210.180.92 - - [29/Nov/2024:18:58:03 +0000] "GET / HTTP/1.1" 200 615 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Safari/605.1.15" "-"
94.210.180.92 - - [29/Nov/2024:18:58:03 +0000] "GET / HTTP/1.1" 200 615 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Safari/605.1.15" "-"
94.210.180.92 - - [29/Nov/2024:18:58:03 +0000] "GET / HTTP/1.1" 200 615 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Safari/605.1.15" "-"
^Ccontext canceled

echo $?
1

tail -f /var/log/syslog
...
2024-11-29T19:00:18.949712+00:00 ubuntu-s-1vcpu-1gb-ams3-01 systemd[1]: Starting sysstat-collect.service - system activity accounting tool...
2024-11-29T19:00:18.962572+00:00 ubuntu-s-1vcpu-1gb-ams3-01 systemd[1]: sysstat-collect.service: Deactivated successfully.
2024-11-29T19:00:18.963116+00:00 ubuntu-s-1vcpu-1gb-ams3-01 systemd[1]: Finished sysstat-collect.service - system activity accounting tool.
^C
echo $?
130

Benehiko commented 1 day ago

@thaJeztah do you think we should omit the help output as well? So that no error is printed in the case of context.Cancelled?

Benehiko commented 1 day ago

We wrap the error for run with the withHelp function. https://github.com/docker/cli/blob/master/cli/command/container/run.go#L299

Regarding status codes, 125 is the default for the toStatusError function which is only run sometimes depending on the code path used inside the runContainer function https://github.com/docker/cli/blob/master/cli/command/container/run.go#L311

thaJeztah commented 1 day ago

do you think we should omit the help output as well? So that no error is printed in the case of context.Cancelled?

I'm leaning towards "yes" there; assuming we know that context.Cancelled would always be triggered by a user-action (canceling the CLI). The --help output was meant for cases where a user used the CLI incorrectly (e.g. invalid / unknown flag etc), which isn't the case here.

Not sure if we can also run into context timeout cases (which may still be something we should print?), or if this means we could use the helper utility that's in errdefs; https://github.com/docker/cli/blob/5afa739692fbf3c96f063d3f822f27f48dad3f6a/vendor/github.com/docker/docker/errdefs/is.go#L120-L123

docker / cli

don't print "context canceled" errors when canceling an action (CTRL-C) #5659

Description