fix: Handle missing leaderboard JSON gracefully instead of crashing#263
fix: Handle missing leaderboard JSON gracefully instead of crashing#263notlevi911 wants to merge 3 commits intoCircuitVerse:mainfrom
Conversation
✅ Deploy Preview for cv-community-dashboard ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe leaderboard page now validates the Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
app/leaderboard/[period]/page.tsx (3)
64-120: Extract duplicated fallback data and rendering into a helper.The
fallbackDataobject (lines 66-74 and 98-106) and theSuspense+LeaderboardViewJSX block are repeated three times. Extract both to reduce duplication.♻️ Proposed refactor
Add a helper function near the top of the file (after the type/constant declarations):
function renderLeaderboard( data: LeaderboardJSON, isGridView: boolean ) { return ( <Suspense fallback={<LeaderboardSkeleton count={10} variant={isGridView ? "grid" : "list"} />}> <LeaderboardView entries={data.entries} period={data.period} startDate={new Date(data.startDate)} endDate={new Date(data.endDate)} topByActivity={data.topByActivity} hiddenRoles={data.hiddenRoles} /> </Suspense> ); } function createFallbackData(period: "week" | "month" | "year"): LeaderboardJSON { return { period, updatedAt: Date.now(), startDate: new Date().toISOString(), endDate: new Date().toISOString(), entries: [], topByActivity: {}, hiddenRoles: [], }; }Then the page function simplifies to:
if (!fs.existsSync(filePath)) { - const fallbackData: LeaderboardJSON = { - period, - updatedAt: Date.now(), - startDate: new Date().toISOString(), - endDate: new Date().toISOString(), - entries: [], - topByActivity: {}, - hiddenRoles: [] - }; - - return ( - <Suspense fallback={<LeaderboardSkeleton count={10} variant={isGridView ? "grid" : "list"} />}> - <LeaderboardView - entries={fallbackData.entries} - period={period} - startDate={new Date(fallbackData.startDate)} - endDate={new Date(fallbackData.endDate)} - topByActivity={fallbackData.topByActivity} - hiddenRoles={fallbackData.hiddenRoles} - /> - </Suspense> - ); + return renderLeaderboard(createFallbackData(period), isGridView); } let data: LeaderboardJSON; try { const file = fs.readFileSync(filePath, "utf-8"); data = JSON.parse(file); } catch (error) { console.error(`Failed to parse leaderboard data for ${period}:`, error); - - // fallback for corrupted JSON files - const fallbackData: LeaderboardJSON = { - period, - updatedAt: Date.now(), - startDate: new Date().toISOString(), - endDate: new Date().toISOString(), - entries: [], - topByActivity: {}, - hiddenRoles: [] - }; - - return ( - <Suspense fallback={<LeaderboardSkeleton count={10} variant={isGridView ? "grid" : "list"} />}> - <LeaderboardView - entries={fallbackData.entries} - period={period} - startDate={new Date(fallbackData.startDate)} - endDate={new Date(fallbackData.endDate)} - topByActivity={fallbackData.topByActivity} - hiddenRoles={fallbackData.hiddenRoles} - /> - </Suspense> - ); + return renderLeaderboard(createFallbackData(period), isGridView); } - return ( - <Suspense fallback={<LeaderboardSkeleton count={10} variant={isGridView ? "grid" : "list"} />}> - <LeaderboardView - entries={data.entries} - period={period} - startDate={new Date(data.startDate)} - endDate={new Date(data.endDate)} - topByActivity={data.topByActivity} - hiddenRoles={data.hiddenRoles} - /> - </Suspense> - ); + return renderLeaderboard(data, isGridView);
94-95: Use a structured logger instead ofconsole.error.As per coding guidelines, use a structured logger with
debug,info,warn, anderrorlevels rather thanconsole.error.
90-93: Parsed JSON is not validated against the expected shape.
JSON.parsesucceeds on any valid JSON. If the file contains valid JSON but with an unexpected structure (e.g., missingentriesortopByActivity), the cast toLeaderboardJSONwon't catch it and the component will receive malformed props, potentially causing a runtime error. Consider adding a lightweight schema check (e.g., verifyArray.isArray(data.entries)) and falling back if it fails.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/leaderboard/`[period]/page.tsx:
- Line 93: Change the type guard signature for isValidLeaderboardData from
accepting any to accepting unknown, and update the function body to properly
narrow and validate the unknown value (using typeof checks, Array.isArray, and
property existence checks against the expected LeaderboardJSON shape) so the
type guard compiles and still returns data is LeaderboardJSON; reference the
isValidLeaderboardData function and the LeaderboardJSON type when making the
change.
🧹 Nitpick comments (2)
app/leaderboard/[period]/page.tsx (2)
151-166: Use a structured logger instead ofconsole.error.The coding guidelines call for a structured logger with debug/info/warn/error levels rather than raw
console.error. If the project has a shared logger utility, use it here; otherwise, consider introducing one for server-side code to enable consistent log formatting and level control.As per coding guidelines, "Use structured logger from context with debug, info, warn, and error levels".
52-62: Minor:createFallbackDataproduces potentially misleading dates.
startDateandendDateare both set tonew Date().toISOString()(i.e., the same timestamp). If any downstream code computes a date range or displays it, this could look odd. Consider using more meaningful defaults (e.g., empty strings or the actual period boundaries) if the dates are surfaced in the UI.
All issues have been addressed: ✅ Line 93 - Type Guard Fixed: Changed ✅ Structured Logger Implemented: Created ✅ Meaningful Fallback Dates: Fixed All changes have been tested and TypeScript compilation passes successfully. |
Description
Replaces hard error throwing with graceful fallbacks when leaderboard JSON files are missing or corrupted. Instead of crashing with a 500 error, the page now displays an empty state with the existing "No contributors with points in this period" message.
Related Issue
Fixes #252
Type of change
Checklist
Screenshots (if applicable)
Not applicable - this is an error handling improvement that prevents crashes rather than adding visual changes.
Summary by CodeRabbit
Bug Fixes
Refactor