sentry-okhttp reports spurious 504 errors for CacheControl.FORCE_CACHE requests

[angusholder]

Integration

sentry-android

Build System

Gradle

AGP Version

9.0.1

Proguard

Enabled

Other Error Monitoring Solution

No

Version

8.36.0

Steps to Reproduce

When using OkHttp's CacheControl.FORCE_CACHE, a cache miss returns a synthetic HTTP 504 (FORCE_CACHE's docs say "If the response isn't available in the cache or requires server validation, the call will fail with a 504 Unsatisfiable Request."). The Sentry OkHttp integration captures this as a real server error.

This failing test demonstrates the problem:

import io.sentry.Hint
import io.sentry.IScopes
import io.sentry.NoOpScopes
import io.sentry.SentryEvent
import io.sentry.SentryOptions
import io.sentry.okhttp.SentryOkHttpInterceptor
import io.sentry.protocol.SentryId
import okhttp3.CacheControl
import okhttp3.OkHttpClient
import okhttp3.Request
import kotlin.test.Test
import kotlin.test.assertEquals

class SentryOkHttpForceCacheBugTest {

    @Test
    fun `SentryOkHttpInterceptor reports spurious 504 for FORCE_CACHE cache miss`() {
        val capturedEvents = mutableListOf<SentryEvent>()

        val scopes: IScopes = object : IScopes by NoOpScopes.getInstance() {
            override fun getOptions(): SentryOptions = SentryOptions().apply {
                dsn = "https://example.invalid/"
            }
            override fun captureEvent(event: SentryEvent, hint: Hint?): SentryId {
                capturedEvents += event
                return SentryId()
            }
        }

        val client = OkHttpClient.Builder()
            .addInterceptor(SentryOkHttpInterceptor(scopes))
            .cache(null) // Cache is disabled for demonstration purposes, so we always get a cache miss
            .build()

        val resp = client.newCall(Request.Builder()
            .url("http://example.com")
            .cacheControl(CacheControl.FORCE_CACHE)
            .build()
        ).execute()

        assertEquals(504, resp.code)

        // This SHOULD be empty — the 504 is a synthetic cache-miss response from OkHttp,
        // not a real server error. But SentryOkHttpInterceptor reports it anyway.
        assertEquals(0, capturedEvents.size)
    }
}

Expected Result

These aren't genuine 504 Gateway Timeout responses from a server - Sentry should not report them as errors.

A possible fix would be to check request.cacheControl.onlyIfCached before capturing 504 responses in the OkHttp integration.

We're currently working around this with a custom EventProcessor that drops the event when onlyIfCached is true and the status is 504:

class OkHttpWorkaroundEventProcessor : EventProcessor {
    override fun process(event: SentryEvent, hint: Hint): SentryEvent? {
        val okhttpRequest = hint.getAs(TypeCheckHint.OKHTTP_REQUEST, Request::class.java)

        // Not a real 504, it means you asked the cache for something that's not present -
        // see the docs of CacheControl.FORCE_CACHE.
        if (
            okhttpRequest != null && // this is an okhttp request event
            okhttpRequest.cacheControl.onlyIfCached && // using CacheControl.FORCE_CACHE
            event.contexts.response?.statusCode == 504 // response was HTTP 504
        ) {
            return null // Suppress this event.
        }

        return event
    }
}

Actual Result

The 504 is reported to our Sentry dashboard as an error from our app

[linear-code]

JAVA-477