Refactor GitHub service for improved caching and error handling#99
Refactor GitHub service for improved caching and error handling#99rudrakshtank wants to merge 2 commits into
Conversation
github-actions
Bot
added
enhancement
WalkthroughThe GitHub service now uses token-aware IndexedDB caching, versioned GitHub requests, structured API error handling, and cancellation-aware fetch helpers. Public fetch functions accept an ChangesGitHub fetch caching and error handling rework
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant fetchWithCache as fetchWithCache
participant IndexedDB as IndexedDB cache
participant GitHubAPI as GitHub API
participant Window as window
fetchWithCache->>IndexedDB: read token-scoped cached response
fetchWithCache->>GitHubAPI: request with Bearer auth and API version
GitHubAPI-->>fetchWithCache: response and rate-limit headers
fetchWithCache->>Window: dispatch rate-limit update
fetchWithCache->>IndexedDB: store successful response asynchronously
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.1)src/services/github.jsFile contains syntax errors that prevent linting: Line 358: Illegal use of an export declaration not at the top level; Line 376: expected 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/L
![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
🤖 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`:
- Around line 28-35: The IndexedDB connection cached by dbPromise is not being
reset when db.onversionchange closes the database, so later cacheGet/cacheSet
calls keep reusing a closed handle and fail. Update the db.onversionchange
handler in the open DB flow to clear dbPromise before closing the db, so the
next cache access in the GitHub service opens a fresh connection instead of
reusing the stale one.
🪄 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: 182a475a-ff65-4f11-914e-9c2937ddd5b5
📒 Files selected for processing (1)
src/services/github.js
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
github-actions
Bot
added
size/L
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Around line 314-316: Remove the stale unbounded pagination header from both
helpers so only the bounded loop remains; in fetchContributors and fetchIssues,
keep the maxPages-based for (let page = 1; page <= maxPages; page++) form and
delete the leftover for (let page = 1; ; page++) line that leaves an extra open
block and breaks the file.
- Around line 338-340: Remove the stale outer pagination loop in fetchIssues and
keep only the bounded page loop driven by maxPages. Update the control flow
around the pagination logic in fetchIssues so there is a single, properly braced
for-loop using page and maxPages, with no nested infinite loop remaining.
🪄 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: 790b873f-3ead-4903-8bf8-b0cc663bc923
📒 Files selected for processing (1)
src/services/github.js
| for (let page = 1; ; page++) { | ||
| const maxPages = pat ? 10 : 1 | ||
| for(let page = 1; page<=maxPages ; page++) { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the leftover loop header and brace balance in the two helpers.
rg -nP 'for\s*\(let page' src/services/github.js
node -e "require('fs').readFileSync('src/services/github.js','utf8')" 2>&1 | head -5 || trueRepository: AOSSIE-Org/OrgExplorer
Length of output: 389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- surrounding lines ---'
sed -n '306,346p' src/services/github.js | cat -n
echo
echo '--- syntax check ---'
node --check src/services/github.js 2>&1 || trueRepository: AOSSIE-Org/OrgExplorer
Length of output: 1716
Remove the duplicated pagination headers in both helpers.
fetchContributors and fetchIssues still keep the old for (let page = 1; ; page++) line above the new bounded loop, which leaves an extra open block and breaks the file. Keep only the bounded for (let page = 1; page <= maxPages; page++) form in both places.
Proposed fix
- for (let page = 1; ; page++) {
- const maxPages = pat ? 10 : 1
- for(let page = 1; page<=maxPages ; page++) {
+ const maxPages = pat ? 10 : 1
+ for (let page = 1; page <= maxPages; page++) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (let page = 1; ; page++) { | |
| const maxPages = pat ? 10 : 1 | |
| for(let page = 1; page<=maxPages ; page++) { | |
| const maxPages = pat ? 10 : 1 | |
| for (let page = 1; page <= maxPages; page++) { |
🤖 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 314 - 316, Remove the stale unbounded
pagination header from both helpers so only the bounded loop remains; in
fetchContributors and fetchIssues, keep the maxPages-based for (let page = 1;
page <= maxPages; page++) form and delete the leftover for (let page = 1; ;
page++) line that leaves an extra open block and breaks the file.
| for (let page = 1; ; page++) { | ||
| const maxPages = pat ? 10 : 1 | ||
| for(let page = 1; page<=maxPages ; page++) { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Remove the stale outer pagination loop in fetchIssues. The bounded loop already handles paging, so keeping for (let page = 1; ; page++) creates nested loops and leaves the braces unbalanced.
Proposed fix
- for (let page = 1; ; page++) {
- const maxPages = pat ? 10 : 1
- for(let page = 1; page<=maxPages ; page++) {
+ const maxPages = pat ? 10 : 1
+ for (let page = 1; page <= maxPages; page++) {🤖 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 338 - 340, Remove the stale outer
pagination loop in fetchIssues and keep only the bounded page loop driven by
maxPages. Update the control flow around the pagination logic in fetchIssues so
there is a single, properly braced for-loop using page and maxPages, with no
nested infinite loop remaining.
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
1 participant
Addressed Issues:
Fixes #98
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