From 5c0db429b3e1a5f024c346f5ea126883c62a87f6 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 4 Jun 2026 22:57:57 -0400 Subject: [PATCH] =?UTF-8?q?fix(android):=20don't=20flip=20offline=20on=20W?= =?UTF-8?q?AN-validation=20flicker=20=E2=80=94=20trust=20/healthz?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A self-hosted Minstrel server is usually on the LAN, but ConnectivityObserver gated 'online' on NET_CAPABILITY_VALIDATED — which tracks whether Android reached its own WAN internet-validation probe, not whether Minstrel is reachable. A transient WAN/DNS blip (or Android's periodic re-validation) momentarily drops VALIDATED while the LAN server stays reachable. That flipped ServerHealth -> Offline with NO debounce (only the /healthz path got hysteresis), and OfflineGatedDataSource fast-failed the in-flight stream read with OfflineException -> ExoPlayer SOURCE error -> the load_failed 'Source error' event. On-device: 'app said server offline while it wasn't', one track failed, then recovered when VALIDATED returned. - ConnectivityObserver: require INTERNET only, not VALIDATED. The /healthz poll (VersionCheckController, with its own failure hysteresis) is the authority on whether Minstrel is reachable; the device-link signal only answers 'is there a network at all' (airplane mode). - ServerHealthController: add a WARN-tier transition log. The signal had zero instrumentation, which is why this was hard to diagnose from logcat. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../connectivity/ConnectivityObserver.kt | 42 ++++++++++++------- .../connectivity/ServerHealthController.kt | 30 +++++++++---- 2 files changed, 48 insertions(+), 24 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ConnectivityObserver.kt b/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ConnectivityObserver.kt index 49470361..09db0a07 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ConnectivityObserver.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ConnectivityObserver.kt @@ -14,11 +14,23 @@ import javax.inject.Inject import javax.inject.Singleton /** - * Single source of truth for the device's "is the internet usable - * right now" signal — wraps [ConnectivityManager] and exposes a hot - * cold-startable Flow that emits `false` while the active network - * lacks INTERNET + VALIDATED capabilities (airplane mode, no carrier, - * captive portal, etc.) and `true` once a usable network appears. + * Single source of truth for "does the device have a network link at + * all" — wraps [ConnectivityManager] and exposes a hot cold-startable + * Flow that emits `false` only when there is no active INTERNET-capable + * network (airplane mode, no carrier/Wi-Fi) and `true` once any network + * link appears. + * + * Deliberately does NOT require `NET_CAPABILITY_VALIDATED`. VALIDATED + * tracks whether Android reached its own WAN internet-validation probe + * (Google's `generate_204`) — which is the wrong question for a + * self-hosted server that is usually on the LAN. A transient WAN/DNS + * blip (or Android's periodic re-validation) momentarily drops VALIDATED + * while the Minstrel box stays perfectly reachable; gating on it flipped + * the app to Offline with no debounce and fast-failed in-flight playback + * via [com.fabledsword.minstrel.player.OfflineGatedDataSource]. The + * authority on whether *Minstrel* is reachable is the `/healthz` poll + * ([com.fabledsword.minstrel.update.data.VersionCheckController], which + * has its own failure hysteresis), not this coarse device-link signal. * * Used by the shell-level ConnectionErrorBanner; downstream * repositories can also collect this to gate retry loops. @@ -35,36 +47,36 @@ class ConnectivityObserver @Inject constructor( .build() val callback = object : ConnectivityManager.NetworkCallback() { override fun onAvailable(network: Network) { - trySend(hasUsableInternet()) + trySend(hasActiveNetwork()) } override fun onLost(network: Network) { - trySend(hasUsableInternet()) + trySend(hasActiveNetwork()) } override fun onCapabilitiesChanged( network: Network, capabilities: NetworkCapabilities, ) { + // INTERNET only -- NOT VALIDATED. A WAN/validation flicker + // must not read as "device offline" when the LAN (and the + // Minstrel server on it) is still reachable. /healthz is the + // authority on server reachability. trySend( capabilities.hasCapability( NetworkCapabilities.NET_CAPABILITY_INTERNET, - ) && - capabilities.hasCapability( - NetworkCapabilities.NET_CAPABILITY_VALIDATED, - ), + ), ) } } cm.registerNetworkCallback(request, callback) // Seed the initial value so the banner doesn't flash before the // first capability callback fires. - trySend(hasUsableInternet()) + trySend(hasActiveNetwork()) awaitClose { cm.unregisterNetworkCallback(callback) } }.distinctUntilChanged() - private fun hasUsableInternet(): Boolean { + private fun hasActiveNetwork(): Boolean { val caps = cm.activeNetwork?.let { cm.getNetworkCapabilities(it) } return caps != null && - caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) && - caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) + caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) } } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ServerHealthController.kt b/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ServerHealthController.kt index eb6c1a81..a2b83f8d 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ServerHealthController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ServerHealthController.kt @@ -7,7 +7,10 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn +import timber.log.Timber import javax.inject.Inject import javax.inject.Singleton @@ -19,11 +22,14 @@ import javax.inject.Singleton * Composed from two existing signals -- this controller doesn't poll its own * endpoint: * - * - [ConnectivityObserver.online] -- system-level NetworkCallback with the - * INTERNET + VALIDATED capability check (captive portals fail this). + * - [ConnectivityObserver.online] -- system-level NetworkCallback that + * answers only "is there an active INTERNET-capable network link" (NOT + * VALIDATED -- a WAN-validation flicker must not read as offline when + * the LAN server is reachable). * - [VersionCheckController.reachable] -- did the last `/healthz` poll - * succeed. Distinguishes "device has network but our server is down" from - * "no network at all," which the connectivity-only signal can't. + * succeed. This is the authority on whether *Minstrel* is reachable; + * it has its own failure hysteresis. Distinguishes "device has a link + * but our server is down" from "no network at all." * * `version too old` is intentionally *not* folded in here -- it's a separate * UX (the VersionTooOldBanner) and conflating it with offline would mask the @@ -46,11 +52,17 @@ class ServerHealthController @Inject constructor( !serverReachable -> ServerHealth.ServerDown else -> ServerHealth.Healthy } - }.stateIn( - scope = scope, - started = SharingStarted.Eagerly, - initialValue = ServerHealth.Healthy, - ) + } + // Transition log -- the signal had no instrumentation, which is why a + // false-offline (WAN flicker flipping playback to "Source error") was + // hard to diagnose from logcat. WARN-tier so ReleaseTree surfaces it. + .distinctUntilChanged() + .onEach { Timber.w("ServerHealth -> %s", it) } + .stateIn( + scope = scope, + started = SharingStarted.Eagerly, + initialValue = ServerHealth.Healthy, + ) } /**