From 10dca25deb84233c13c8e3e9ad817f46d171d1cd Mon Sep 17 00:00:00 2001
From: Jacob Hands <jacob@gogit.io>
Date: Wed, 3 Jul 2024 16:05:48 -0500
Subject: [PATCH] fix: Match internalfailed errors in formatErrorMessage()
 (fixes #4221) (#4222)

See #4221 for details and how to repro.

Added tests to validate that the regex matches as expected.

Didn't have time to add tests for formatErrorMessage(), but I confirmed
that it correctly formats the error now:
![CleanShot 2024-06-24 at 08 54
16@2x](https://github.com/earthly/earthly/assets/10719325/f6ed2fdd-776c-4045-a959-a28f15ea4962)

Also verified that it doesn't panic when the regex fails to match:
![CleanShot 2024-06-24 at 08 54
00@2x](https://github.com/earthly/earthly/assets/10719325/5a232481-2674-4501-bb84-02e94f0eaf43)
---
 logbus/solvermon/vertexmon.go      |  8 +++-
 logbus/solvermon/vertexmon_test.go | 61 ++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/logbus/solvermon/vertexmon.go b/logbus/solvermon/vertexmon.go
index 4e0296fb..8280c6b5 100644
--- a/logbus/solvermon/vertexmon.go
+++ b/logbus/solvermon/vertexmon.go
@@ -63,7 +63,7 @@ func getExitCode(errString string) (int, error) {
 	return 0, errNoExitCode
 }
 
-var reErrNotFound = regexp.MustCompile(`^failed to calculate checksum of ref ([^ ]*): (.*)$`)
+var reErrNotFound = regexp.MustCompile(`^\s*(internal)?failed to calculate checksum of ref ([^ ]::[^ ]*|[^ ]*): (.*)\s*$`)
 var reHint = regexp.MustCompile(`^(?P<msg>.+?):Hint: .+`)
 
 // determineFatalErrorType returns logstream.FailureType
@@ -114,10 +114,14 @@ func formatErrorMessage(errString, operation string, internal bool, fatalErrorTy
 				"      did not complete successfully. Exit code %d", internalStr, operation, exitCode)
 	case logstream.FailureType_FAILURE_TYPE_FILE_NOT_FOUND:
 		m := reErrNotFound.FindStringSubmatch(errString)
+		reason := fmt.Sprintf("unable to parse file_not_found error:%s", errString)
+		if len(m) > 2 {
+			reason = m[3]
+		}
 		return fmt.Sprintf(
 			"      The%s command\n"+
 				"          %s\n"+
-				"      failed: %s", internalStr, operation, m[2])
+				"      failed: %s", internalStr, operation, reason)
 	case logstream.FailureType_FAILURE_TYPE_GIT:
 		gitStdErr, shorterErr, ok := errutil.ExtractEarthlyGitStdErr(errString)
 		if ok {
diff --git a/logbus/solvermon/vertexmon_test.go b/logbus/solvermon/vertexmon_test.go
index ac435305..1cd7208f 100644
--- a/logbus/solvermon/vertexmon_test.go
+++ b/logbus/solvermon/vertexmon_test.go
@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/earthly/cloud-api/logstream"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestGetExitCode(t *testing.T) {
@@ -94,6 +95,22 @@ func TestDetermineFatalErrorType(t *testing.T) {
 			expectedType:  logstream.FailureType_FAILURE_TYPE_FILE_NOT_FOUND,
 			expectedFatal: true,
 		},
+		{
+			name:          "file not found (internal)",
+			errString:     "internalfailed to calculate checksum of ref foo: bar",
+			exitCode:      0,
+			parseErr:      nil,
+			expectedType:  logstream.FailureType_FAILURE_TYPE_FILE_NOT_FOUND,
+			expectedFatal: true,
+		},
+		{
+			name:          "file not found (internal with space)",
+			errString:     " internalfailed to calculate checksum of ref foo: bar",
+			exitCode:      0,
+			parseErr:      nil,
+			expectedType:  logstream.FailureType_FAILURE_TYPE_FILE_NOT_FOUND,
+			expectedFatal: true,
+		},
 		{
 			name:          "git error",
 			errString:     "EARTHLY_GIT_STDERR: Z2l0IC1jI...",
@@ -129,3 +146,47 @@ func TestDetermineFatalErrorType(t *testing.T) {
 		})
 	}
 }
+
+func TestReErrNotFound(t *testing.T) {
+	tests := []struct {
+		name      string
+		errString string
+		expected  []string
+	}{
+		{
+			name:      "simple",
+			errString: "failed to calculate checksum of ref foo: bar",
+			expected:  []string{"", "foo", "bar"},
+		},
+		{
+			name:      "simple (internal)",
+			errString: "internalfailed to calculate checksum of ref foo: bar",
+			expected:  []string{"internal", "foo", "bar"},
+		},
+		{
+			name:      "simple (internal with space)",
+			errString: " internalfailed to calculate checksum of ref foo: bar",
+			expected:  []string{"internal", "foo", "bar"},
+		},
+		{
+			name:      "complex",
+			errString: ` failed to calculate checksum of ref p4gz72iufvk3t1nsqq07p9sim::m4m7o7gui4zuuoy9vynbrzx8f: "/doesnotexist": not found`,
+			expected:  []string{"", "p4gz72iufvk3t1nsqq07p9sim::m4m7o7gui4zuuoy9vynbrzx8f", `"/doesnotexist": not found`},
+		},
+		{
+			name:      "complex (internal)",
+			errString: ` internalfailed to calculate checksum of ref p4gz72iufvk3t1nsqq07p9sim::m4m7o7gui4zuuoy9vynbrzx8f: "/doesnotexist": not found`,
+			expected:  []string{"internal", "p4gz72iufvk3t1nsqq07p9sim::m4m7o7gui4zuuoy9vynbrzx8f", `"/doesnotexist": not found`},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			match := reErrNotFound.FindStringSubmatch(tt.errString)
+
+			if len(match) == 0 || !assert.ElementsMatch(t, match[1:], tt.expected) {
+				t.Errorf("reErrNotFound.FindStringSubmatch(%s) = %v, want %v", tt.errString, match, tt.expected)
+			}
+		})
+	}
+}