Make service worker network-first instead of cache-first - #1022
Open
FlatterAtMainz wants to merge 9 commits into
Open
Make service worker network-first instead of cache-first#1022FlatterAtMainz wants to merge 9 commits into
FlatterAtMainz wants to merge 9 commits into
Conversation
.output-control__inner capped its height with max-height on mobile but used overflow-y: visible, so a snapserver device/group list taller than the popup just overflowed off-screen with no way to scroll to it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
max-height: calc(100vh - 140px) sizes the popup using the full layout viewport, which on mobile includes space the browser's address bar can occupy. When the address bar is visible, the actually-visible viewport is shorter than 100vh, so the popup (anchored to the bottom, growing upward) extends above what's visible, hiding its top row. Add a 100dvh override, which tracks the real visible viewport, after the 100vh fallback for browsers that don't support dvh. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
On mobile the popup only auto-opens via PlaybackControls expanding (force_expanded=true), which also hides the popup's own close button (display:none in CSS) and suppressed its own click-outside overlay. That left no way to dismiss just the output-control popup - only tapping in the empty space above it (collapsing the whole playback bar) worked. Always render the click-outside overlay regardless of force_expanded so the popup can close independently. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Errors surfaced through handleException often ended up either blank or as a bare technical fragment (e.g. "Could not load your profile" with no explanation, or a raw "SyntaxError: JSON.parse: unexpected end of input"). Add a humanizeError util that recognises common failure shapes (invalid/empty JSON responses, network failures, expired/revoked OAuth grants, known HTTP status codes) and prefixes the description with a plain-language explanation, applied centrally in the HANDLE_EXCEPTION handler so every existing call site benefits. Also: - handleException now falls back to a plain Error object's .message for the description, and no longer throws when data.error is undefined (message derivation had the same unguarded access). - Wrap the JSON.parse of xhr.responseText in HANDLE_EXCEPTION in a try/catch - a non-JSON error response would otherwise throw inside the exception handler itself. - Spotify getMe() and refreshToken() failures now get specific messages pointing at expired/revoked authorization, since that was the actual root cause behind the vague errors we hit in practice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… optional chaining - Humanized error explanations now live under errors.* in en.yaml (via i18n()) instead of being hardcoded in JS, so they're translatable like the rest of the UI. - Replace `x && x.y` null checks with optional chaining per review feedback.
…ailures getSearchResults never resolved its process notification on request failure or on cancel-button click, leaving it stuck in the UI forever. The five Spotify library-fetch actions had the same underlying gap (no rejection handler at all), silently swallowing errors and leaking stuck process state on any failed request.
The fetch handler served every cached response indefinitely with no invalidation path: the cache is only purged in 'activate', which only fires when the service worker script's own bytes change - something a plain webpack rebuild of app.min.js/app.min.css never does. In practice, once a browser cached the app bundle on first visit, it kept serving that exact bundle forever, silently masking every subsequent deploy (bug fixes included) until a user manually cleared the service worker/cache storage. Flip to network-first: always attempt a live fetch first (still caching successful GET responses under the same blacklist rules as before), and only fall back to the cache when the network request itself fails. The cache now serves its intended purpose - an offline fallback - without being able to hide live updates from users who are online.
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
1 participant
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.
Summary
fetchhandler was cache-first with no real invalidation path:activateonly purges old caches when the service worker script's own bytes change, which a normal webpack rebuild ofapp.min.js/app.min.cssnever does (fixed filenames, no content hash).Test plan
me/tracks,me/albums,following/contains, etc.) are still never cached.