cmd: custom exit codes for internal, resource and canceled errors #3214
Conversation
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
| case codes.Canceled: | ||
| exitCode = 130 |
There was a problem hiding this comment.
ISTR 130 may also be used for containers, but perhaps thats 1 + 129 🤔 https://docs.docker.com/reference/cli/docker/container/attach/#attach-to-and-detach-from-a-running-container
Looking at that square project linked above, that also looks to take 128+ into account for signals; is that a concern?
https://github.com/square/exit/blob/v1.3.0/exit.go#L92-L102
There was a problem hiding this comment.
130 is 128 + SIGINT . Same as you get when you cancel a command in shell.
| case codes.Internal: | ||
| exitCode = 100 // https://github.com/square/exit/blob/v1.3.0/exit.go#L70 |
There was a problem hiding this comment.
The only thing with Internal errors that I wanted to comment on in the ticket (but didn't get round to) is that, at least in moby/moby, but not unlikely "elsewhere", we consider anything that's not explicitly assigned an errdefs type to be an internal server error;
https://github.com/moby/moby/blob/a8d36459572b6d50ab34341bd0c3d62ffa8c3e7b/api/server/httpstatus/status.go#L72
Will it be a problem if errors fall through that were not explicitly given a type to be 1 instead of 100 (or vice-versa)?
There was a problem hiding this comment.
In buildkit most errors are from executing user code or params/config somehow derived from user provided information. So only specific errors can be guaranteed to be internal if they actually represent some algorithm/storage/machine etc. error. What is internal and what is not is defined by BuildKit side so not directly related to this PR.
fix #3174