diff --git a/internal/backend/github_enrichment.go b/internal/backend/github_enrichment.go index d135ee6b..a105a865 100644 --- a/internal/backend/github_enrichment.go +++ b/internal/backend/github_enrichment.go @@ -3,6 +3,7 @@ package backend import ( "context" "encoding/base64" + "errors" "fmt" "math" "regexp" @@ -320,11 +321,11 @@ func (c *GitHubClient) hydrateTopRepoEvidence(ctx context.Context, repos []TopRe for index := 0; index < limit; index++ { owner := valueOrString(repos[index].OwnerLogin, fallbackOwner) readme, readmeErr := c.fetchReadmeDocument(ctx, owner, repos[index].Name) - if readmeErr != nil && readmeErr != ErrGitHubUnavailable { + if readmeErr != nil && !errors.Is(readmeErr, ErrGitHubUnavailable) { return readmeErr } languages, languageErr := c.fetchRepoLanguages(ctx, owner, repos[index].Name) - if languageErr != nil && languageErr != ErrGitHubUnavailable { + if languageErr != nil && !errors.Is(languageErr, ErrGitHubUnavailable) { return languageErr } if readme != nil { diff --git a/internal/backend/github_enrichment_test.go b/internal/backend/github_enrichment_test.go new file mode 100644 index 00000000..982420ae --- /dev/null +++ b/internal/backend/github_enrichment_test.go @@ -0,0 +1,174 @@ +package backend + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" +) + +// A plain 403 (no Retry-After, X-Ratelimit-Remaining != 0) wraps +// ErrGitHubUnavailable. hydrateTopRepoEvidence used direct comparison, which +// cannot match that sentinel and aborted the remaining candidate repositories. + +func newEvidenceTestClient(t *testing.T, handler http.HandlerFunc) *GitHubClient { + t.Helper() + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + return NewGitHubClient("token").withBaseURL(server.URL).withHTTPClient(server.Client()) +} + +// base64 of a README carrying install/usage/api/testing signals, so the hydrated +// repository has real parsed features rather than an empty document. +const evidenceReadmeBase64 = "IyBFeGFtcGxlCgpBIHNtYWxsIGxpYnJhcnkuCgojIyBJbnN0YWxsCgpgYGBiYXNoCm5wbSBpIGV4YW1wbGUKYGBgCgojIyBVc2FnZQoKU2VlIHRoZSBkZW1vLgoKIyMgQVBJCgphcGkgc3VyZmFjZQoKIyMgVGVzdGluZwoKcnVuIHRlc3RzCg==" + +func TestHydrateTopRepoEvidenceContinuesAfterPlainForbidden(t *testing.T) { + requested := map[string]int{} + client := newEvidenceTestClient(t, func(w http.ResponseWriter, r *http.Request) { + requested[r.URL.Path]++ + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Ratelimit-Remaining", "4998") + switch { + // The first repository's README is forbidden on every attempt. + case r.URL.Path == "/repos/alice/first/readme": + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"Forbidden","documentation_url":"https://docs.github.com"}`)) + case r.URL.Path == "/repos/alice/first/languages": + _, _ = w.Write([]byte(`{"Go":1200}`)) + case r.URL.Path == "/repos/alice/first/contents": + _, _ = w.Write([]byte(`[{"name":"main.go"}]`)) + case r.URL.Path == "/repos/alice/second/readme": + _, _ = w.Write([]byte(`{"path":"README.md","encoding":"base64","size":120,"content":"` + evidenceReadmeBase64 + `"}`)) + case r.URL.Path == "/repos/alice/second/languages": + _, _ = w.Write([]byte(`{"TypeScript":800}`)) + case r.URL.Path == "/repos/alice/second/contents": + _, _ = w.Write([]byte(`[{"name":"src"}]`)) + default: + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"Not Found"}`)) + } + }) + + repos := []TopRepo{ + {Name: "first", OwnerLogin: stringPointer("alice"), Stars: 40}, + {Name: "second", OwnerLogin: stringPointer("alice"), Stars: 20}, + } + if err := client.hydrateTopRepoEvidence(context.Background(), repos, "alice", 6); err != nil { + t.Fatalf("hydrateTopRepoEvidence returned error: %v", err) + } + + // The forbidden README must not stop the loop: the second repository still gets + // its README, languages and root listing. + if requested["/repos/alice/second/readme"] == 0 { + t.Errorf("second repository README was never requested; paths seen = %v", requested) + } + if repos[1].Readme == nil { + t.Fatalf("second repository lost its README despite only the first being forbidden") + } + if len(repos[1].Languages) != 1 || repos[1].Languages[0].Name != "TypeScript" { + t.Errorf("second repository languages = %#v", repos[1].Languages) + } + if len(repos[1].RootPaths) != 1 || repos[1].RootPaths[0] != "src" { + t.Errorf("second repository root paths = %#v", repos[1].RootPaths) + } + // The degraded repository keeps its other evidence and simply has no README. + if repos[0].Readme != nil { + t.Errorf("first repository should have no README, got %#v", repos[0].Readme) + } + if len(repos[0].Languages) != 1 { + t.Errorf("first repository languages = %#v, want the languages call to have succeeded", repos[0].Languages) + } +} + +func TestHydrateTopRepoEvidenceContinuesAfterForbiddenLanguages(t *testing.T) { + client := newEvidenceTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Ratelimit-Remaining", "4998") + switch r.URL.Path { + case "/repos/alice/first/readme", "/repos/alice/second/readme": + _, _ = w.Write([]byte(`{"path":"README.md","encoding":"base64","size":120,"content":"` + evidenceReadmeBase64 + `"}`)) + case "/repos/alice/first/languages": + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"Forbidden"}`)) + case "/repos/alice/second/languages": + _, _ = w.Write([]byte(`{"TypeScript":800}`)) + default: + _, _ = w.Write([]byte(`[]`)) + } + }) + + repos := []TopRepo{{Name: "first", OwnerLogin: stringPointer("alice")}, {Name: "second", OwnerLogin: stringPointer("alice")}} + if err := client.hydrateTopRepoEvidence(context.Background(), repos, "alice", 6); err != nil { + t.Fatalf("a plain languages 403 must not abort later evidence: %v", err) + } + if repos[1].Readme == nil || len(repos[1].Languages) != 1 { + t.Fatalf("second repository was not hydrated: %#v", repos[1]) + } + if len(repos[0].Languages) != 0 || repos[0].Readme == nil { + t.Fatalf("forbidden languages should retain the README and omit languages: %#v", repos[0]) + } +} + +func TestHydrateTopRepoEvidenceStillAbortsOnRateLimit(t *testing.T) { + requested := map[string]int{} + client := newEvidenceTestClient(t, func(w http.ResponseWriter, r *http.Request) { + requested[r.URL.Path]++ + w.Header().Set("Content-Type", "application/json") + if r.URL.Path == "/repos/alice/first/readme" { + // A real rate-limit signal: remaining == 0. This is ErrGitHubRateLimited, + // not ErrGitHubUnavailable, and must still stop collection. + w.Header().Set("X-Ratelimit-Remaining", "0") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"API rate limit exceeded"}`)) + return + } + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"Not Found"}`)) + }) + + repos := []TopRepo{ + {Name: "first", OwnerLogin: stringPointer("alice"), Stars: 40}, + {Name: "second", OwnerLogin: stringPointer("alice"), Stars: 20}, + } + err := client.hydrateTopRepoEvidence(context.Background(), repos, "alice", 6) + if !errors.Is(err, ErrGitHubRateLimited) { + t.Fatalf("expected ErrGitHubRateLimited after retries, got %v", err) + } + if requested["/repos/alice/first/readme"] != 2 { + t.Errorf("primary rate-limit response should be retried once, got %d requests", requested["/repos/alice/first/readme"]) + } + if requested["/repos/alice/second/readme"] != 0 { + t.Errorf("rate limited pool should abort, but later repos were requested: %v", requested) + } +} + +func TestHydrateTopRepoEvidenceTreatsMissingReadmeAsAbsent(t *testing.T) { + // 404 is returned unwrapped as ErrGitHubAccountNotFound and means "no README". + // It must stay non-fatal after the errors.Is change. + client := newEvidenceTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/repos/alice/first/readme": + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"Not Found"}`)) + case "/repos/alice/first/languages": + _, _ = w.Write([]byte(`{"Go":10}`)) + case "/repos/alice/first/contents": + _, _ = w.Write([]byte(`[]`)) + default: + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"Not Found"}`)) + } + }) + repos := []TopRepo{{Name: "first", OwnerLogin: stringPointer("alice"), Stars: 1}} + if err := client.hydrateTopRepoEvidence(context.Background(), repos, "alice", 6); err != nil { + t.Fatalf("a missing README must not abort collection: %v", err) + } + if repos[0].Readme != nil { + t.Errorf("expected no README, got %#v", repos[0].Readme) + } + if valueOrString(repos[0].ReadmeExcerpt, "") != "" { + t.Errorf("expected empty README excerpt, got %q", valueOrString(repos[0].ReadmeExcerpt, "")) + } +}