fix: bump service worker cache to v4 and evict stale caches (#99)#100
fix: bump service worker cache to v4 and evict stale caches (#99)#100
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…te (#99) After the js-tts-wrapper migration (PR #89), browsers with the service worker installed were serving old cached chunks that still called the removed /api/tts/speak route, causing 404 errors and TTS failure. Bumping CACHE to podium-v4 forces all clients to fetch fresh bundles. Added old-cache cleanup in the activate handler so stale caches are automatically deleted on each SW update going forward.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 47 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughChanges modify TTS audio caching from concurrent to sequential processing with updated abort handling in OnlineTalkPage, and bump the service worker cache version from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/talk/[id]/OnlineTalkPage.tsx (1)
150-180:⚠️ Potential issue | 🟠 MajorReturn on abort before saving prepared-state metadata.
Breaking out of the loop still falls through to
saveTalkPreparedState(...)below. After cleanup runs, Line 188 clearscurrentAudioUrls, so an aborted run can persistcachedAudioSegments: 0/hasAudio: falseeven when some blobs were already written to IndexedDB.Suggested change
for (const { segmentIndex, cacheKey, segment } of misses) { if (signal.aborted) break; try { const ttsText = isAzure && segment.elements ? buildSSML(segment.elements as SegmentElement[]) : segment.text; const blob = await fetchTTSBlob(ttsText, activeTtsConfig); if (signal.aborted) break; await setCachedAudio(cacheKey, blob); audioUrls.current.set(segmentIndex, URL.createObjectURL(blob)); } catch (err) { if ((err as Error).name === 'AbortError') break; hadFailure = true; setCacheFailed(true); } finally { if (!signal.aborted) { completed++; setCacheLoaded(completed); if (completed === segments.length) setCacheReady(true); } } } + + if (signal.aborted) return; await saveTalkPreparedState(userId, id, { talkId: id, hasDocument: true, hasAudio: !hadFailure && currentAudioUrls.size === segments.length,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/talk/`[id]/OnlineTalkPage.tsx around lines 150 - 180, The loop that processes `misses` can break on `signal.aborted` but the function still falls through to calling `saveTalkPreparedState(...)`, causing incorrect metadata since cleanup later clears `currentAudioUrls`; update the control flow so that if `signal.aborted` is observed (inside the for loop or immediately after it) you return early (or otherwise skip calling `saveTalkPreparedState`) to avoid persisting partial/incorrect state — adjust logic around the `for (const { segmentIndex, cacheKey, segment } of misses)` loop and the subsequent call to `saveTalkPreparedState`, ensuring checks for `signal.aborted` occur before calling `setCachedAudio`, `setCacheFailed`, or `saveTalkPreparedState` and referencing `currentAudioUrls`, `setCacheLoaded`, and `setCacheReady` as needed.
🧹 Nitpick comments (1)
public/sw.js (1)
21-25: Scope cache eviction to Podium-owned caches.
keys.filter((k) => k !== CACHE)removes every other CacheStorage entry for this origin, not just old Podium service-worker caches. That makes future unrelated caches easy to wipe accidentally. Filter by your cache namespace instead.Suggested change
self.addEventListener('activate', (event) => { event.waitUntil( caches.keys() - .then((keys) => Promise.all(keys.filter((k) => k !== CACHE).map((k) => caches.delete(k)))) + .then((keys) => Promise.all( + keys + .filter((k) => k.startsWith('podium-v') && k !== CACHE) + .map((k) => caches.delete(k)) + )) .then(() => self.clients.claim()) ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/sw.js` around lines 21 - 25, The activate handler currently deletes all caches except the current CACHE which can remove unrelated origin caches; update the eviction to only target Podium-owned caches by filtering cache keys using your Podium namespace (e.g., a prefix or pattern) instead of comparing to CACHE directly. In the activate event listener where caches.keys() is used (and where CACHE is referenced), change keys.filter((k) => k !== CACHE) to filter keys that startWith/contain your Podium cache prefix (or match a RegExp) and then delete only those matching keys, keeping unrelated caches untouched and still removing old Podium caches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/talk/`[id]/OnlineTalkPage.tsx:
- Around line 150-180: The loop that processes `misses` can break on
`signal.aborted` but the function still falls through to calling
`saveTalkPreparedState(...)`, causing incorrect metadata since cleanup later
clears `currentAudioUrls`; update the control flow so that if `signal.aborted`
is observed (inside the for loop or immediately after it) you return early (or
otherwise skip calling `saveTalkPreparedState`) to avoid persisting
partial/incorrect state — adjust logic around the `for (const { segmentIndex,
cacheKey, segment } of misses)` loop and the subsequent call to
`saveTalkPreparedState`, ensuring checks for `signal.aborted` occur before
calling `setCachedAudio`, `setCacheFailed`, or `saveTalkPreparedState` and
referencing `currentAudioUrls`, `setCacheLoaded`, and `setCacheReady` as needed.
---
Nitpick comments:
In `@public/sw.js`:
- Around line 21-25: The activate handler currently deletes all caches except
the current CACHE which can remove unrelated origin caches; update the eviction
to only target Podium-owned caches by filtering cache keys using your Podium
namespace (e.g., a prefix or pattern) instead of comparing to CACHE directly. In
the activate event listener where caches.keys() is used (and where CACHE is
referenced), change keys.filter((k) => k !== CACHE) to filter keys that
startWith/contain your Podium cache prefix (or match a RegExp) and then delete
only those matching keys, keeping unrelated caches untouched and still removing
old Podium caches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5acff146-4066-4e96-ae98-e6523b5b90ae
📒 Files selected for processing (2)
app/talk/[id]/OnlineTalkPage.tsxpublic/sw.js
Instead of a manually-bumped hardcoded version string, the SW cache key is now derived from the app version: - next.config.ts exposes npm_package_version as NEXT_PUBLIC_APP_VERSION - layout.tsx registers /sw.js?v=<version> so the query string changes automatically on each release - sw.js reads self.location.search to build its cache key (podium-<version>) Cache busting now happens automatically whenever package.json version is bumped — no manual SW edits required.
2 participants
Summary
podium-v4to evict old/_next/static/chunks cached by the previouscacheFirststrategyactivateevent so stale caches are automatically deleted on every future SW updateProblem
After PR #89 replaced the
/api/tts/speakserver-side proxy with directjs-tts-wrapperbrowser calls, browsers that already had the service worker installed kept serving old cached JS bundles. Those bundles still contained the oldfetch('/api/tts/speak', ...)call, causing repeated 404 errors and complete TTS failure.Test plan
/api/tts/speak404sCloses #99
Summary by CodeRabbit