Skip to content

Commit

Permalink
fix: Match internalfailed errors in formatErrorMessage() (fixes #4221…
Browse files Browse the repository at this point in the history
…) (#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)
  • Loading branch information
jahands authored Jul 3, 2024
1 parent c7a6f9c commit 10dca25
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 2 deletions.
8 changes: 6 additions & 2 deletions logbus/solvermon/vertexmon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
61 changes: 61 additions & 0 deletions logbus/solvermon/vertexmon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/earthly/cloud-api/logstream"
"github.com/stretchr/testify/assert"
)

func TestGetExitCode(t *testing.T) {
Expand Down Expand Up @@ -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...",
Expand Down Expand Up @@ -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)
}
})
}
}

0 comments on commit 10dca25

Please sign in to comment.