Add GitHub API rate limit handling UI#560
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthrough
ChangesGitHub Contributors Rate-Limit Error Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @vivekktrivedi06-hue for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/pages/Contributors/Contributors.tsx (1)
46-65: ⚡ Quick winRemove temporary commented test code from the component.
This debug block adds noise in production code and is easy to forget during future edits. Prefer keeping this in tests or local-only mocks.
Proposed cleanup
- /* - // ================================================== - // TEMPORARY TESTING CODE - // Uncomment this block ONLY if you want to test - // the rate limit UI manually. - // ================================================== - - // // throw { - // // isAxiosError: true, - // // response: { - // // status: 403, - // // headers: { - // // "x-ratelimit-remaining": 0, - // // "x-ratelimit-reset": 1750000000, - // // }, - // // }, - // // }; - - // ================================================== - // */🤖 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/pages/Contributors/Contributors.tsx` around lines 46 - 65, Remove the temporary commented-out testing block in Contributors.tsx (the multi-line comment that contains the simulated Axios rate-limit throw) from the Contributors component file; locate the commented section between the marker lines "TEMPORARY TESTING CODE" and the closing "*/" and delete it so only production code remains, or move any necessary test logic into dedicated test/mocks rather than leaving commented debug code in Contributors.tsx.
🤖 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/pages/Contributors/Contributors.tsx`:
- Around line 223-225: The anchor rendered for contributors uses target="_blank"
on href={contributor.html_url} (in Contributors.tsx) without a rel attribute;
update the Link/anchor element that sets href={contributor.html_url} and
target="_blank" to also include rel="noopener noreferrer" to prevent tab-napping
and improve security (ensure the change is applied where the contributor link is
created, e.g., the element rendering contributor.html_url).
- Around line 70-91: The code currently assumes any HTTP 403 is a GitHub
rate-limit and directly interpolates headers that may be absent; update the 403
branch in the Contributors component to first check for the presence of
rate-limit headers (error.response.headers["x-ratelimit-remaining"] and
["x-ratelimit-reset"]) and only treat it as a rate-limit when the headers exist,
otherwise setError to a generic 403 authorization/permission message that
includes safe fallbacks (e.g., statusText or error.message) instead of
interpolating undefined; when building the rate-limit message use a default
value (like "unknown" or "N/A") for remaining and only compute/resetMessage if
x-ratelimit-reset is present before calling setError.
---
Nitpick comments:
In `@src/pages/Contributors/Contributors.tsx`:
- Around line 46-65: Remove the temporary commented-out testing block in
Contributors.tsx (the multi-line comment that contains the simulated Axios
rate-limit throw) from the Contributors component file; locate the commented
section between the marker lines "TEMPORARY TESTING CODE" and the closing "*/"
and delete it so only production code remains, or move any necessary test logic
into dedicated test/mocks rather than leaving commented debug code in
Contributors.tsx.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44ce97d8-2ea5-48c3-88b8-0b134eab9987
📒 Files selected for processing (1)
src/pages/Contributors/Contributors.tsx
| if (error.response?.status === 403) { | ||
|
|
||
| const remaining = | ||
| error.response.headers["x-ratelimit-remaining"]; | ||
|
|
||
| const reset = | ||
| error.response.headers["x-ratelimit-reset"]; | ||
|
|
||
| let resetMessage = ""; | ||
|
|
||
| if (reset) { | ||
| const resetTime = new Date( | ||
| Number(reset) * 1000 | ||
| ); | ||
|
|
||
| resetMessage = | ||
| ` Try again after ${resetTime.toLocaleTimeString()}.`; | ||
| } | ||
|
|
||
| setError( | ||
| `GitHub API rate limit exceeded. Remaining requests: ${remaining}.${resetMessage}` | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In GitHub REST API, are all HTTP 403 responses considered rate-limit errors? What is the recommended way to distinguish rate-limit responses (e.g., x-ratelimit-remaining/reset headers and response message) from other forbidden responses?
💡 Result:
No, not all HTTP 403 (Forbidden) responses from the GitHub REST API are caused by rate-limit errors [1][2]. A 403 status code is a general response indicating that the server refuses to authorize the request, which can occur due to insufficient permissions, invalid credentials, or other access-related issues, in addition to both primary and secondary rate limiting [1][3][4]. To distinguish a rate-limit error from other types of 403 Forbidden responses, you should examine the response body and specific HTTP headers [1][3][5]. Recommended approach to identify and handle rate limits: 1. Check the response body: GitHub explicitly includes information in the error message when a rate limit has been exceeded [1][3]. For example, messages regarding secondary rate limits will explicitly state that a secondary rate limit was exceeded [1][3]. 2. Check for the Retry-After header: If this header is present, it is a strong indicator that you have hit a rate limit (typically a secondary rate limit), and the value specifies how many seconds you must wait before retrying [1][3][5]. 3. Check x-ratelimit-remaining: If the x-ratelimit-remaining header is 0, it confirms that your primary rate limit has been exhausted [1][3][5]. The x-ratelimit-reset header will provide the UTC epoch time when your limit will reset [1][3][5]. If a 403 response does not contain these specific rate-limit indicators, it is likely a standard authorization or permission error [2][6][4]. In such cases, retrying will not resolve the issue and may be considered abusive behavior [1][5]. For rate-limit errors, follow the prescribed backoff strategies—such as waiting until the time specified by the x-ratelimit-reset header or using exponential backoff if a retry-after header is present—to remain compliant and avoid potential banning of your integration [1][3][5].
Citations:
- 1: https://docs.github.com/en/rest/using-the-rest-api/troubleshooting-the-rest-api?algolia-query=third%2Bparty&apiVersion=2022-11-28
- 2: https://stackoverflow.com/questions/38378337/receiving-github-api-403-error-when-i-have-not-exceeded-my-rate-limit
- 3: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api
- 4: https://dev.to/coding_sprint_e9acaaa6d35/mastering-nodejs-http-module-build-servers-rest-apis-and-handle-requests-953
- 5: https://github.com/github/docs/blob/main/content/rest/using-the-rest-api/best-practices-for-using-the-rest-api.md
- 6: https://dev.to/137foundry/how-to-implement-exponential-backoff-for-rate-limited-apis-in-python-28b5
Avoid mislabeling all GitHub 403s as rate limits (and prevent undefined in error text)
At src/pages/Contributors/Contributors.tsx (lines 70-91), the code treats every 403 as a rate-limit issue, but GitHub can return 403 for many non-rate-limit authorization/permission problems. Also, it interpolates x-ratelimit-remaining directly, which can show undefined to users when the header isn’t present.
Proposed hardening
- if (error.response?.status === 403) {
- const remaining =
- error.response.headers["x-ratelimit-remaining"];
- const reset =
- error.response.headers["x-ratelimit-reset"];
+ if (error.response?.status === 403) {
+ const remainingHeader =
+ error.response.headers["x-ratelimit-remaining"];
+ const resetHeader =
+ error.response.headers["x-ratelimit-reset"];
+ const message = String(error.response.data?.message ?? "").toLowerCase();
+ const isRateLimit =
+ remainingHeader === "0" || message.includes("rate limit");
+
+ if (!isRateLimit) {
+ setError("Access forbidden. Please verify permissions and try again.");
+ return;
+ }
let resetMessage = "";
-
- if (reset) {
+ if (resetHeader && !Number.isNaN(Number(resetHeader))) {
const resetTime = new Date(
- Number(reset) * 1000
+ Number(resetHeader) * 1000
);
resetMessage =
` Try again after ${resetTime.toLocaleTimeString()}.`;
}
setError(
- `GitHub API rate limit exceeded. Remaining requests: ${remaining}.${resetMessage}`
+ `GitHub API rate limit exceeded. Remaining requests: ${remainingHeader ?? "unknown"}.${resetMessage}`
);📝 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.
| if (error.response?.status === 403) { | |
| const remaining = | |
| error.response.headers["x-ratelimit-remaining"]; | |
| const reset = | |
| error.response.headers["x-ratelimit-reset"]; | |
| let resetMessage = ""; | |
| if (reset) { | |
| const resetTime = new Date( | |
| Number(reset) * 1000 | |
| ); | |
| resetMessage = | |
| ` Try again after ${resetTime.toLocaleTimeString()}.`; | |
| } | |
| setError( | |
| `GitHub API rate limit exceeded. Remaining requests: ${remaining}.${resetMessage}` | |
| ); | |
| if (error.response?.status === 403) { | |
| const remainingHeader = | |
| error.response.headers["x-ratelimit-remaining"]; | |
| const resetHeader = | |
| error.response.headers["x-ratelimit-reset"]; | |
| const message = String(error.response.data?.message ?? "").toLowerCase(); | |
| const isRateLimit = | |
| remainingHeader === "0" || message.includes("rate limit"); | |
| if (!isRateLimit) { | |
| setError("Access forbidden. Please verify permissions and try again."); | |
| return; | |
| } | |
| let resetMessage = ""; | |
| if (resetHeader && !Number.isNaN(Number(resetHeader))) { | |
| const resetTime = new Date( | |
| Number(resetHeader) * 1000 | |
| ); | |
| resetMessage = | |
| ` Try again after ${resetTime.toLocaleTimeString()}.`; | |
| } | |
| setError( | |
| `GitHub API rate limit exceeded. Remaining requests: ${remainingHeader ?? "unknown"}.${resetMessage}` | |
| ); |
🤖 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/pages/Contributors/Contributors.tsx` around lines 70 - 91, The code
currently assumes any HTTP 403 is a GitHub rate-limit and directly interpolates
headers that may be absent; update the 403 branch in the Contributors component
to first check for the presence of rate-limit headers
(error.response.headers["x-ratelimit-remaining"] and ["x-ratelimit-reset"]) and
only treat it as a rate-limit when the headers exist, otherwise setError to a
generic 403 authorization/permission message that includes safe fallbacks (e.g.,
statusText or error.message) instead of interpolating undefined; when building
the rate-limit message use a default value (like "unknown" or "N/A") for
remaining and only compute/resetMessage if x-ratelimit-reset is present before
calling setError.
| href={contributor.html_url} | ||
| target="_blank" | ||
| sx={{ |
There was a problem hiding this comment.
Add rel on the external link opened in a new tab.
Line 224 uses target="_blank" without rel="noopener noreferrer", which is a security gap.
Proposed fix
<Button
variant="contained"
startIcon={<FaGithub />}
href={contributor.html_url}
target="_blank"
+ rel="noopener noreferrer"
sx={{📝 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.
| href={contributor.html_url} | |
| target="_blank" | |
| sx={{ | |
| href={contributor.html_url} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| sx={{ |
🤖 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/pages/Contributors/Contributors.tsx` around lines 223 - 225, The anchor
rendered for contributors uses target="_blank" on href={contributor.html_url}
(in Contributors.tsx) without a rel attribute; update the Link/anchor element
that sets href={contributor.html_url} and target="_blank" to also include
rel="noopener noreferrer" to prevent tab-napping and improve security (ensure
the change is applied where the contributor link is created, e.g., the element
rendering contributor.html_url).
1 participant
📌 Description
This PR adds proper handling for GitHub API rate limit errors.
✨ Changes
🔗 Related Issue
Fixes #496
Summary by CodeRabbit
Release Notes
Bug Fixes
UI Improvements