main.go: print ExitError before exiting #4168
Conversation
AkihiroSuda
force-pushed
the
fix-4167
branch
2 times, most recently
from
408f507 to
a1edf08
Compare
Fix issue 4167 Signed-off-by: Akihiro Suda <[email protected]>
There was a problem hiding this comment.
I think this change is wrong. You should return a non-ExitError if you want limactl to print it. Getting a FATAL error just because a subprocess exited with a non-zero code is annoying. HandleExitError() exists to prevent it from being printed while all other errors still show up.
Now we are getting this:
❯ cat ~/bin/limactl-false
#!/bin/sh
false
❯ limactl false
FATA[0000] external command "/Users/jan/bin/limactl-false" failed error="exit status 1"
❯ echo $?
1There should be no output except when the error is internal to limactl itself. Those errors have always been reported (this is from master):
❯ echo false > ~/bin/limactl-nohashbang
❯ chmod +x ~/bin/limactl-nohashbang
❯ limactl-nohashbang
❯ echo $?
1
❯ limactl nohashbang
FATA[0000] external command "/Users/jan/bin/limactl-nohashbang" failed: fork/exec /Users/jan/bin/limactl-nohashbang: exec format error| ) | ||
|
|
||
| func HandleExitError(err error) { | ||
| func HandleExitError(err error) int { |
There was a problem hiding this comment.
The name is now misleading because it just returns the exit error and doesn't handle it.
| func HandleExitError(err error) int { | |
| func ExitError(err error) int { |
| } | ||
|
|
||
| var exitErr *exec.ExitError | ||
| if errors.As(err, &exitErr) { |
There was a problem hiding this comment.
I think the right fix would be to change from errors.As back to a type assertion, so it doesn't match on wrapped ExitErrors (which is what networks.sudo() is returning on failure).
This is a regression that I introduced in #3938. Add a comment to make sure it isn't being changed again.
We just don't want to print a message on "naked" ExitErrors that are directly returned by cmd.Run etc. because then propagating the exit code is enough.
Fix #4167