fix(android): don't flip offline on WAN-validation flicker — trust /healthz
android / Build + lint + test (push) Successful in 3m38s
android / Build + lint + test (push) Successful in 3m38s
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) <noreply@anthropic.com>
This commit is contained in:
+27
-15
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
+21
-9
@@ -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,
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user