From c215bc7301997f659397213773d57e1a34e01a48 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Fri, 14 Aug 2026 10:52:54 +0200 Subject: [PATCH 1/2] fix(sentry): don't report self-hosted backend outages as app errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sentry's OkHttp auto-instrumentation captures every backend HTTP error as an error-level SentryHttpClientException. This app points at the user's OWN self-hosted server, so a backend outage is the server's state, not a bug here — and it generated noise like two "HTTP Client Error 503" events (one on the /health probe, one on a cover image) when a QNAP-hosted instance was briefly unavailable. Add a beforeSend that drops the two clearly-not-our-fault cases: a failed /health probe (whose whole job is to detect a down server) and transient upstream 5xx (502/503/504). A real 500 or a 4xx — which can point at an app-side request bug — still comes through. --- .../com/pinakes/app/PinakesApplication.kt | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/app/src/main/java/com/pinakes/app/PinakesApplication.kt b/app/src/main/java/com/pinakes/app/PinakesApplication.kt index 51ce419..8dba196 100644 --- a/app/src/main/java/com/pinakes/app/PinakesApplication.kt +++ b/app/src/main/java/com/pinakes/app/PinakesApplication.kt @@ -13,6 +13,8 @@ import com.pinakes.app.data.network.NetworkEntryPoint import dagger.hilt.android.EntryPointAccessors import okhttp3.OkHttpClient import dagger.hilt.android.HiltAndroidApp +import io.sentry.SentryEvent +import io.sentry.SentryOptions import io.sentry.android.core.SentryAndroid import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -43,6 +45,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, _ -> + if (isExpectedBackendHttpFailure(event)) null else event + } } // Refresh the cached catalog every time the app comes to the foreground, so the @@ -63,6 +76,30 @@ 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. + */ + private fun isExpectedBackendHttpFailure(event: SentryEvent): Boolean { + val detail = buildString { + event.throwable?.let { append(it.toString()) } + event.exceptions?.forEach { append(' ').append(it.type).append(' ').append(it.value) } + } + val isHttpClientError = detail.contains("SentryHttpClientException") || + detail.contains("HTTP Client Error with status code:") + if (!isHttpClientError) return false + + // The health probe's job is to detect a down server — a failure there is expected. + val path = event.request?.url?.substringBefore('?')?.trimEnd('/').orEmpty() + if (path.endsWith("/health")) return true + + // Transient upstream unavailability, not an app bug. + val status = Regex("""status code:\s*(\d{3})""").find(detail)?.groupValues?.get(1)?.toIntOrNull() + return status == 502 || status == 503 || status == 504 + } + /** * 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 From b45785c5b02a0d35449a84aeba30170d3fa74ebf Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Fri, 14 Aug 2026 11:12:01 +0200 Subject: [PATCH 2/2] refactor(sentry): classify HTTP failures from structured data, add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: - Classify OkHttp failures from the Sentry integration's structured signals — the Request/Response on the Hint (TypeCheckHint.OKHTTP_REQUEST/OKHTTP_RESPONSE) and contexts.response.statusCode — instead of matching exception text. A non-HTTP crash whose message happens to contain "HTTP Client Error with status code:" is no longer misclassified and dropped. - Extract the decision into a pure `isExpectedBackendFailure(...)` and cover it with a unit-test matrix: 502/503/504 dropped; 500 and 4xx kept; /health dropped for any status incl. query string and trailing slash; a non-HTTP failure kept even with a 503; unknown status/URL handled. testDebugUnitTest green. --- .../com/pinakes/app/PinakesApplication.kt | 60 +++++++++++++------ .../pinakes/app/SentryBackendFilterTest.kt | 52 ++++++++++++++++ 2 files changed, 94 insertions(+), 18 deletions(-) create mode 100644 app/src/test/java/com/pinakes/app/SentryBackendFilterTest.kt diff --git a/app/src/main/java/com/pinakes/app/PinakesApplication.kt b/app/src/main/java/com/pinakes/app/PinakesApplication.kt index 8dba196..7dbcc24 100644 --- a/app/src/main/java/com/pinakes/app/PinakesApplication.kt +++ b/app/src/main/java/com/pinakes/app/PinakesApplication.kt @@ -13,9 +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 @@ -53,8 +57,8 @@ class PinakesApplication : Application(), ImageLoaderFactory { // 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, _ -> - if (isExpectedBackendHttpFailure(event)) null else event + options.beforeSend = SentryOptions.BeforeSendCallback { event, hint -> + if (isExpectedBackendHttpFailure(event, hint)) null else event } } @@ -81,23 +85,21 @@ class PinakesApplication : Application(), ImageLoaderFactory { * 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): Boolean { - val detail = buildString { - event.throwable?.let { append(it.toString()) } - event.exceptions?.forEach { append(' ').append(it.type).append(' ').append(it.value) } - } - val isHttpClientError = detail.contains("SentryHttpClientException") || - detail.contains("HTTP Client Error with status code:") - if (!isHttpClientError) return false - - // The health probe's job is to detect a down server — a failure there is expected. - val path = event.request?.url?.substringBefore('?')?.trimEnd('/').orEmpty() - if (path.endsWith("/health")) return true - - // Transient upstream unavailability, not an app bug. - val status = Regex("""status code:\s*(\d{3})""").find(detail)?.groupValues?.get(1)?.toIntOrNull() - return status == 502 || status == 503 || status == 504 + 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) } /** @@ -128,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)) +}