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.
bitrise-io · May 16, 2024 · 613f4cb · 613f4cb
1 parent 9606561
commit 613f4cb
Show file tree

Hide file tree

Showing 5 changed files with 178 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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)
+			}
+		})
+	}
+}