[BUG] GET /api/streams returns stale progress after stream is canceled via API - #664
[BUG] GET /api/streams returns stale progress after stream is canceled via API#664barnabasolutayo-lgtm wants to merge 1 commit into
Conversation
Move cache invalidation to after DB transaction in cancelStream to prevent race condition. Also fix duplicate variable declarations in calculateProgress.
|
Someone is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@barnabasolutayo-lgtm Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughUpdates stream vesting progress calculations and changes cancellation ordering so database state is committed before related caches are invalidated. ChangesStream consistency updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
backend/src/services/streamStore.ts (1)
457-458: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not report future zero-duration streams as fully vested.
When
at < stream.startAt,computeStatusreturns"scheduled", but this branch setsratioto1unconditionally fordurationSeconds <= 0. The response can therefore be scheduled while showing 100% completion and the full vested amount.Proposed fix
- const ratio = stream.durationSeconds <= 0 ? 1 : Math.min(1, elapsed / stream.durationSeconds); + const ratio = + stream.durationSeconds <= 0 + ? effectiveAt >= stream.startAt ? 1 : 0 + : Math.min(1, elapsed / stream.durationSeconds);🤖 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 `@backend/src/services/streamStore.ts` around lines 457 - 458, Update the ratio calculation in computeStatus so zero-duration streams that have not started remain at 0% rather than being treated as fully vested; preserve the existing 100% behavior for zero-duration streams whose start time has been reached, and keep elapsedSeconds consistent with that status.
🤖 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 `@backend/src/services/streamStore.ts`:
- Around line 1187-1192: Update the post-commit cache invalidation in
cancelStream to make each invalidateCache call resilient to rejection, using
Promise.allSettled or equivalent failure handling and logging any invalidation
errors. Ensure resetStatsCache() and resetStreamMetricsCache() always execute
after the committed mutation, while continuing to access the cache through
getCache() and reset stats via resetStatsCache().
---
Outside diff comments:
In `@backend/src/services/streamStore.ts`:
- Around line 457-458: Update the ratio calculation in computeStatus so
zero-duration streams that have not started remain at 0% rather than being
treated as fully vested; preserve the existing 100% behavior for zero-duration
streams whose start time has been reached, and keep elapsedSeconds consistent
with that status.
🪄 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 Plus
Run ID: feca7df6-a0d2-4bee-b350-85af745ed9a2
📒 Files selected for processing (1)
backend/src/services/streamStore.ts
| // Invalidate cache after DB commit so the next read picks up canceled status. | ||
| await invalidateCache(`stream:${id}`); | ||
| await invalidateCache("streams:list:"); | ||
| await invalidateCache("streams:export:"); | ||
| resetStatsCache(); | ||
| resetStreamMetricsCache(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make post-commit cache invalidation resilient.
If any getCache().del() call rejects, cancelStream throws after the SQLite transaction has committed, so the API can return 500 while resetStatsCache() and resetStreamMetricsCache() are skipped. Run invalidations independently with Promise.allSettled or try/finally, log failures, and always reset dependent caches.
As per coding guidelines, backend TypeScript code must access the in-memory LRU cache through getCache() and call resetStatsCache() after mutations that affect stats.
🤖 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 `@backend/src/services/streamStore.ts` around lines 1187 - 1192, Update the
post-commit cache invalidation in cancelStream to make each invalidateCache call
resilient to rejection, using Promise.allSettled or equivalent failure handling
and logging any invalidation errors. Ensure resetStatsCache() and
resetStreamMetricsCache() always execute after the committed mutation, while
continuing to access the cache through getCache() and reset stats via
resetStatsCache().
Source: Coding guidelines
|
This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
2 participants
##closes #600
Move cache invalidation to after DB transaction in cancelStream to prevent race condition. Also fix duplicate variable declarations in calculateProgress.
What changed
Testing done
Related issues
Closes #
Checklist
Summary by CodeRabbit