Add offline detection with a generic "You're Offline" screen - #101
Merged
Conversation
Adds app-wide connectivity detection so a LAN-only Jellyfin server being unreachable gets a clear, recoverable UI instead of a silent hang or a bare error message. - ConnectivityMonitor (new): a MainActor singleton, same convention as DeviceTiltObserver.shared, updated from the single choke point every Jellyfin API call already funnels through (JellyfinAPIClient.sendRaw). A transport-level failure reports offline; any real HTTP response (success or an error status) reports online. - AppState gains a Phase.offline case, reached only when session restore at launch can't reach the server at all — kept separate from .login so a user actively signing in never has the screen pulled out from under them by a background connectivity flip. - Home/Search/Collection/AssetDetail show a shared OfflineStateView in place of their "nothing to show yet" states when offline, each with a Retry that reloads just that screen. The tab bar (and Profile) stay reachable throughout since none of this touches MainTabView. - DionysusPlayerApp probes Jellyfin's /health endpoint on every foreground transition to catch a dropped connection on resume. - The pre-login offline screen offers "Go to Settings" (embeds ProfileView directly) rather than jumping straight to "Change Server". - sendRaw races the real request against an explicit 20s timeout instead of relying on .shared's default 60s, which was making an unreachable-but-routable server (e.g. cellular with Wi-Fi off) read as an indefinite hang rather than a prompt offline screen. - AssetDetailView now checks offline-and-not-fully-loaded ahead of its preloaded-item branch — previously a preload from Home made a genuinely offline detail fetch look like a permanently part-loaded page with no way back to a retry. - HomeViewModel tracks dynamic-rail-discovery failures separately from "library has none to offer" and retries once ConnectivityMonitor reports back online, since that fetch fails silently by design and nothing else was retriggering it. Mid-playback connectivity loss: - PlaybackState gains a .reconnecting case, bridged from AetherEngine's .stalled phase instead of being folded into .buffering, so PlayerControlsOverlay can show "Reconnecting…" for an actual dropped source connection. - PlayerViewModel's onStateChange now derives errorMessage from a terminal .failed engine state, not just from start()'s own catch — previously a mid-playback failure just froze the video with no message. PlayerView picks the shared offline screen or the existing generic error view depending on ConnectivityMonitor at that moment; the offline retry resumes in place via a new start(resumeSeconds:) override rather than restarting from zero. Verified on a physical device: launch-time offline (Wi-Fi off), background/foreground resume, per-screen offline+retry, dynamic-rail self-recovery, and item-detail offline handling, via a live console capture across a full reconnect session (Search, item details, a full Show detail, heavy Home scrolling) with no unexpected errors. 489 tests pass (12 new): ConnectivityMonitor, sendRaw's reporting behavior, healthCheck(), AppState's new offline phase, PlayerViewModel's failure/resume handling, and HomeViewModel's dynamic-rail retry logic. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two performance fixes found in review, both internal to sendRaw's transport-failure classification, no behavior change: - sendRaw's 20s request timeout was enforced via a withThrowingTaskGroup racing session.data(for:) against Task.sleep — spinning up an extra Task on every single API call in the app just to get a timeout URLRequest already supports natively. Replaced with a per-request request.timeoutInterval set in makeRequest, honored by URLSession regardless of session config, so it doesn't need the custom-session workaround that broke MockURLProtocol interception last time either. - ConnectivityMonitor.reportFailure()/reportSuccess() wrote isOffline unconditionally. @observable's change tracking fires on every assignment regardless of whether the value actually changes, and these run on every network call app-wide (including the 10s playback-progress heartbeat during video playback) — since four screens (Home/Search/Collection/AssetDetail) read isOffline directly in body, every completed request was re-invalidating and recomputing whichever of those screens was on screen, whether or not connectivity actually changed. Guarded both methods to no-op when the value wouldn't change. Verified: full test suite (492 tests) passes unmodified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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
Adds app-wide offline/connectivity detection for Dionysus Player, primarily used against LAN-only Jellyfin servers.
ConnectivityMonitor— a@MainActor @Observablesingleton (isOffline), the single signal any part of the app can route to. The only writer isJellyfinAPIClient.sendRaw, the choke point every endpoint call funnels through: a transport-level failure (no response at all) reports offline; reaching any real HTTP response — success or an HTTP error status — reports back online.AppState.start()routes a launch-time connectivity failure to a new.offlinephase (distinct from.login);DionysusPlayerAppfires a lightweightGET /healthprobe on every foreground transition; every screen's own load already goes throughsendRaw.OfflineStateView) that Home, Search, Collection, and Asset Detail gate their "nothing to show yet" states on, with a Retry that reloads whatever the user was doing. Already-loaded/preloaded content is never blanked out by a stale/background offline flag..offlinescreen offers a "Go to Settings" button that opensProfileViewdirectly; post-login the tab bar is always present regardless of connectivity state.PlaybackState.reconnectingcase (bridged from AetherEngine's.stalledphase) drives a "Reconnecting…" indicator distinct from ordinary buffering, and a terminal.failedstate now actually surfaces an error overlay (previously silent — the video just froze with no message). When that failure coincides with the app already being offline, the shared offline screen is shown instead, with a retry that resumes in place rather than restarting from zero.Bugs found and fixed via live on-device testing
.shared's default 60s request timeout — fixed with a 20s per-request timeout.AssetDetailViewnow checks offline state before its preloaded-item branch.Follow-up review (second commit)
Two internal performance fixes found reviewing the above, no behavior change:
withThrowingTaskGroupracing againstTask.sleep, spinning up an extraTaskon every API call — replaced with a plain per-requestURLRequest.timeoutInterval.ConnectivityMonitor.reportFailure()/reportSuccess()wroteisOfflineunconditionally, which — since@Observablechange-tracking fires on every assignment regardless of whether the value changes, and four screens readisOfflinedirectly inbody— was re-rendering whichever of those screens was on screen on every single network call app-wide, including the 10s playback-progress heartbeat during video playback. Guarded both to no-op when the value wouldn't change.Testing
🤖 Generated with Claude Code