fix: add safety guards for paginated GitHub API requests#104
fix: add safety guards for paginated GitHub API requests#104KaranUnique wants to merge 2 commits into
Conversation
github-actions
Bot
added
bug
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe pagination loops in ChangesPagination Safety Guard
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size/XS
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/github.js (1)
101-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared pagination logic to reduce duplication.
The page-cap and array-validation loop bodies are now identical between
fetchContributorsandfetchIssues, differing only by the URL template. Consider factoring this into a shared paginated-fetch helper to avoid maintaining two copies of the same safety logic.♻️ Suggested refactor
+async function fetchAllPages(urlBuilder, pat) { + const all = [] + for (let page = 1; page <= 50; page++) { + const data = await fetchWithCache(urlBuilder(page), pat) + if (!Array.isArray(data)) break + all.push(...data) + if (data.length < 100) break + } + return all +} + export async function fetchContributors(org, repo, pat) { - const all = [] - for(let page = 1; ; page++) { - if (page > 50) break - const url = `https://api.github.com/repos/${org}/${repo}/contributors?per_page=100&page=${page}` - const data = await fetchWithCache(url, pat) - if (!Array.isArray(data)) break - all.push(...data) - if(data.length < 100) break - } - return all + return fetchAllPages( + page => `https://api.github.com/repos/${org}/${repo}/contributors?per_page=100&page=${page}`, + pat + ) }Also applies to: 114-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/github.js` around lines 101 - 112, `fetchContributors` duplicates the same pagination and array-validation flow used by `fetchIssues`, so extract that shared loop into a reusable helper and have both functions call it with their URL template. Keep the existing safety checks (page cap, `Array.isArray` guard, and stop-on-short-page behavior) inside the shared helper, and update `fetchContributors` to only define the contributors URL and delegate the rest.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/github.js`:
- Line 107: The pagination loop in github.js silently exits when `data` is not
an array, which masks malformed or rate-limit responses as a normal
end-of-results condition. Update the `if (!Array.isArray(data)) break` path in
the relevant loop(s) to emit a warning (for example via `console.warn` or the
existing logger) with enough context to identify the unexpected payload before
breaking, keeping the behavior but making the failure visible.
---
Outside diff comments:
In `@src/services/github.js`:
- Around line 101-112: `fetchContributors` duplicates the same pagination and
array-validation flow used by `fetchIssues`, so extract that shared loop into a
reusable helper and have both functions call it with their URL template. Keep
the existing safety checks (page cap, `Array.isArray` guard, and
stop-on-short-page behavior) inside the shared helper, and update
`fetchContributors` to only define the contributors URL and delegate the rest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f916622b-7d34-418b-97b7-ba21e7f69c13
📒 Files selected for processing (1)
src/services/github.js
github-actions
Bot
added
size/XS
|
Nice work @KaranUnique. Please attach before and after screen recording to review the outcomes. |
|
@Ri1tik This fix addresses a defensive edge case rather than a visible UI behavior. Under normal GitHub API responses, the application behaves the same before and after. The added guards only prevent unexpected or malformed API responses from causing issues, so there isn't a meaningful visual difference to demonstrate in a screen recording |
2 participants
Addressed Issues:
Fixes #103
Additional Notes:
fetchContributors and fetchIssues both used an unbounded for(let page = 1; ; page++) loop with no upper limit. If GitHub returned a malformed or non-array response (e.g. a rate-limit error object), data.length < 100 would never evaluate to true, causing an infinite loop and crashing the app.
Two guards were added to each function, placed at the top of the loop body before any data is consumed:
if (page > 50) break — hard cap at 50 pages (5,000 items max), consistent with the existing guard already present in fetchRepos.
if (!Array.isArray(data)) break — exits safely if the API returns anything other than an array (error object, rate-limit response, etc.), preventing the spread operator from throwing.
No behaviour changes for normal responses — the existing data.length < 100 early-exit still handles the happy path.
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit