[CI-267] Command errors (#174)

tothszabi · godrei · web-flow · commit d00ca1ae660e · 2022-12-09T09:32:29.000Z
* Update command errors

* Add nil dereference prevention

* Extend error wrapping to all of the command run functions

* Update one of the test cases to be linux compatible

* Update the rest of the test cases to be linux friendly

* Use ont the fly error collection

* Unexport method

* Fix test

* Add a default instruction

Co-authored-by: Krisztián Gödrei &lt;krisztian.godrei@bitrise.io&gt;
diff --git a/command/command.go b/command/command.go
@@ -1,6 +1,8 @@
 package command
 
 import (
+	"errors"
+	"fmt"
 	"io"
 	"os/exec"
 	"strconv"
@@ -9,13 +11,17 @@ import (
 	"github.com/bitrise-io/go-utils/v2/env"
 )
 
+// ErrorFinder ...
+type ErrorFinder func(out string) []string
+
 // Opts ...
 type Opts struct {
-	Stdout io.Writer
-	Stderr io.Writer
-	Stdin  io.Reader
-	Env    []string
-	Dir    string
+	Stdout      io.Writer
+	Stderr      io.Writer
+	Stdin       io.Reader
+	Env         []string
+	Dir         string
+	ErrorFinder ErrorFinder
 }
 
 // Factory ...
@@ -35,7 +41,13 @@ func NewFactory(envRepository env.Repository) Factory {
 // Create ...
 func (f factory) Create(name string, args []string, opts *Opts) Command {
 	cmd := exec.Command(name, args...)
+	var collector *errorCollector
+
 	if opts != nil {
+		if opts.ErrorFinder != nil {
+			collector = &errorCollector{errorFinder: opts.ErrorFinder}
+		}
+
 		cmd.Stdout = opts.Stdout
 		cmd.Stderr = opts.Stderr
 		cmd.Stdin = opts.Stdin
@@ -47,7 +59,10 @@ func (f factory) Create(name string, args []string, opts *Opts) Command {
 		cmd.Env = append(f.envRepository.List(), opts.Env...)
 		cmd.Dir = opts.Dir
 	}
-	return command{cmd}
+	return &command{
+		cmd:            cmd,
+		errorCollector: collector,
+	}
 }
 
 // Command ...
@@ -62,7 +77,8 @@ type Command interface {
 }
 
 type command struct {
-	cmd *exec.Cmd
+	cmd            *exec.Cmd
+	errorCollector *errorCollector
 }
 
 // PrintableCommandArgs ...
@@ -71,13 +87,24 @@ func (c command) PrintableCommandArgs() string {
 }
 
 // Run ...
-func (c command) Run() error {
-	return c.cmd.Run()
+func (c *command) Run() error {
+	c.wrapOutputs()
+
+	if err := c.cmd.Run(); err != nil {
+		return c.wrapError(err)
+	}
+
+	return nil
 }
 
 // RunAndReturnExitCode ...
 func (c command) RunAndReturnExitCode() (int, error) {
+	c.wrapOutputs()
 	err := c.cmd.Run()
+	if err != nil {
+		err = c.wrapError(err)
+	}
+
 	exitCode := c.cmd.ProcessState.ExitCode()
 	return exitCode, err
 }
@@ -86,24 +113,44 @@ func (c command) RunAndReturnExitCode() (int, error) {
 func (c command) RunAndReturnTrimmedOutput() (string, error) {
 	outBytes, err := c.cmd.Output()
 	outStr := string(outBytes)
+	if err != nil {
+		if c.errorCollector != nil {
+			c.errorCollector.collectErrors(outStr)
+		}
+		err = c.wrapError(err)
+	}
+
 	return strings.TrimSpace(outStr), err
 }
 
 // RunAndReturnTrimmedCombinedOutput ...
 func (c command) RunAndReturnTrimmedCombinedOutput() (string, error) {
 	outBytes, err := c.cmd.CombinedOutput()
 	outStr := string(outBytes)
+	if err != nil {
+		if c.errorCollector != nil {
+			c.errorCollector.collectErrors(outStr)
+		}
+		err = c.wrapError(err)
+	}
+
 	return strings.TrimSpace(outStr), err
 }
 
 // Start ...
 func (c command) Start() error {
+	c.wrapOutputs()
 	return c.cmd.Start()
 }
 
 // Wait ...
 func (c command) Wait() error {
-	return c.cmd.Wait()
+	err := c.cmd.Wait()
+	if err != nil {
+		err = c.wrapError(err)
+	}
+
+	return err
 }
 
 func printableCommandArgs(isQuoteFirst bool, fullCommandArgs []string) string {
@@ -118,3 +165,34 @@ func printableCommandArgs(isQuoteFirst bool, fullCommandArgs []string) string {
 
 	return strings.Join(cmdArgsDecorated, " ")
 }
+
+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")))
+		}
+		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 fmt.Errorf("executing command failed (%s): %w", c.PrintableCommandArgs(), err)
+}
+
+func (c command) wrapOutputs() {
+	if c.errorCollector == nil {
+		return
+	}
+
+	if c.cmd.Stdout != nil {
+		outWriter := io.MultiWriter(c.errorCollector, c.cmd.Stdout)
+		c.cmd.Stdout = outWriter
+	} else {
+		c.cmd.Stdout = c.errorCollector
+	}
+
+	if c.cmd.Stderr != nil {
+		errWriter := io.MultiWriter(c.errorCollector, c.cmd.Stderr)
+		c.cmd.Stderr = errWriter
+	} else {
+		c.cmd.Stderr = c.errorCollector
+	}
+}
diff --git a/command/command_test.go b/command/command_test.go
@@ -1,10 +1,75 @@
 package command
 
 import (
-	"github.com/bitrise-io/go-utils/v2/env"
+	"bytes"
+	"os/exec"
+	"strings"
 	"testing"
+
+	"github.com/bitrise-io/go-utils/v2/env"
 )
 
+func TestRunErrors(t *testing.T) {
+	tests := []struct {
+		name    string
+		cmd     command
+		wantErr string
+	}{
+		{
+			name:    "command without stdout set",
+			cmd:     command{cmd: exec.Command("bash", "testdata/exit_with_message.sh")},
+			wantErr: `command failed with exit status 1 (bash "testdata/exit_with_message.sh"): check the command's output for details`,
+		},
+		{
+			name: "command with stdout set",
+			cmd: func() command {
+				c := exec.Command("bash", "testdata/exit_with_message.sh")
+				var out bytes.Buffer
+				c.Stdout = &out
+				return command{cmd: c}
+			}(),
+			wantErr: `command failed with exit status 1 (bash "testdata/exit_with_message.sh"): check the command's output for details`,
+		},
+		{
+			name: "command with error finder",
+			cmd: func() command {
+				c := exec.Command("bash", "testdata/exit_with_message.sh")
+				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
+				}
+
+				return command{
+					cmd:            c,
+					errorCollector: &errorCollector{errorFinder: errorFinder},
+				}
+			}(),
+			wantErr: `command failed with exit status 1 (bash "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.cmd.Run()
+			var gotErrMsg string
+			if err != nil {
+				gotErrMsg = err.Error()
+			}
+			if gotErrMsg != tt.wantErr {
+				t.Errorf("command.Run() error = %v, wantErr %v", gotErrMsg, tt.wantErr)
+				return
+			}
+		})
+	}
+}
+
 func TestRunCmdAndReturnExitCode(t *testing.T) {
 	type args struct {
 		cmd Command
@@ -62,3 +127,113 @@ func TestRunCmdAndReturnExitCode(t *testing.T) {
 		})
 	}
 }
+
+func TestRunAndReturnTrimmedOutput(t *testing.T) {
+	tests := []struct {
+		name    string
+		cmd     command
+		wantErr string
+	}{
+		{
+			name: "command without error finder",
+			cmd: func() command {
+				c := exec.Command("bash", "testdata/exit_with_message.sh")
+				return command{
+					cmd: c,
+				}
+			}(),
+			wantErr: "command failed with exit status 1 (bash \"testdata/exit_with_message.sh\"): check the command's output for details",
+		},
+		{
+			name: "command with error finder",
+			cmd: func() command {
+				c := exec.Command("bash", "testdata/exit_with_message.sh")
+				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
+				}
+
+				return command{
+					cmd:            c,
+					errorCollector: &errorCollector{errorFinder: errorFinder},
+				}
+			}(),
+			wantErr: `command failed with exit status 1 (bash "testdata/exit_with_message.sh"): Error: first error
+Error: second error`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := tt.cmd.RunAndReturnTrimmedOutput()
+			var gotErrMsg string
+			if err != nil {
+				gotErrMsg = err.Error()
+			}
+			if gotErrMsg != tt.wantErr {
+				t.Errorf("command.Run() error = %v, wantErr %v", gotErrMsg, tt.wantErr)
+				return
+			}
+		})
+	}
+}
+
+func TestRunAndReturnTrimmedCombinedOutput(t *testing.T) {
+	tests := []struct {
+		name    string
+		cmd     command
+		wantErr string
+	}{
+		{
+			name: "command without error finder",
+			cmd: func() command {
+				c := exec.Command("bash", "testdata/exit_with_message.sh")
+				return command{
+					cmd: c,
+				}
+			}(),
+			wantErr: "command failed with exit status 1 (bash \"testdata/exit_with_message.sh\"): check the command's output for details",
+		},
+		{
+			name: "command with error finder",
+			cmd: func() command {
+				c := exec.Command("bash", "testdata/exit_with_message.sh")
+				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
+				}
+
+				return command{
+					cmd:            c,
+					errorCollector: &errorCollector{errorFinder: errorFinder},
+				}
+			}(),
+			wantErr: `command failed with exit status 1 (bash "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.cmd.RunAndReturnTrimmedCombinedOutput()
+			var gotErrMsg string
+			if err != nil {
+				gotErrMsg = err.Error()
+			}
+			if gotErrMsg != tt.wantErr {
+				t.Errorf("command.Run() error = %v, wantErr %v", gotErrMsg, tt.wantErr)
+				return
+			}
+		})
+	}
+}
diff --git a/command/errorcollector.go b/command/errorcollector.go
@@ -0,0 +1,18 @@
+package command
+
+type errorCollector struct {
+	errorLines  []string
+	errorFinder ErrorFinder
+}
+
+func (e *errorCollector) Write(p []byte) (n int, err error) {
+	e.collectErrors(string(p))
+	return len(p), nil
+}
+
+func (e *errorCollector) collectErrors(output string) {
+	lines := e.errorFinder(output)
+	if len(lines) > 0 {
+		e.errorLines = append(e.errorLines, lines...)
+	}
+}
diff --git a/command/testdata/exit_with_message.sh b/command/testdata/exit_with_message.sh
@@ -0,0 +1,9 @@
+#!/usr/bin/env bash
+# These messages are written to stdout
+echo Error: first error
+echo Error: second error
+echo This is not an stdout error
+# These messages are written to stderr
+echo Error: third error >&2
+echo Error: fourth error >&2
+exit 1
