From 5330599c93470f755a5e034c14639f7626ccde95 Mon Sep 17 00:00:00 2001
From: Dan Sosedoff <dan.sosedoff@gmail.com>
Date: Fri, 1 Mar 2019 21:16:43 -0600
Subject: [PATCH 1/2] Capture errors from docker log output

- Refactored logDockerResponse function to remove extra if-else nesting
- logDockerResponse func now returns an error if error was detected from the log stream
- logDockerResponse will check for msg.ErrorDetail.Message and bail if there's an error
---
 Makefile                   |  3 +++
 container/docker_build.go  |  3 ++-
 container/docker_common.go | 49 +++++++++++++++++++++++---------------
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/Makefile b/Makefile
index 2866dcc..35fcfe2 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,9 @@ export GITHUB_TOKEN = $(shell cat ~/.config/github/token)
 
 default: check
 
+test:
+	go test -cover -short ./...
+
 check:
 	$(ACT) -ra check
 
diff --git a/container/docker_build.go b/container/docker_build.go
index f885add..8fac5f4 100644
--- a/container/docker_build.go
+++ b/container/docker_build.go
@@ -51,7 +51,8 @@ func NewDockerBuildExecutor(input NewDockerBuildExecutorInput) common.Executor {
 
 		input.Logger.Debugf("Creating image from context dir '%s' with tag '%s'", input.ContextDir, input.ImageTag)
 		resp, err := cli.ImageBuild(input.Ctx, buildContext, options)
-		input.logDockerResponse(resp.Body, err != nil)
+
+		err = input.logDockerResponse(resp.Body, err != nil)
 		if err != nil {
 			return err
 		}
diff --git a/container/docker_common.go b/container/docker_common.go
index 6665cdc..1a33eb0 100644
--- a/container/docker_common.go
+++ b/container/docker_common.go
@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -64,42 +65,52 @@ func (i *DockerExecutorInput) writeLog(isError bool, format string, args ...inte
 
 }
 
-func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, isError bool) {
+func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, isError bool) error {
 	if dockerResponse == nil {
-		return
+		return nil
 	}
 	defer dockerResponse.Close()
 
 	scanner := bufio.NewScanner(dockerResponse)
 	msg := dockerMessage{}
+
 	for scanner.Scan() {
 		line := scanner.Bytes()
+
 		msg.ID = ""
 		msg.Stream = ""
 		msg.Error = ""
 		msg.ErrorDetail.Message = ""
 		msg.Status = ""
 		msg.Progress = ""
-		if err := json.Unmarshal(line, &msg); err == nil {
-			if msg.Error != "" {
-				i.writeLog(isError, "%s", msg.Error)
-				return
-			}
 
-			if msg.Status != "" {
-				if msg.Progress != "" {
-					i.writeLog(isError, "%s :: %s :: %s\n", msg.Status, msg.ID, msg.Progress)
-				} else {
-					i.writeLog(isError, "%s :: %s\n", msg.Status, msg.ID)
-				}
-			} else if msg.Stream != "" {
-				i.writeLog(isError, msg.Stream)
-			} else {
-				i.writeLog(false, "Unable to handle line: %s", string(line))
-			}
-		} else {
+		if err := json.Unmarshal(line, &msg); err != nil {
 			i.writeLog(false, "Unable to unmarshal line [%s] ==> %v", string(line), err)
+			continue
+		}
+
+		if msg.Error != "" {
+			i.writeLog(isError, "%s", msg.Error)
+			return errors.New(msg.Error)
+		}
+
+		if msg.ErrorDetail.Message != "" {
+			i.writeLog(isError, "%s", msg.ErrorDetail.Message)
+			return errors.New(msg.Error)
+		}
+
+		if msg.Status != "" {
+			if msg.Progress != "" {
+				i.writeLog(isError, "%s :: %s :: %s\n", msg.Status, msg.ID, msg.Progress)
+			} else {
+				i.writeLog(isError, "%s :: %s\n", msg.Status, msg.ID)
+			}
+		} else if msg.Stream != "" {
+			i.writeLog(isError, msg.Stream)
+		} else {
+			i.writeLog(false, "Unable to handle line: %s", string(line))
 		}
 	}
 
+	return nil
 }

From 1c25ee9d710c1c011185093112e33eaba00d6368 Mon Sep 17 00:00:00 2001
From: Dan Sosedoff <dan.sosedoff@gmail.com>
Date: Fri, 1 Mar 2019 21:36:07 -0600
Subject: [PATCH 2/2] Add an extra test

---
 actions/runner_test.go                       | 3 ++-
 actions/testdata/buildfail-action/Dockerfile | 2 ++
 actions/testdata/buildfail.workflow          | 8 ++++++++
 3 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 actions/testdata/buildfail-action/Dockerfile
 create mode 100644 actions/testdata/buildfail.workflow

diff --git a/actions/runner_test.go b/actions/runner_test.go
index 92ef52c..46c969f 100644
--- a/actions/runner_test.go
+++ b/actions/runner_test.go
@@ -21,6 +21,7 @@ func TestRunEvent(t *testing.T) {
 		{"basic.workflow", "push", ""},
 		{"pipe.workflow", "push", ""},
 		{"fail.workflow", "push", "exit with `FAILURE`: 1"},
+		{"buildfail.workflow", "push", "COPY failed"},
 		{"regex.workflow", "push", "exit with `NEUTRAL`: 78"},
 		{"gitref.workflow", "push", ""},
 		{"env.workflow", "push", ""},
@@ -42,7 +43,7 @@ func TestRunEvent(t *testing.T) {
 		if table.errorMessage == "" {
 			assert.NilError(t, err, table.workflowPath)
 		} else {
-			assert.Error(t, err, table.errorMessage)
+			assert.ErrorContains(t, err, table.errorMessage)
 		}
 	}
 }
diff --git a/actions/testdata/buildfail-action/Dockerfile b/actions/testdata/buildfail-action/Dockerfile
new file mode 100644
index 0000000..3a4b7b1
--- /dev/null
+++ b/actions/testdata/buildfail-action/Dockerfile
@@ -0,0 +1,2 @@
+FROM alpine:3.8
+COPY foobar /foo/bar
\ No newline at end of file
diff --git a/actions/testdata/buildfail.workflow b/actions/testdata/buildfail.workflow
new file mode 100644
index 0000000..51eae00
--- /dev/null
+++ b/actions/testdata/buildfail.workflow
@@ -0,0 +1,8 @@
+workflow "test" {
+  on = "push"
+  resolves = ["test-action"]
+}
+
+action "test-action" {
+  uses = "./buildfail-action"
+}
\ No newline at end of file