diff --git a/app/src/main/java/com/pinakes/app/PinakesApplication.kt b/app/src/main/java/com/pinakes/app/PinakesApplication.kt index 51ce419..7dbcc24 100644 --- a/app/src/main/java/com/pinakes/app/PinakesApplication.kt +++ b/app/src/main/java/com/pinakes/app/PinakesApplication.kt @@ -13,7 +13,13 @@ import com.pinakes.app.data.network.NetworkEntryPoint import dagger.hilt.android.EntryPointAccessors import okhttp3.OkHttpClient import dagger.hilt.android.HiltAndroidApp +import io.sentry.Hint +import io.sentry.SentryEvent +import io.sentry.SentryOptions +import io.sentry.TypeCheckHint import io.sentry.android.core.SentryAndroid +import okhttp3.Request +import okhttp3.Response import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -43,6 +49,17 @@ class PinakesApplication : Application(), ImageLoaderFactory { options.isDebug = false options.isSendDefaultPii = false // no IP / user data attached by default options.tracesSampleRate = 0.0 // crash reporting only — no performance tracing + + // Sentry's OkHttp auto-instrumentation reports every backend HTTP error as an + // error-level SentryHttpClientException. This app talks to the user's OWN + // self-hosted server, so a backend outage is the server's state, not an app + // bug. Drop the two clearly-not-our-fault cases so they don't create noise: + // the health probe (whose whole job is to detect a down server) and transient + // upstream 5xx (502 bad gateway / 503 unavailable / 504 timeout). A real 500 + // or a 4xx (which can point at an app-side request bug) still comes through. + options.beforeSend = SentryOptions.BeforeSendCallback { event, hint -> + if (isExpectedBackendHttpFailure(event, hint)) null else event + } } // Refresh the cached catalog every time the app comes to the foreground, so the @@ -63,6 +80,28 @@ class PinakesApplication : Application(), ImageLoaderFactory { CatalogSyncWorker.schedule(this) } + /** + * True when a Sentry event is a backend HTTP failure that reflects the user's own + * server being unavailable rather than a bug in this app: a failed `/health` probe, + * or a transient upstream 5xx (502/503/504). Such events are pure noise for an app + * that points at self-hosted instances, so [onCreate]'s `beforeSend` drops them. + * + * Classification is driven by Sentry's OkHttp integration data (the request/response + * carried on the [Hint], and the structured `contexts.response.statusCode`), never by + * matching exception text — so a non-HTTP crash whose message happens to mention an + * HTTP status is never discarded. + */ + private fun isExpectedBackendHttpFailure(event: SentryEvent, hint: Hint): Boolean { + val response = hint.getAs(TypeCheckHint.OKHTTP_RESPONSE, Response::class.java) + val request = hint.getAs(TypeCheckHint.OKHTTP_REQUEST, Request::class.java) + // Only OkHttp-instrumented HTTP failures carry these signals. + val isHttpResponseFailure = + response != null || request != null || event.contexts.response?.statusCode != null + val statusCode = response?.code ?: event.contexts.response?.statusCode + val url = request?.url?.toString() ?: event.request?.url + return isExpectedBackendFailure(isHttpResponseFailure, statusCode, url) + } + /** * App-wide Coil loader with a persistent 256 MB disk cache that ignores server cache * headers, so book covers are downloaded once and reused across sessions instead of @@ -91,3 +130,25 @@ class PinakesApplication : Application(), ImageLoaderFactory { .build() } } + +/** + * Pure classification for the Sentry `beforeSend` filter, extracted so it can be unit + * tested without constructing SDK/OkHttp objects. + * + * @param isHttpResponseFailure whether the event is an OkHttp-instrumented HTTP-response + * failure at all (false for any non-HTTP crash → never dropped, whatever its message). + * @param statusCode the structured HTTP status, or null when unknown. + * @param url the request URL, or null when unknown. + * @return true only for a failure that reflects the user's server state rather than an + * app bug: a `/health` probe (any status), or a transient upstream 5xx (502/503/504). + */ +internal fun isExpectedBackendFailure( + isHttpResponseFailure: Boolean, + statusCode: Int?, + url: String?, +): Boolean { + if (!isHttpResponseFailure) return false + val path = (url ?: "").substringBefore('?').trimEnd('/') + if (path.endsWith("/health")) return true + return statusCode == 502 || statusCode == 503 || statusCode == 504 +} diff --git a/app/src/test/java/com/pinakes/app/SentryBackendFilterTest.kt b/app/src/test/java/com/pinakes/app/SentryBackendFilterTest.kt new file mode 100644 index 0000000..923003b --- /dev/null +++ b/app/src/test/java/com/pinakes/app/SentryBackendFilterTest.kt @@ -0,0 +1,52 @@ +package com.pinakes.app + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Matrix for [isExpectedBackendFailure], the pure core of the Sentry `beforeSend` + * filter that drops self-hosted-backend outage noise while keeping real signal. + */ +class SentryBackendFilterTest { + + private val apiUrl = "https://lib.example.org/api/v1/books" + private val healthUrl = "https://lib.example.org/api/v1/health" + + // ── Transient upstream 5xx on a normal endpoint → dropped ─────────────── + @Test fun drops502() = assertTrue(isExpectedBackendFailure(true, 502, apiUrl)) + @Test fun drops503() = assertTrue(isExpectedBackendFailure(true, 503, apiUrl)) + @Test fun drops504() = assertTrue(isExpectedBackendFailure(true, 504, apiUrl)) + + // ── Real server / client errors on a normal endpoint → kept ───────────── + @Test fun keeps500() = assertFalse(isExpectedBackendFailure(true, 500, apiUrl)) + @Test fun keeps400() = assertFalse(isExpectedBackendFailure(true, 400, apiUrl)) + @Test fun keeps404() = assertFalse(isExpectedBackendFailure(true, 404, apiUrl)) + @Test fun keeps401() = assertFalse(isExpectedBackendFailure(true, 401, apiUrl)) + + // ── Health probe failures are expected → dropped for any status ───────── + @Test fun dropsHealth503() = assertTrue(isExpectedBackendFailure(true, 503, healthUrl)) + @Test fun dropsHealth500() = assertTrue(isExpectedBackendFailure(true, 500, healthUrl)) + @Test fun dropsHealthWithQueryString() = + assertTrue(isExpectedBackendFailure(true, 500, "$healthUrl?ts=123")) + @Test fun dropsHealthWithTrailingSlash() = + assertTrue(isExpectedBackendFailure(true, 500, "$healthUrl/")) + @Test fun dropsHealthUnknownStatus() = + assertTrue(isExpectedBackendFailure(true, null, healthUrl)) + + // ── A non-HTTP crash is NEVER dropped, even with a 5xx-looking status ──── + @Test fun keepsNonHttpEvenWith503() = + assertFalse(isExpectedBackendFailure(false, 503, apiUrl)) + @Test fun keepsNonHttpOnHealthUrl() = + assertFalse(isExpectedBackendFailure(false, 503, healthUrl)) + + // ── Unknowns on a normal endpoint → kept ──────────────────────────────── + @Test fun keepsUnknownStatusOnApi() = + assertFalse(isExpectedBackendFailure(true, null, apiUrl)) + @Test fun keepsNullUrlWith500() = + assertFalse(isExpectedBackendFailure(true, 500, null)) + + // ── A transient 5xx with an unknown URL is still transient → dropped ──── + @Test fun dropsNullUrlWith503() = + assertTrue(isExpectedBackendFailure(true, 503, null)) +}