Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
thaJeztah opened this issue Nov 29, 2024 · 3 comments · May be fixed by #5666
Open

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

thaJeztah opened this issue Nov 29, 2024 · 3 comments · May be fixed by #5666
Assignees
@Benehiko
Labels
area/ux kind/enhancement version/27.4

Comments

@thaJeztah
Copy link
Member

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
Copy link
Member

Benehiko commented Dec 2, 2024

@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
Copy link
Member

Benehiko commented Dec 2, 2024

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
Copy link
Member Author

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;

cli/vendor/github.com/docker/docker/errdefs/is.go

Lines 120 to 123 in 5afa739

// IsContext returns if the passed in error is due to context cancellation or deadline exceeded.
func IsContext(err error) bool {
return errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)
}

@Benehiko Benehiko linked a pull request Dec 3, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Benehiko Benehiko

Labels
area/ux kind/enhancement version/27.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cli/command: ctx cancel should not print or produce a non zero exit code Benehiko/docker-cli
2 participants
@thaJeztah @Benehiko