From 613f4cb847163f875e5a781ff5851b552e742e55 Mon Sep 17 00:00:00 2001 From: lpusok <7979773+lpusok@users.noreply.github.com> Date: Thu, 16 May 2024 12:58:50 +0200 Subject: [PATCH] Return ExitError type (#196) Shove the original command error in the error tree. This allows checking for the specific exit code by code like this: var exitError *exec.ExitError if errors.As(err, &exitError) { Added command.FormattedError type to work around issue of not returning the original exec.ExitError type. The Unwrap() method returns the original error, so the above example will now work. --- command/command.go | 9 ++- command/command_test.go | 15 ++++- command/errors.go | 45 +++++++++++++ errorutil/formatted_error.go | 8 ++- errorutil/formatted_error_test.go | 106 ++++++++++++++++++++++++++++++ 5 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 command/errors.go diff --git a/command/command.go b/command/command.go index 5672ef6..37b650f 100644 --- a/command/command.go +++ b/command/command.go @@ -168,11 +168,14 @@ func printableCommandArgs(isQuoteFirst bool, fullCommandArgs []string) string { func (c command) wrapError(err error) error { var exitErr *exec.ExitError if errors.As(err, &exitErr) { - if c.errorCollector != nil && len(c.errorCollector.errorLines) > 0 { - return fmt.Errorf("command failed with exit status %d (%s): %w", exitErr.ExitCode(), c.PrintableCommandArgs(), errors.New(strings.Join(c.errorCollector.errorLines, "\n"))) + errorLines := []string{} + if c.errorCollector != nil { + errorLines = c.errorCollector.errorLines } - return fmt.Errorf("command failed with exit status %d (%s): %w", exitErr.ExitCode(), c.PrintableCommandArgs(), errors.New("check the command's output for details")) + + return NewExitStatusError(c.PrintableCommandArgs(), exitErr, errorLines) } + return fmt.Errorf("executing command failed (%s): %w", c.PrintableCommandArgs(), err) } diff --git a/command/command_test.go b/command/command_test.go index e53b032..71811f2 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -2,6 +2,7 @@ package command import ( "bytes" + "errors" "fmt" "os/exec" "strings" @@ -65,7 +66,7 @@ Error: fourth error`, gotErrMsg = err.Error() } if gotErrMsg != tt.wantErr { - t.Errorf("command.Run() error = %v, wantErr %v", gotErrMsg, tt.wantErr) + t.Errorf("command.Run() error = \n%v\n, wantErr \n%v\n", gotErrMsg, tt.wantErr) return } }) @@ -123,6 +124,18 @@ func TestRunCmdAndReturnExitCode(t *testing.T) { t.Errorf("command.RunAndReturnExitCode() error = %v, wantErr %v", err, tt.wantErr) return } + if tt.wantErr && tt.wantExitCode > 0 { + var exitErr *exec.ExitError + + if ok := errors.As(err, &exitErr); !ok { + t.Errorf("command.RunAndReturnExitCode() did nor return ExitError type: %s", err) + return + } + + if exitErr.ExitCode() != tt.wantExitCode { + t.Errorf("command.RunAndReturnExitCode() exit code = %v, want %v", exitErr.ExitCode(), tt.wantExitCode) + } + } if gotExitCode != tt.wantExitCode { t.Errorf("command.RunAndReturnExitCode() = %v, want %v", gotExitCode, tt.wantExitCode) } diff --git a/command/errors.go b/command/errors.go new file mode 100644 index 0000000..f88c0f9 --- /dev/null +++ b/command/errors.go @@ -0,0 +1,45 @@ +package command + +import ( + "errors" + "fmt" + "os/exec" + "strings" +) + +// ExitStatusError ... +type ExitStatusError struct { + readableReason error + originalExitErr error +} + +// NewExitStatusError ... +func NewExitStatusError(printableCmdArgs string, exitErr *exec.ExitError, errorLines []string) error { + reasonMsg := fmt.Sprintf("command failed with exit status %d (%s)", exitErr.ExitCode(), printableCmdArgs) + if len(errorLines) == 0 { + return &ExitStatusError{ + readableReason: fmt.Errorf("%s: %w", reasonMsg, errors.New("check the command's output for details")), + originalExitErr: exitErr, + } + } + + return &ExitStatusError{ + readableReason: fmt.Errorf("%s: %w", reasonMsg, errors.New(strings.Join(errorLines, "\n"))), + originalExitErr: exitErr, + } +} + +// Error returns the formatted error message. Does not include the original error message (`exit status 1`). +func (e *ExitStatusError) Error() string { + return e.readableReason.Error() +} + +// Unwrap is needed for errors.Is and errors.As to work correctly. +func (e *ExitStatusError) Unwrap() error { + return e.originalExitErr +} + +// Reason returns the user-friendly error, to be used by errorutil.ErrorFormatter. +func (e *ExitStatusError) Reason() error { + return e.readableReason +} diff --git a/errorutil/formatted_error.go b/errorutil/formatted_error.go index 842c74b..036a069 100644 --- a/errorutil/formatted_error.go +++ b/errorutil/formatted_error.go @@ -3,6 +3,8 @@ package errorutil import ( "errors" "strings" + + "github.com/bitrise-io/go-utils/v2/command" ) // FormattedError ... @@ -13,8 +15,12 @@ func FormattedError(err error) string { for { i++ - reason := err.Error() + // Use the user-friendly error message, ignore the original exec.ExitError. + if commandExitStatusError, ok := err.(*command.ExitStatusError); ok { + err = commandExitStatusError.Reason() + } + reason := err.Error() if err = errors.Unwrap(err); err == nil { formatted = appendError(formatted, reason, i, true) return formatted diff --git a/errorutil/formatted_error_test.go b/errorutil/formatted_error_test.go index dfb9c03..ed2fccf 100644 --- a/errorutil/formatted_error_test.go +++ b/errorutil/formatted_error_test.go @@ -3,7 +3,12 @@ package errorutil import ( "errors" "fmt" + "strings" "testing" + + "github.com/bitrise-io/go-utils/v2/command" + "github.com/bitrise-io/go-utils/v2/env" + "github.com/stretchr/testify/require" ) func TestFormattedError(t *testing.T) { @@ -60,3 +65,104 @@ func TestFormattedError(t *testing.T) { }) } } + +func TestFormattedErrorWithCommand(t *testing.T) { + commandFactory := command.NewFactory(env.NewRepository()) + + tests := []struct { + name string + cmdFn func() error + wantErr string + wantMsg string + }{ + { + name: "command exit status error", + cmdFn: func() error { + cmd := commandFactory.Create("bash", []string{"../command/testdata/exit_with_message.sh"}, nil) + return cmd.Run() + }, + wantErr: `command failed with exit status 1 (bash "../command/testdata/exit_with_message.sh"): check the command's output for details`, + wantMsg: `command failed with exit status 1 (bash "../command/testdata/exit_with_message.sh"): + check the command's output for details`, + }, + { + name: "command execution failed, wrapped", + cmdFn: func() error { + cmd := commandFactory.Create("__notfoundinpath", []string{}, nil) + if err := cmd.Run(); err != nil { + return fmt.Errorf("wrapped: %w", err) + } + return nil + }, + wantErr: `wrapped: executing command failed (__notfoundinpath): exec: "__notfoundinpath": executable file not found in $PATH`, + wantMsg: `wrapped: + executing command failed (__notfoundinpath): + exec: "__notfoundinpath": + executable file not found in $PATH`, + }, + { + name: "command error, wrapped", + cmdFn: func() error { + cmd := commandFactory.Create("bash", []string{"../command/testdata/exit_with_message.sh"}, nil) + if err := cmd.Run(); err != nil { + return fmt.Errorf("wrapped: %w", err) + } + return nil + }, + wantErr: `wrapped: command failed with exit status 1 (bash "../command/testdata/exit_with_message.sh"): check the command's output for details`, + wantMsg: `wrapped: + command failed with exit status 1 (bash "../command/testdata/exit_with_message.sh"): + check the command's output for details`, + }, + { + name: "command with error finder", + cmdFn: func() error { + errorFinder := func(out string) []string { + var errors []string + for _, line := range strings.Split(out, "\n") { + if strings.Contains(line, "Error:") { + errors = append(errors, line) + } + } + return errors + } + + cmd := commandFactory.Create("bash", []string{"../command/testdata/exit_with_message.sh"}, &command.Opts{ + ErrorFinder: errorFinder, + }) + + err := cmd.Run() + return err + }, + wantErr: `command failed with exit status 1 (bash "../command/testdata/exit_with_message.sh"): Error: first error +Error: second error +Error: third error +Error: fourth error`, + wantMsg: `command failed with exit status 1 (bash "../command/testdata/exit_with_message.sh"): + Error: first error + Error: second error + Error: third error + Error: fourth error`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.cmdFn() + + var gotErrMsg string + if err != nil { + gotErrMsg = err.Error() + } + if gotErrMsg != tt.wantErr { + t.Errorf("command.Run() error = \n%v\n, wantErr \n%v\n", gotErrMsg, tt.wantErr) + return + } + + gotFormattedMsg := FormattedError(err) + require.Equal(t, tt.wantMsg, gotFormattedMsg, "FormattedError() error = \n%v\n, wantErr \n%v\n", gotFormattedMsg, tt.wantErr) + if gotFormattedMsg != tt.wantMsg { + t.Errorf("FormattedError() error = \n%v\n, wantErr \n%v\n", gotFormattedMsg, tt.wantErr) + } + }) + } +}