Skip to content

Commit

Permalink
chore: improve error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
marcsanmi committed Aug 26, 2024
1 parent 0f4cf2c commit ffe2521
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 5 deletions.
17 changes: 17 additions & 0 deletions pkg/querier/vcs/client/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,20 @@ func toString(s *string) string {
}
return *s
}

func MapErrorToConnectCode(err error) error {
var githubErr *github.ErrorResponse
if errors.As(err, &githubErr) {
switch githubErr.Response.StatusCode {
case http.StatusNotFound:
return connect.NewError(connect.CodeNotFound, err)
case http.StatusForbidden:
return connect.NewError(connect.CodePermissionDenied, err)
case http.StatusUnauthorized:
return connect.NewError(connect.CodeUnauthenticated, err)
case http.StatusTooManyRequests:
return connect.NewError(connect.CodeResourceExhausted, err)
}
}
return connect.NewError(connect.CodeUnknown, err)
}
8 changes: 5 additions & 3 deletions pkg/querier/vcs/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (q *Service) GetCommit(ctx context.Context, req *connect.Request[vcsv1.GetC

commit, err := q.tryGetCommit(ctx, ghClient, owner, repo, ref)
if err != nil {
return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("failed to get commit: %v", err))
return nil, client.MapErrorToConnectCode(err)
}

return connect.NewResponse(commit), nil
Expand All @@ -214,16 +214,18 @@ func (q *Service) tryGetCommit(ctx context.Context, client gitHubCommitGetter, o
"tags/" + ref, // Try as a tag
}

var lastErr error
for _, format := range refFormats {
commit, err := client.GetCommit(ctx, owner, repo, format)
if err == nil {
return commit, nil
}

q.logger.Log("err", err, "msg", "Failed to get commit", "ref", format)
lastErr = err
q.logger.Log("err", lastErr, "msg", "Failed to get commit", "ref", format)
}

return nil, fmt.Errorf("no commit found for %s/%s", owner, repo)
return nil, lastErr
}

func rejectExpiredToken(token *oauth2.Token) error {
Expand Down
12 changes: 10 additions & 2 deletions pkg/querier/vcs/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ package vcs

import (
"context"
"errors"
"net/http"
"testing"

"github.com/go-kit/log"
"github.com/google/go-github/v58/github"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"

Expand Down Expand Up @@ -69,10 +72,13 @@ func TestTryGetCommit(t *testing.T) {
wantErr: false,
},
{
name: "Nonexistent reference",
name: "GitHub API returns not found error",
setupMock: func(m *gitHubCommitGetterMock) {
notFoundErr := &github.ErrorResponse{
Response: &http.Response{StatusCode: http.StatusNotFound},
}
m.On("GetCommit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(nil, assert.AnError).Times(3)
Return(nil, notFoundErr).Times(3)
},
ref: "nonexistent",
wantCommit: nil,
Expand All @@ -89,6 +95,8 @@ func TestTryGetCommit(t *testing.T) {

if tt.wantErr {
assert.Error(t, err)
var githubErr *github.ErrorResponse
assert.True(t, errors.As(err, &githubErr), "Expected a GitHub error")
} else {
assert.NoError(t, err)
}
Expand Down

0 comments on commit ffe2521

Please sign in to comment.