fix: report correct cache status for cold requests (#5503)#5931
Conversation
|
@vipul674 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@JhaSourav07 Ready for review. |
X-Cache-Status was always HIT when bypassCache was not requested, even for first-time/cold requests where no cache entry existed. Now checks if the cache had data before the fetch and reports MISS for cold requests. Fixes JhaSourav07#5503
5062f15 to
08fa6ef
Compare
…sCache in validation tests
Aamod-Dev
left a comment
There was a problem hiding this comment.
Difficulty: beginner – Fixes X-Cache-Status in route.ts by checking cache.has() before fetch, reports MISS for cold requests (5 additions, updates test).
Quality: clean – Simple correctness fix.
Type: bug – Fixes issue #5503, accurate cache reporting.
Good fix!
Aamod-Dev
left a comment
There was a problem hiding this comment.
Review
This PR cannot be approved in its current state due to blocking issues (status:blocked label, merge conflicts, needs-rebase label, and/or failing CI checks). Please resolve the blocking issues and re-request review.
Once unblocked, I'm happy to re-review! 💚
|
@JhaSourav07 All CI checks passing! Fixed the failing tests. What was wrong:
Fix:
CI: Format/Lint/Typecheck/Test pass | Production Build pass | CodeQL pass | Analyze pass |
📦 Next.js Bundle Size Report (Gzipped Sizes)✨ No significant bundle size changes detected. 📊 Summary of Totals
|
|
@Aamod-Dev All blocking issues resolved! CI is now fully passing.
Ready for re-review. 💚 |
Aamod-Dev
left a comment
There was a problem hiding this comment.
Fixing the cache reporting for cold requests is a great quality-of-life improvement for debugging API performance. Adding the mock for contributionsCache.has in the test files keeps the test suite robust. Approved!
|
🎉 Congratulations @vipul674! Your PR has been successfully merged. 🚀 Thank you for contributing to CommitPulse. Your work helps us build a better tool for the community.
Keep building! 💻✨ |
Fixes #5503
Problem
The
X-Cache-Statusheader reportedHITfor all non-bypass requests, even cold/first-time requests where no cache entry existed. This was misleading because the data was freshly fetched from GitHub API, not served from cache.Fix
Check if the cache had data before the fetch using
contributionsCache.has(). Now correctly reports:MISSfor cold requests (no cache entry)HITfor requests served from cacheMISSfor bypass requests