Skip to content

Commit

Permalink
Make error less brittle to fix upcoming error with HCP SDK Go changin…
Browse files Browse the repository at this point in the history
…g how we return 404 error
  • Loading branch information
JenGoldstrich committed Jan 21, 2025
1 parent dddc1fb commit cf6a82f
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 2 deletions.
17 changes: 15 additions & 2 deletions internal/hcp/api/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ package api

import (
"fmt"
"strings"
"regexp"
"strconv"

"google.golang.org/grpc/codes"
)
Expand All @@ -26,6 +27,8 @@ func (c *ClientError) Error() string {
return fmt.Sprintf("status %d: err %v", c.StatusCode, c.Err)
}

var errCodeRegex = regexp.MustCompilePOSIX(`[Cc]ode"?:([0-9]+)`)

// CheckErrorCode checks the error string for err for some code and returns true
// if the code is found. Ideally this function should use status.FromError
// https://pkg.go.dev/google.golang.org/grpc/status#pkg-functions but that
Expand All @@ -35,5 +38,15 @@ func CheckErrorCode(err error, code codes.Code) bool {
return false
}

return strings.Contains(err.Error(), fmt.Sprintf("Code:%d", code))
// If the error string doesn't match the code we're looking for, we
// can ignore it and return false immediately.
matches := errCodeRegex.FindStringSubmatch(err.Error())
if len(matches) == 0 {
return false
}

// Safe to ignore the error here since the regex's submatch is always a
// valid integer given the format ([0-9]+)
errCode, _ := strconv.Atoi(matches[1])
return errCode == int(code)
}
57 changes: 57 additions & 0 deletions internal/hcp/api/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package api

import (
"fmt"
"testing"

"google.golang.org/grpc/codes"
)

func TestCheckErrorCode(t *testing.T) {
tests := []struct {
name string
codeString string
expectCode codes.Code
expectSuccess bool
}{
{
"old format, code matches what is looked for",
`{Code:5,"details":[],"message":"Error: The bucket etc."}`,
codes.Code(5),
true,
},
{
"old format, code doesn't match what is looked for",
`{Code:55,"details":[],"message":"Error: The bucket etc."}`,
codes.Code(5),
false,
},
{
"new format, code matches what is looked for",
`{"code":5,"details":[],"message":"Error: The bucket etc."}`,
codes.Code(5),
true,
},
{
"new format, code doesn't match what is looked for",
`{"code":55,"details":[],"message":"Error: The bucket etc."}`,
codes.Code(5),
false,
},
{
"bad format, should always be false",
`"ceod":55`,
codes.Code(5),
false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
found := CheckErrorCode(fmt.Errorf(tt.codeString), tt.expectCode)
if found != tt.expectSuccess {
t.Errorf("check error code returned %t, expected %t", found, tt.expectSuccess)
}
})
}
}

0 comments on commit cf6a82f

Please sign in to comment.