fix: validate ActivityFeed API response schema#507
Merged
mehul-m-prajapati merged 1 commit intoMay 29, 2026
Merged
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThe PR adds comprehensive error handling and runtime response validation to the ChangesActivity Feed Error Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Improves runtime safety of the ActivityFeed React component by validating GitHub API responses at runtime and surfacing structured, user-friendly errors instead of letting unexpected responses crash rendering.
Changes:
- Added a runtime type-guard (
isEventTypeArray) to validate the/users/:username/eventspayload before setting component state. - Added structured error handling via
ActivityFeedError+FetchErrorstate, including special-casing for rate-limit and 404 scenarios. - Updated the UI to show clearer loading, empty-state, and error-state messaging.
Comments suppressed due to low confidence (1)
src/components/ActivityFeed.tsx:166
- The rate-limit helper text says "The limit resets in 1 hour", but GitHub rate limits reset at a specific time (available via
X-RateLimit-Reset) and may be sooner/later than 1 hour depending on when the limit was hit. Consider either computing the reset time from the header or using a non-specific message to avoid misleading users.
<p className="text-xs text-red-600 dark:text-red-400 mt-2">
You've hit GitHub's API rate limit. The limit resets in 1 hour.
</p>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/components/ActivityFeed.tsx (1)
77-141:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent stale/overlapping fetches from overwriting newer state.
Line 77-141 can commit outdated responses when requests overlap (interval tick, slow network, or username change), which can show the wrong feed/error state.
Suggested fix (ignore stale responses + cleanup-safe updates)
useEffect(() => { + let isActive = true; + let latestRequestId = 0; + const fetchEvents = async () => { + const requestId = ++latestRequestId; try { setLoading(true); setError(null); const res = await fetch( `https://api.github.com/users/${username}/events` ); @@ - setEvents(data); + if (!isActive || requestId !== latestRequestId) return; + setEvents(data); } catch (err) { + if (!isActive || requestId !== latestRequestId) return; const fetchError: FetchError = { message: "Failed to fetch activity. Please try again.", isRateLimited: false, }; @@ setError(fetchError); console.error("ActivityFeed fetch error:", fetchError); setEvents([]); } finally { - setLoading(false); + if (isActive && requestId === latestRequestId) { + setLoading(false); + } } }; fetchEvents(); const interval = setInterval(fetchEvents, 30000); - return () => clearInterval(interval); + return () => { + isActive = false; + clearInterval(interval); + }; }, [username]);Also applies to: 146-148
🤖 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/components/ActivityFeed.tsx` around lines 77 - 141, The fetchEvents useEffect can let slower responses overwrite newer state; fix it by making fetchEvents cancellation-safe: create an AbortController (or increment a local requestId/flag) inside the useEffect, pass its signal to fetch, and in the finally/then blocks only call setEvents, setError, and setLoading if the request wasn't aborted and the requestId matches the latest; also abort the controller (or flip the flag) in the effect cleanup so overlapping interval ticks or username changes can't commit stale responses; this touches useEffect, fetchEvents, ActivityFeedError handling, and the setEvents/setError/setLoading calls.
🤖 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/components/ActivityFeed.tsx`:
- Around line 89-103: The code currently treats any 403 as a rate-limit by using
isRateLimited = res.status === 403; change this to detect rate limiting from
response headers and message: inspect res.headers for x-ratelimit-remaining,
x-ratelimit-reset, Retry-After (and treat remaining === "0" or presence of
Retry-After as rate-limited), and also check errorData.message or errorData for
rate-limit phrases like "rate limit" or "secondary rate limit"; then set
isRateLimited based on those signals and use that boolean when constructing the
ActivityFeedError (referencing isRateLimited, res.headers, and
errorData.message) so only true rate-limit responses produce the rate-limit
message.
---
Outside diff comments:
In `@src/components/ActivityFeed.tsx`:
- Around line 77-141: The fetchEvents useEffect can let slower responses
overwrite newer state; fix it by making fetchEvents cancellation-safe: create an
AbortController (or increment a local requestId/flag) inside the useEffect, pass
its signal to fetch, and in the finally/then blocks only call setEvents,
setError, and setLoading if the request wasn't aborted and the requestId matches
the latest; also abort the controller (or flip the flag) in the effect cleanup
so overlapping interval ticks or username changes can't commit stale responses;
this touches useEffect, fetchEvents, ActivityFeedError handling, and the
setEvents/setError/setLoading calls.
🪄 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: df365c70-9185-453d-8499-6878fbb3a899
📒 Files selected for processing (1)
src/components/ActivityFeed.tsx
|
🎉🎉 Thank you for your contribution! Your PR #507 has been merged! 🎉🎉 |
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.
Related Issue
Description
Implemented runtime validation and improved error handling for the
ActivityFeedcomponent to prevent unsafe API responses from causing runtime failures.Changes made:
res.okActivityFeedErrorclass for structured error handlingEventType[]repo.namefieldThis improves runtime safety, API reliability, and overall stability of the ActivityFeed component.
How Has This Been Tested?
Screenshots (if applicable)
N/A
Type of Change
###Suggested Labels :
gssoc'26,gssoc:approved , type:fix , level:intermediate , quality:clean
Summary by CodeRabbit