Refactor: extract paginatedFetch and fetchWithRetry into src/api-utils.ts#20
Merged
Refactor: extract paginatedFetch and fetchWithRetry into src/api-utils.ts#20
Conversation
|
Coverage after merging refactor/dry-api-pagination-retry into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors GitHub API access by extracting shared pagination and retry behavior out of src/api.ts into a dedicated utility module, with accompanying unit tests, and then updates the API client to use those helpers.
Changes:
- Added
fetchWithRetry(429/503 retry with backoff/jitter) andpaginatedFetchhelpers insrc/api-utils.ts. - Added unit tests for the new helpers in
src/api-utils.test.ts. - Refactored
src/api.tsto deduplicate GitHub request headers and replace several directfetch/ pagination loops with the new helpers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/api.ts | Uses fetchWithRetry/paginatedFetch, deduplicates GitHub headers, and adjusts error handling/logging for team repo fetches. |
| src/api-utils.ts | Introduces shared retry + pagination helpers intended for GitHub API usage. |
| src/api-utils.test.ts | Adds unit tests for retry logic and pagination behavior. |
shouze
force-pushed
the
refactor/dry-api-pagination-retry
branch
from
c727a85 to
7f2887c
Compare
|
Coverage after merging refactor/dry-api-pagination-retry into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
- Add src/api-utils.ts with two pure-async helpers:
- paginatedFetch<T>: replaces the three duplicated while-pagination loops
- fetchWithRetry: retries on 429/503 with exponential backoff and Retry-After
header support; returns last response after maxRetries exhausted
- Add src/api-utils.test.ts with 14 unit tests (mocked fetch, instant timers)
- Refactor src/api.ts to consume the new helpers:
- searchCode: use fetchWithRetry instead of bare fetch
- fetchAllResults: use fetchWithRetry for raw file content resolution
- fetchRepoTeams (list teams): replace while loop with paginatedFetch
- fetchRepoTeams (repos per team): replace while loop with paginatedFetch
- Fix: silent break on HTTP error for team repos now logs a warning and
returns [], instead of swallowing errors silently
- Extract githubHeaders() helper to deduplicate header objects
Closes #17
shouze
force-pushed
the
refactor/dry-api-pagination-retry
branch
from
7f2887c to
4690cb8
Compare
|
Coverage after merging refactor/dry-api-pagination-retry into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause / motivation
src/api.tscontained duplicated pagination patterns and no rate-limit handling. A 429 or 503 from GitHub caused an immediate crash or silent data loss (thebreakon team-repo errors was swallowing errors without any diagnostic).Changes
New:
src/api-utils.tsTwo async helpers (performs network I/O — documented in
AGENTS.mdas part of the allowed side-effectful surface alongsideapi.ts):paginatedFetch<T>(fetchPage, pageSize?, delayMs?)— callsfetchPage(page)starting at 1, accumulates results, stops when a page returns fewer items thanpageSize.fetchWithRetry(url, options, maxRetries?)— retries on 429 and 503 with exponential backoff (1s × 2^attempt, capped at 60 s, ±10 % jitter). ReadsRetry-Afterheader when present. Drains the response body withres.body?.cancel()before sleeping to allow HTTP connection reuse. Returns the lastResponseaftermaxRetriesexhausted — callers still checkres.ok.New:
src/api-utils.test.ts14 unit tests covering all branches with mocked
fetchand instantsetTimeout.Refactored:
src/api.tssearchCode: barefetch→fetchWithRetryfetchAllResults(search pagination):whileloop →paginatedFetch(250 ms inter-page delay preserved)fetchAllResults(raw file resolution): barefetch→fetchWithRetryfetchRepoTeams(list teams): barefetch→fetchWithRetry; pagination remains a manualwhileloop to filter matching slugs per page and avoid accumulating the full team list in memory for large orgsfetchRepoTeams(repos per team): barefetch→fetchWithRetry; pagination remains a manualwhileloop to updaterepoTeamsincrementally per page and avoid holding a full repos array per team in memorybreakon HTTP error for team repos now reads and logs the body (truncated at 200 chars) as adimwarning; 404 (nested/secret teams) is still silentgithubHeaders()helper to deduplicate the three identical header objectsUpdated:
AGENTS.mdapi-utils.tsto the project layout tableapi-utils.tsHow to verify
Behaviour
No observable behaviour change for the end user. The only semantic differences are:
Closes #17