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.

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

exec: report cmd.Run() error; e.g. command not found. #30

Merged
merged 1 commit into from
Oct 6, 2015

Conversation

pda
Copy link
Collaborator

@pda pda commented Oct 5, 2015

Capture and pass the error from cmd.Run() to ui.Error.Fatal() so that failed commands are reported and exit with a non-zero status.

Before:

$ aws-vault exec 99designs true ; echo $?
0

$ aws-vault exec 99designs false ; echo $?
0

$ aws-vault exec 99designs blargh ; echo $?
0

After:

$ aws-vault exec 99designs true ; echo $?
0

$ aws-vault exec 99designs false ; echo $?
exit status 1
1

$ aws-vault exec 99designs blargh ; echo $?
exec: "blargh": executable file not found in $PATH
1

This needs fine-tuning so that the second case silently passes through the exit status, while the third case still reports that the command wasn't found. But for now this fixes the much bigger problem of failing to propagate errors at all.

Capture and pass the error from `cmd.Run()` to `ui.Error.Fatal()` so that
failed commands are reported and exit with a non-zero status.

Before:

```
$ aws-vault exec 99designs true ; echo $?
0

$ aws-vault exec 99designs false ; echo $?
0

$ aws-vault exec 99designs blargh ; echo $?
0
```

After:

```
$ aws-vault exec 99designs true ; echo $?
0

$ aws-vault exec 99designs false ; echo $?
exit status 1
1

$ aws-vault exec 99designs blargh ; echo $?
exec: "blargh": executable file not found in $PATH
1
```
lox added a commit that referenced this pull request Oct 6, 2015
exec: report cmd.Run() error; e.g. command not found.
@lox lox merged commit 1dad960 into master Oct 6, 2015
@lox lox deleted the propagate-exec-error branch October 6, 2015 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants