Skip to content

Commit 026775d

Browse files
Fix crash when PR references a deleted or inaccessible repository (#29)
The GitHub GraphQL API returns null for the repository field on PR nodes when the repo has been deleted or the token lacks access. Skip these nodes instead of crashing with TypeError. Fixes #28
1 parent 5fc7d30 commit 026775d

2 files changed

Lines changed: 113 additions & 0 deletions

File tree

src/good_egg/github_client.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,8 @@ async def fetch_user_merged_prs(
267267
if len(prs) >= max_prs:
268268
break
269269
repo = node["repository"] # type: ignore[index]
270+
if repo is None:
271+
continue
270272
prs.append(
271273
MergedPR(
272274
repo_name_with_owner=repo["nameWithOwner"],
@@ -434,6 +436,8 @@ async def get_user_contribution_data(
434436
if len(prs) >= max_prs:
435437
break
436438
repo = node["repository"] # type: ignore[index]
439+
if repo is None:
440+
continue
437441
repo_name = repo["nameWithOwner"]
438442
prs.append(
439443
MergedPR(
@@ -471,6 +475,8 @@ async def get_user_contribution_data(
471475
if len(prs) >= max_prs:
472476
break
473477
repo = node["repository"] # type: ignore[index]
478+
if repo is None:
479+
continue
474480
repo_name = repo["nameWithOwner"]
475481
prs.append(
476482
MergedPR(

tests/test_github_client.py

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,59 @@ async def test_fetch_user_merged_prs(self) -> None:
131131
assert prs[1].title == "Add WebSocket compression support"
132132
assert prs[1].additions == 200
133133

134+
@respx.mock
135+
async def test_fetch_user_merged_prs_skips_null_repository(self) -> None:
136+
"""PRs whose repository is null (deleted/inaccessible) should be skipped."""
137+
fixture = {
138+
"data": {
139+
"user": {
140+
"login": "testuser",
141+
"createdAt": "2020-01-01T00:00:00Z",
142+
"__typename": "User",
143+
"followers": {"totalCount": 50},
144+
"repositories": {"totalCount": 20},
145+
"pullRequests": {
146+
"totalCount": 3,
147+
"pageInfo": {"hasNextPage": False, "endCursor": None},
148+
"nodes": [
149+
{
150+
"title": "PR to deleted repo",
151+
"mergedAt": "2024-06-15T00:00:00Z",
152+
"additions": 10,
153+
"deletions": 5,
154+
"changedFiles": 1,
155+
"repository": None,
156+
},
157+
{
158+
"title": "PR to accessible repo",
159+
"mergedAt": "2024-03-10T00:00:00Z",
160+
"additions": 20,
161+
"deletions": 3,
162+
"changedFiles": 2,
163+
"repository": {
164+
"nameWithOwner": "elixir-lang/elixir",
165+
"stargazerCount": 23000,
166+
"forkCount": 3200,
167+
"primaryLanguage": {"name": "Elixir"},
168+
"isArchived": False,
169+
"isFork": False,
170+
},
171+
},
172+
],
173+
},
174+
}
175+
}
176+
}
177+
respx.post(GRAPHQL_URL).mock(
178+
return_value=httpx.Response(200, json=fixture)
179+
)
180+
181+
async with _make_client() as client:
182+
prs = await client.fetch_user_merged_prs("testuser")
183+
184+
assert len(prs) == 1
185+
assert prs[0].title == "PR to accessible repo"
186+
134187
@respx.mock
135188
async def test_fetch_user_merged_prs_pagination(self) -> None:
136189
"""Two pages of results should be fetched and concatenated."""
@@ -299,6 +352,60 @@ async def test_get_user_contribution_data(self) -> None:
299352
assert "phoenixframework/phoenix" in data.contributed_repos
300353
assert data.contributed_repos["elixir-lang/elixir"].stargazer_count == 23000
301354

355+
@respx.mock
356+
async def test_null_repository_skipped(self) -> None:
357+
"""PRs with null repository (deleted/inaccessible) should be skipped."""
358+
fixture = {
359+
"data": {
360+
"user": {
361+
"login": "testuser",
362+
"createdAt": "2020-01-01T00:00:00Z",
363+
"__typename": "User",
364+
"followers": {"totalCount": 50},
365+
"repositories": {"totalCount": 20},
366+
"pullRequests": {
367+
"totalCount": 3,
368+
"pageInfo": {"hasNextPage": False, "endCursor": None},
369+
"nodes": [
370+
{
371+
"title": "PR to deleted repo",
372+
"mergedAt": "2024-06-15T00:00:00Z",
373+
"additions": 10,
374+
"deletions": 5,
375+
"changedFiles": 1,
376+
"repository": None,
377+
},
378+
{
379+
"title": "PR to accessible repo",
380+
"mergedAt": "2024-03-10T00:00:00Z",
381+
"additions": 20,
382+
"deletions": 3,
383+
"changedFiles": 2,
384+
"repository": {
385+
"nameWithOwner": "elixir-lang/elixir",
386+
"stargazerCount": 23000,
387+
"forkCount": 3200,
388+
"primaryLanguage": {"name": "Elixir"},
389+
"isArchived": False,
390+
"isFork": False,
391+
},
392+
},
393+
],
394+
},
395+
}
396+
}
397+
}
398+
respx.post(GRAPHQL_URL).mock(
399+
return_value=httpx.Response(200, json=fixture)
400+
)
401+
402+
async with _make_client() as client:
403+
data = await client.get_user_contribution_data("testuser")
404+
405+
assert len(data.merged_prs) == 1
406+
assert data.merged_prs[0].title == "PR to accessible repo"
407+
assert "elixir-lang/elixir" in data.contributed_repos
408+
302409

303410
# ---------------------------------------------------------------------------
304411
# REST: PR comments

0 commit comments

Comments
 (0)