Skip to content

fix(okhttp): Don't block on response bodies with no known length - #6231

Draft
markushi wants to merge 14 commits into
mainfrom
fix/okhttp-unknown-length-response-body
Draft

markushi wants to merge 14 commits into
mainfrom
fix/okhttp-unknown-length-response-body

Conversation

@markushi

@markushi markushi commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

📜 Description

SentryOkHttpInterceptor captured the response body with peekBody(MAX_NETWORK_BODY_SIZE + 1). OkHttp
implements a peek as request(n), which reads until n bytes are buffered or the stream ends — a body
with no Content-Length satisfies neither, so the interceptor never returned and the application never
received the response.

Every response body now goes through the new NetworkBodyCapturingResponseBody, which copies the bytes
as the application consumes them into a capped buffer and reports the capture once it can no longer
grow. A body of known length that nobody read is taken while it is closed, where the read is bounded,
so the peek is replaced rather than kept beside the new path. The status code and the headers are
recorded before anything is consumed, so a stream that stays open still produces a usable breadcrumb.

💡 Motivation and Context

With Session Replay network details enabled for a URL, every Server-Sent Events, long-poll or open
chunked endpoint hung forever or failed with a SocketTimeoutException. Gzipped responses were
affected too: OkHttp strips Content-Length when it decompresses, so an application interceptor sees
-1.

Continues carlonzo#1 by @carlonzo.

💚 How did you test it?

returns the response even though the body never ends is the regression test — a real chunked origin,
a TimeoutException without the fix. The suite also covers gzip, HTTP/2, enqueue(), a dropped
connection, a cancelled stream, the cap with its truncation warning, and the known-length behaviour
that must not change. :sentry-okhttp:test 139, :sentry:test 3689,
:sentry-android-replay:testReleaseUnitTest 265 — green, as are detekt and apiCheck.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

On the internal NetworkRequestData: setResponseDetails(int, ...) became
setResponseDetails(ResponseDetails); ResponseDetails and getResponseDetails() are new.

🔮 Next steps

  • A response that declares a large Content-Length and then trickles still blocks in the peek.
  • For a known-length body the body arrives later than before — when it is consumed or closed, not
    inside intercept. The status code and the headers still arrive immediately.

carlonzo and others added 14 commits October 6, 2026 13:32
Capturing a response body for session replay peeked it up front with
`Response.peekBody(MAX_NETWORK_BODY_SIZE + 1)`. OkHttp implements a peek as
`request(byteCount)`, which keeps reading until that many bytes are buffered or
the stream ends. A response with no Content-Length never satisfies either
condition, so for server-sent events, long-poll and any chunked endpoint that
stays open the interceptor never returned and the caller never received the
response at all.

Bodies with a known length still end on their own, so they keep the existing
up-front capture. Bodies of unknown length are now wrapped in a body that
copies what the application consumes into a capped buffer and reports it once
no more bytes can arrive: when the stream ends, when the application closes
the body, or when the cap is reached.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split the capture wrapping into its own function to keep a single return, and
suppress TooManyFunctions on the interceptor as done elsewhere in this module.
A response body has a single consumer, and neither Http1ExchangeCodec.cancel()
nor Http2ExchangeCodec.cancel() closes the body, so nothing reaches this class
from another thread. The AtomicBoolean already guarantees the capture is
reported exactly once.
A streamed body is only known once it has been consumed, which can be after the
NetworkRequestData carrying it was handed to the scope, so the replay thread can
read it while it is still being written. Keeping the three response values
behind one volatile reference means a reader sees either nothing or the
complete set.
@sentry

sentry Bot commented Oct 7, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.59.0 (1) release

⚙️ sentry-android Build Distribution Settings

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants