diff --git a/internal/api/pullrequest_handler_test.go b/internal/api/pullrequest_handler_test.go index 0d984b2..c937da9 100644 --- a/internal/api/pullrequest_handler_test.go +++ b/internal/api/pullrequest_handler_test.go @@ -222,6 +222,17 @@ func TestPullRequestHandlerErrResponse(t *testing.T) { assert.Contains(t, errResp.Message, "unauthorized") }) + t.Run("not found error returns 404", func(t *testing.T) { + err := fmt.Errorf("project deleted-group/deleted-repo: %w", gferrors.ErrNotFound) + + resp := handler.errResponse(err) + + errResp, ok := resp.(ListPullRequests404JSONResponse) + require.True(t, ok, "expected ListPullRequests404JSONResponse") + assert.Equal(t, fmt.Sprintf("%d", http.StatusNotFound), errResp.Code) + assert.Contains(t, errResp.Message, "not found") + }) + t.Run("generic error returns 500", func(t *testing.T) { resp := handler.errResponse(errors.New("something went wrong")) diff --git a/internal/services/bitbucket/bitbucket.go b/internal/services/bitbucket/bitbucket.go index e54012b..84c6ecb 100644 --- a/internal/services/bitbucket/bitbucket.go +++ b/internal/services/bitbucket/bitbucket.go @@ -3,6 +3,7 @@ package bitbucket import ( "context" "encoding/base64" + "errors" "fmt" "net/http" "net/url" @@ -27,6 +28,18 @@ import ( // would require changes across all Bitbucket methods and the underlying library. const defaultBitbucketAPIURL = "https://api.bitbucket.org/2.0" +// isBitbucketNotFound checks whether the error from the go-bitbucket library +// indicates a 404 response. The library returns *UnexpectedResponseStatusError +// with Status set to the HTTP status text (e.g. "404 Not Found"). +func isBitbucketNotFound(err error) bool { + var statusErr *bitbucket.UnexpectedResponseStatusError + if errors.As(err, &statusErr) { + return strings.HasPrefix(statusErr.Status, "404") + } + + return false +} + type BitbucketService struct { httpClient *resty.Client } @@ -56,7 +69,7 @@ func (b *BitbucketService) GetRepository( repository, err := client.Repositories.Repository.Get(repoOptions) if err != nil { - if err.Error() == "404 Not Found" { + if isBitbucketNotFound(err) { return nil, fmt.Errorf("repository %s/%s %w", owner, repo, gferrors.ErrNotFound) } @@ -85,6 +98,10 @@ func (b *BitbucketService) ListRepositories( repositories, err := client.Repositories.ListForAccount(repoOptions) if err != nil { + if isBitbucketNotFound(err) { + return nil, fmt.Errorf("account %s: %w", account, gferrors.ErrNotFound) + } + return nil, fmt.Errorf("failed to list repositories for account %s: %w", account, err) } @@ -157,6 +174,10 @@ func (b *BitbucketService) ListBranches( for b, err := range scanBranches { if err != nil { + if isBitbucketNotFound(err) { + return nil, fmt.Errorf("repository %s/%s: %w", owner, repo, gferrors.ErrNotFound) + } + return nil, fmt.Errorf("failed to list branches: %w", err) } @@ -266,6 +287,14 @@ func (b *BitbucketService) ListPullRequests( return nil, fmt.Errorf("failed to list pull requests for %s/%s: %w", owner, repo, err) } + if resp.StatusCode() == http.StatusNotFound { + return nil, fmt.Errorf("repository %s/%s: %w", owner, repo, gferrors.ErrNotFound) + } + + if resp.StatusCode() == http.StatusUnauthorized || resp.StatusCode() == http.StatusForbidden { + return nil, fmt.Errorf("invalid credentials: %w", gferrors.ErrUnauthorized) + } + if resp.IsError() { return nil, fmt.Errorf("failed to list pull requests for %s/%s: status %d, body: %s", owner, repo, resp.StatusCode(), resp.String()) diff --git a/internal/services/bitbucket/bitbucket_pullrequests_test.go b/internal/services/bitbucket/bitbucket_pullrequests_test.go index 2bc74f0..a414d1a 100644 --- a/internal/services/bitbucket/bitbucket_pullrequests_test.go +++ b/internal/services/bitbucket/bitbucket_pullrequests_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "net/http" "net/http/httptest" "net/url" @@ -13,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + gferrors "github.com/KubeRocketCI/gitfusion/internal/errors" "github.com/KubeRocketCI/gitfusion/internal/models" "github.com/KubeRocketCI/gitfusion/internal/services/krci" ) @@ -276,6 +278,7 @@ func TestBitbucketServiceListPullRequests(t *testing.T) { statusCode int wantErr bool wantErrContain string + wantSentinel error wantCount int wantState models.PullRequestState validateQuery func(t *testing.T, r *http.Request) @@ -378,7 +381,8 @@ func TestBitbucketServiceListPullRequests(t *testing.T) { responseBody: bitbucketPRResponse{}, statusCode: http.StatusUnauthorized, wantErr: true, - wantErrContain: "status 401", + wantErrContain: "unauthorized", + wantSentinel: gferrors.ErrUnauthorized, }, } @@ -424,6 +428,10 @@ func TestBitbucketServiceListPullRequests(t *testing.T) { assert.Contains(t, err.Error(), tt.wantErrContain) } + if tt.wantSentinel != nil { + assert.True(t, errors.Is(err, tt.wantSentinel), "error should wrap %v", tt.wantSentinel) + } + return } @@ -768,3 +776,63 @@ func TestBitbucketServiceListPullRequestsSupersededState(t *testing.T) { assert.Equal(t, models.PullRequestStateClosed, result.Data[0].State) assert.Equal(t, models.PullRequestStateClosed, result.Data[1].State) } + +func TestBitbucketServiceListPullRequestsNotFound(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"type":"error","error":{"message":"Repository not found"}}`)) + })) + defer server.Close() + + svc := &BitbucketService{ + httpClient: resty.New().SetTransport(&redirectTransport{ + target: server.URL, + wrapped: http.DefaultTransport, + }), + } + token := testBitbucketToken() + + result, err := svc.ListPullRequests( + context.Background(), + "deleted-owner", + "deleted-repo", + krci.GitServerSettings{Token: token}, + models.PullRequestListOptions{State: "open", Page: 1, PerPage: 20}, + ) + + assert.Error(t, err) + assert.Nil(t, result) + assert.True(t, errors.Is(err, gferrors.ErrNotFound), "error should wrap gferrors.ErrNotFound") + assert.Contains(t, err.Error(), "not found") +} + +func TestBitbucketServiceListPullRequestsUnauthorized(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"type":"error","error":{"message":"Unauthorized"}}`)) + })) + defer server.Close() + + svc := &BitbucketService{ + httpClient: resty.New().SetTransport(&redirectTransport{ + target: server.URL, + wrapped: http.DefaultTransport, + }), + } + token := testBitbucketToken() + + result, err := svc.ListPullRequests( + context.Background(), + "owner", + "repo", + krci.GitServerSettings{Token: token}, + models.PullRequestListOptions{State: "open", Page: 1, PerPage: 20}, + ) + + assert.Error(t, err) + assert.Nil(t, result) + assert.True(t, errors.Is(err, gferrors.ErrUnauthorized), "error should wrap gferrors.ErrUnauthorized") + assert.Contains(t, err.Error(), "unauthorized") +} diff --git a/internal/services/github/github.go b/internal/services/github/github.go index 66b1956..ba975ee 100644 --- a/internal/services/github/github.go +++ b/internal/services/github/github.go @@ -19,6 +19,24 @@ import ( "github.com/KubeRocketCI/gitfusion/pkg/xiter" ) +// classifyGitHubError maps a GitHub API error to a domain sentinel error. +// Returns nil if the error is not a recognized GitHub error response. +func classifyGitHubError(err error) error { + ghErr := &github.ErrorResponse{} + if !errors.As(err, &ghErr) { + return nil + } + + switch ghErr.Response.StatusCode { + case http.StatusNotFound: + return gferrors.ErrNotFound + case http.StatusUnauthorized: + return gferrors.ErrUnauthorized + default: + return nil + } +} + const ( stateOpen = "open" stateClosed = "closed" @@ -42,11 +60,8 @@ func (g *GitHubProvider) GetRepository( repository, _, err := client.Repositories.Get(ctx, owner, repo) if err != nil { - ghErr := &github.ErrorResponse{} - if errors.As(err, &ghErr) { - if ghErr.Response.StatusCode == http.StatusNotFound { - return nil, fmt.Errorf("repository %s/%s: %w", owner, repo, gferrors.ErrNotFound) - } + if sentinel := classifyGitHubError(err); sentinel != nil { + return nil, fmt.Errorf("repository %s/%s: %w", owner, repo, sentinel) } return nil, fmt.Errorf("failed to get repository %s/%s: %w", owner, repo, err) @@ -72,11 +87,8 @@ func (g *GitHubProvider) ListRepositories( for repo, err := range it { if err != nil { - ghErr := &github.ErrorResponse{} - if errors.As(err, &ghErr) { - if ghErr.Response.StatusCode == http.StatusNotFound { - return nil, fmt.Errorf("organization or user %s: %w", owner, gferrors.ErrNotFound) - } + if sentinel := classifyGitHubError(err); sentinel != nil { + return nil, fmt.Errorf("organization or user %s: %w", owner, sentinel) } return nil, fmt.Errorf("failed to list repositories for org %s: %w", owner, err) @@ -291,6 +303,10 @@ func (g *GitHubProvider) ListBranches( for b, err := range it { if err != nil { + if sentinel := classifyGitHubError(err); sentinel != nil { + return nil, fmt.Errorf("repository %s/%s: %w", owner, repo, sentinel) + } + return nil, fmt.Errorf("failed to list branches: %w", err) } @@ -339,6 +355,10 @@ func (g *GitHubProvider) listPullRequestsDirect( }, }) if err != nil { + if sentinel := classifyGitHubError(err); sentinel != nil { + return nil, fmt.Errorf("repository %s/%s: %w", owner, repo, sentinel) + } + return nil, fmt.Errorf("failed to list pull requests for %s/%s: %w", owner, repo, err) } @@ -415,6 +435,10 @@ func (g *GitHubProvider) listPullRequestsWithPostFilter( pagesQueried++ if err != nil { + if sentinel := classifyGitHubError(err); sentinel != nil { + return nil, fmt.Errorf("repository %s/%s: %w", owner, repo, sentinel) + } + return nil, fmt.Errorf("failed to list pull requests for %s/%s: %w", owner, repo, err) } diff --git a/internal/services/github/github_pipelines.go b/internal/services/github/github_pipelines.go index 6742888..0a856be 100644 --- a/internal/services/github/github_pipelines.go +++ b/internal/services/github/github_pipelines.go @@ -2,15 +2,12 @@ package github import ( "context" - "errors" "fmt" - "net/http" "strconv" "time" "github.com/google/go-github/v72/github" - gferrors "github.com/KubeRocketCI/gitfusion/internal/errors" "github.com/KubeRocketCI/gitfusion/internal/models" "github.com/KubeRocketCI/gitfusion/internal/services/common" "github.com/KubeRocketCI/gitfusion/internal/services/krci" @@ -56,15 +53,8 @@ func (g *GitHubProvider) ListPipelines( workflowRuns, _, err := client.Actions.ListRepositoryWorkflowRuns(ctx, owner, repo, ghOpts) if err != nil { - ghErr := &github.ErrorResponse{} - if errors.As(err, &ghErr) { - if ghErr.Response.StatusCode == http.StatusNotFound { - return nil, fmt.Errorf("project %s: %w", project, gferrors.ErrNotFound) - } - - if ghErr.Response.StatusCode == http.StatusUnauthorized { - return nil, fmt.Errorf("invalid credentials: %w", gferrors.ErrUnauthorized) - } + if sentinel := classifyGitHubError(err); sentinel != nil { + return nil, fmt.Errorf("project %s: %w", project, sentinel) } return nil, fmt.Errorf("failed to list pipelines for %s: %w", project, err) diff --git a/internal/services/github/github_pullrequests_test.go b/internal/services/github/github_pullrequests_test.go index 55baddd..3e56620 100644 --- a/internal/services/github/github_pullrequests_test.go +++ b/internal/services/github/github_pullrequests_test.go @@ -3,6 +3,7 @@ package github import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "net/url" @@ -14,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + gferrors "github.com/KubeRocketCI/gitfusion/internal/errors" "github.com/KubeRocketCI/gitfusion/internal/models" "github.com/KubeRocketCI/gitfusion/internal/services/krci" ) @@ -1186,3 +1188,91 @@ func TestGitHubProviderListPullRequestsContextCancellation(t *testing.T) { assert.Error(t, err) assert.Nil(t, result) } + +func TestGitHubProviderListPullRequestsNotFound(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/deleted-owner/deleted-repo/pulls", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(map[string]string{ + "message": "Not Found", + }) + }) + + server := httptest.NewServer(mux) + defer server.Close() + + provider := newTestProvider(server.URL) + + result, err := provider.ListPullRequests( + context.Background(), + "deleted-owner", + "deleted-repo", + krci.GitServerSettings{Token: "test-token"}, + models.PullRequestListOptions{State: "open", Page: 1, PerPage: 20}, + ) + + assert.Error(t, err) + assert.Nil(t, result) + assert.True(t, errors.Is(err, gferrors.ErrNotFound), "error should wrap gferrors.ErrNotFound") + assert.Contains(t, err.Error(), "not found") +} + +func TestGitHubProviderListPullRequestsNotFoundPostFilter(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/deleted-owner/deleted-repo/pulls", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(map[string]string{ + "message": "Not Found", + }) + }) + + server := httptest.NewServer(mux) + defer server.Close() + + provider := newTestProvider(server.URL) + + // Use "merged" state which triggers the post-filter code path + result, err := provider.ListPullRequests( + context.Background(), + "deleted-owner", + "deleted-repo", + krci.GitServerSettings{Token: "test-token"}, + models.PullRequestListOptions{State: "merged", Page: 1, PerPage: 20}, + ) + + assert.Error(t, err) + assert.Nil(t, result) + assert.True(t, errors.Is(err, gferrors.ErrNotFound), "error should wrap gferrors.ErrNotFound for post-filter path") + assert.Contains(t, err.Error(), "not found") +} + +func TestGitHubProviderListPullRequestsUnauthorized(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/owner/repo/pulls", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + _ = json.NewEncoder(w).Encode(map[string]string{ + "message": "Bad credentials", + }) + }) + + server := httptest.NewServer(mux) + defer server.Close() + + provider := newTestProvider(server.URL) + + result, err := provider.ListPullRequests( + context.Background(), + "owner", + "repo", + krci.GitServerSettings{Token: "bad-token"}, + models.PullRequestListOptions{State: "open", Page: 1, PerPage: 20}, + ) + + assert.Error(t, err) + assert.Nil(t, result) + assert.True(t, errors.Is(err, gferrors.ErrUnauthorized), "error should wrap gferrors.ErrUnauthorized") + assert.Contains(t, err.Error(), "unauthorized") +} diff --git a/internal/services/gitlab/gitlab.go b/internal/services/gitlab/gitlab.go index ebab8be..b33287e 100644 --- a/internal/services/gitlab/gitlab.go +++ b/internal/services/gitlab/gitlab.go @@ -157,6 +157,10 @@ func (g *GitlabProvider) ListBranches( for b, err := range it { if err != nil { + if errors.Is(err, gitlab.ErrNotFound) { + return nil, fmt.Errorf("project %s/%s: %w", owner, repo, gferrors.ErrNotFound) + } + return nil, fmt.Errorf("failed to list branches for %s/%s: %w", owner, repo, err) } @@ -198,7 +202,7 @@ func (g *GitlabProvider) TriggerPipeline( return nil, fmt.Errorf("project %s or ref %s: %w", project, ref, gferrors.ErrNotFound) } - if resp != nil && resp.StatusCode == http.StatusUnauthorized { + if resp != nil && (resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden) { return nil, fmt.Errorf("invalid credentials: %w", gferrors.ErrUnauthorized) } @@ -258,7 +262,7 @@ func (g *GitlabProvider) ListPipelines( return nil, fmt.Errorf("project %s: %w", project, gferrors.ErrNotFound) } - if resp != nil && resp.StatusCode == http.StatusUnauthorized { + if resp != nil && (resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden) { return nil, fmt.Errorf("invalid credentials: %w", gferrors.ErrUnauthorized) } @@ -440,6 +444,14 @@ func (g *GitlabProvider) ListPullRequests( gitlab.WithContext(ctx), ) if err != nil { + if errors.Is(err, gitlab.ErrNotFound) || (resp != nil && resp.StatusCode == http.StatusNotFound) { + return nil, fmt.Errorf("project %s/%s: %w", owner, repo, gferrors.ErrNotFound) + } + + if resp != nil && (resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden) { + return nil, fmt.Errorf("invalid credentials: %w", gferrors.ErrUnauthorized) + } + return nil, fmt.Errorf("failed to list merge requests for %s/%s: %w", owner, repo, err) } diff --git a/internal/services/gitlab/gitlab_pullrequests_test.go b/internal/services/gitlab/gitlab_pullrequests_test.go index 54fdc4f..9b37116 100644 --- a/internal/services/gitlab/gitlab_pullrequests_test.go +++ b/internal/services/gitlab/gitlab_pullrequests_test.go @@ -3,6 +3,7 @@ package gitlab import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "testing" @@ -11,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + gferrors "github.com/KubeRocketCI/gitfusion/internal/errors" "github.com/KubeRocketCI/gitfusion/internal/models" "github.com/KubeRocketCI/gitfusion/internal/services/krci" ) @@ -307,3 +309,58 @@ func TestGitLabProviderListPullRequestsNilAuthor(t *testing.T) { require.Len(t, result.Data, 1) assert.Nil(t, result.Data[0].Author, "author should be nil when JSON author is null") } + +func TestGitLabProviderListPullRequestsNotFound(t *testing.T) { + mux := http.NewServeMux() + mrPath := "/api/v4/projects/deleted-group%2Fdeleted-repo/merge_requests" + mux.HandleFunc(mrPath, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"404 Project Not Found"}`)) + }) + + server := httptest.NewServer(mux) + defer server.Close() + + provider := NewGitlabProvider() + + result, err := provider.ListPullRequests( + context.Background(), + "deleted-group", + "deleted-repo", + krci.GitServerSettings{Token: "test-token", Url: server.URL}, + models.PullRequestListOptions{State: "open", Page: 1, PerPage: 20}, + ) + + assert.Error(t, err) + assert.Nil(t, result) + assert.True(t, errors.Is(err, gferrors.ErrNotFound), "error should wrap gferrors.ErrNotFound") + assert.Contains(t, err.Error(), "not found") +} + +func TestGitLabProviderListPullRequestsUnauthorized(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/api/v4/projects/owner%2Frepo/merge_requests", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message":"401 Unauthorized"}`)) + }) + + server := httptest.NewServer(mux) + defer server.Close() + + provider := NewGitlabProvider() + + result, err := provider.ListPullRequests( + context.Background(), + "owner", + "repo", + krci.GitServerSettings{Token: "bad-token", Url: server.URL}, + models.PullRequestListOptions{State: "open", Page: 1, PerPage: 20}, + ) + + assert.Error(t, err) + assert.Nil(t, result) + assert.True(t, errors.Is(err, gferrors.ErrUnauthorized), "error should wrap gferrors.ErrUnauthorized") + assert.Contains(t, err.Error(), "unauthorized") +}