From 4c9450c117547d759a25c68b74582bdb7eabe1b9 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 5 Jun 2026 12:14:50 -0400 Subject: [PATCH] feat(android): feed API/playback/pull-refresh signals into NetworkStatusController OkHttp ReachabilityReportingInterceptor (Lazy to break the Hilt cycle) runs first in the chain and reports only PLACEHOLDER_HOST (Minstrel-bound) 2xx/IO outcomes so external artwork fetches don't read as server reachability. OfflineGatedDataSource reports stream open success/failure; PlaybackErrorReporter arbitrates on track failures; PullToRefreshScaffold re-probes on every pull. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../fabledsword/minstrel/api/NetworkModule.kt | 7 +++ .../ReachabilityReportingInterceptor.kt | 49 +++++++++++++++++++ .../minstrel/player/OfflineGatedDataSource.kt | 9 +++- .../minstrel/player/PlaybackErrorReporter.kt | 6 +++ .../widgets/PullRefreshNetworkViewModel.kt | 21 ++++++++ .../shared/widgets/PullToRefreshScaffold.kt | 7 ++- 6 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/connectivity/ReachabilityReportingInterceptor.kt create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/PullRefreshNetworkViewModel.kt diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/NetworkModule.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/NetworkModule.kt index 040b203b..91233f84 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/api/NetworkModule.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/NetworkModule.kt @@ -1,6 +1,7 @@ package com.fabledsword.minstrel.api import com.fabledsword.minstrel.BuildConfig +import com.fabledsword.minstrel.connectivity.ReachabilityReportingInterceptor import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory import dagger.Module import dagger.Provides @@ -45,9 +46,15 @@ object NetworkModule { fun provideOkHttp( baseUrl: BaseUrlInterceptor, auth: AuthCookieInterceptor, + reachability: ReachabilityReportingInterceptor, logging: HttpLoggingInterceptor, ): OkHttpClient = OkHttpClient.Builder() + // ReachabilityReportingInterceptor MUST run first: it identifies + // Minstrel-bound requests by the still-unrewritten PLACEHOLDER_HOST + // (so external artwork fetches don't read as server reachability) + // and observes the final transport outcome by wrapping the chain. + .addInterceptor(reachability) // AuthCookieInterceptor MUST run before BaseUrlInterceptor. // Both scope on `host == PLACEHOLDER_HOST` to distinguish // Minstrel-server requests from external image fetches diff --git a/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ReachabilityReportingInterceptor.kt b/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ReachabilityReportingInterceptor.kt new file mode 100644 index 00000000..c7d98c8b --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/connectivity/ReachabilityReportingInterceptor.kt @@ -0,0 +1,49 @@ +package com.fabledsword.minstrel.connectivity + +import com.fabledsword.minstrel.api.BaseUrlInterceptor.Companion.PLACEHOLDER_HOST +import dagger.Lazy +import okhttp3.Interceptor +import okhttp3.Response +import java.io.IOException +import javax.inject.Inject +import javax.inject.Singleton + +private const val HEALTHZ_PATH = "/healthz" + +/** + * Feeds real Minstrel API outcomes into [NetworkStatusController]. A 2xx is + * self-proving proof the server is reachable → reportSuccess(); a transport + * [IOException] (no response at all) → reportFailure(), which triggers /healthz + * arbitration. + * + * MUST run first in the OkHttp chain (before [BaseUrlInterceptor]) so the host + * is still the [PLACEHOLDER_HOST] sentinel: this shared client also fetches + * EXTERNAL artwork (musicbrainz / coverartarchive), and an external image + * loading must NOT be read as "our server is reachable" — only sentinel-host + * requests are Minstrel-bound. 5xx is deliberately NOT a failure (the server + * answered), and /healthz is skipped to avoid a feedback loop with the poll. + * + * [NetworkStatusController] is injected as a [Lazy] to break the Hilt cycle: + * the controller needs `Retrofit`, which needs `OkHttpClient`, which needs this + * interceptor. By the time a request flows through, the controller singleton is + * already constructed (construct-the-singleton trick in MinstrelApplication). + */ +@Singleton +class ReachabilityReportingInterceptor @Inject constructor( + private val networkStatus: Lazy, +) : Interceptor { + override fun intercept(chain: Interceptor.Chain): Response { + val request = chain.request() + val isMinstrel = request.url.host == PLACEHOLDER_HOST + val isHealthz = request.url.encodedPath.endsWith(HEALTHZ_PATH) + if (!isMinstrel || isHealthz) return chain.proceed(request) + return try { + val response = chain.proceed(request) + if (response.isSuccessful) networkStatus.get().reportSuccess() + response + } catch (e: IOException) { + networkStatus.get().reportFailure() + throw e + } + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/OfflineGatedDataSource.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/OfflineGatedDataSource.kt index 31b21971..542a915f 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/OfflineGatedDataSource.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/OfflineGatedDataSource.kt @@ -36,7 +36,14 @@ class OfflineGatedDataSource( // Unstable is non-gating: still try the network. Healthy too. ServerHealth.Unstable, ServerHealth.Healthy -> Unit } - return delegate.open(dataSpec) + return try { + val opened = delegate.open(dataSpec) + health.reportSuccess() // bytes flowing from the server == reachable + opened + } catch (e: IOException) { + health.reportFailure() // real network read failed → arbitrate via /healthz + throw e + } } override fun close() = delegate.close() diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt index f423cc5f..24e6458d 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt @@ -1,5 +1,6 @@ package com.fabledsword.minstrel.player +import com.fabledsword.minstrel.connectivity.NetworkStatusController import com.fabledsword.minstrel.di.ApplicationScope import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.channels.Channel @@ -40,6 +41,7 @@ private const val DEBOUNCE_MS = 2_000L class PlaybackErrorReporter @Inject constructor( private val playerController: PlayerController, private val repository: PlaybackErrorRepository, + private val networkStatus: NetworkStatusController, @ApplicationScope private val scope: CoroutineScope, ) { private val outChannel = Channel(Channel.BUFFERED) @@ -52,6 +54,10 @@ class PlaybackErrorReporter @Inject constructor( val buffer = mutableListOf() var debounceJob: kotlinx.coroutines.Job? = null playerController.playbackErrorEvents.collect { event -> + // A track failing to play is ambiguous (dead server vs. one bad + // file) — let the controller arbitrate via /healthz. No-op when + // already Offline; cheap otherwise. + networkStatus.reportFailure() // Fire-and-forget the server report — repository handles // success/queue branching so callers don't see throws. scope.launch { repository.report(event) } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/PullRefreshNetworkViewModel.kt b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/PullRefreshNetworkViewModel.kt new file mode 100644 index 00000000..af28b155 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/PullRefreshNetworkViewModel.kt @@ -0,0 +1,21 @@ +package com.fabledsword.minstrel.shared.widgets + +import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModel +import com.fabledsword.minstrel.connectivity.NetworkStatusController +import dagger.hilt.android.lifecycle.HiltViewModel +import javax.inject.Inject + +/** + * Routes a deliberate pull-to-refresh into a /healthz recheck so the connection + * banner clears within seconds rather than waiting for the next poll — even on + * cache-only screens whose own refresh never touches the network. Backs + * [PullToRefreshScaffold]. + */ +@HiltViewModel +class PullRefreshNetworkViewModel @Inject constructor( + private val networkStatus: NetworkStatusController, + @Suppress("UnusedPrivateProperty") savedStateHandle: SavedStateHandle, +) : ViewModel() { + fun recheck() = networkStatus.recheck() +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/PullToRefreshScaffold.kt b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/PullToRefreshScaffold.kt index 4a83d0ed..b3fdefa2 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/PullToRefreshScaffold.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/PullToRefreshScaffold.kt @@ -10,12 +10,15 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier +import androidx.hilt.navigation.compose.hiltViewModel import kotlinx.coroutines.launch /** * Wraps [content] in a Material3 PullToRefreshBox so the user can * swipe-down to trigger [onRefresh]. The wrapper manages the - * `isRefreshing` indicator while [onRefresh] is in flight. + * `isRefreshing` indicator while [onRefresh] is in flight. Every pull also + * fires a /healthz recheck (via [PullRefreshNetworkViewModel]) so a stale + * connection banner clears promptly on a deliberate user refresh. * * [onRefresh] is suspend: pass `{ viewModel.refresh().join() }` so the * indicator hides exactly when the underlying coroutine completes, @@ -32,6 +35,7 @@ import kotlinx.coroutines.launch fun PullToRefreshScaffold( onRefresh: suspend () -> Unit, modifier: Modifier = Modifier, + netVm: PullRefreshNetworkViewModel = hiltViewModel(), content: @Composable () -> Unit, ) { var isRefreshing by remember { mutableStateOf(false) } @@ -42,6 +46,7 @@ fun PullToRefreshScaffold( scope.launch { isRefreshing = true try { + netVm.recheck() // deliberate pull → re-probe the server now onRefresh() } finally { isRefreshing = false