diff --git a/CHANGELOG.md b/CHANGELOG.md index 4126d1706dd..0f0620c9fa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Fix `SentryOkHttpInterceptor` hanging forever on responses whose body has no known length, such as Server-Sent Events or a gzipped response ([#6231](https://github.com/getsentry/sentry-java/pull/6231)) + ### Features - Report the cellular network technology generation in `device.connection_effective_type`, for example `4g` or `5g` ([#6146](https://github.com/getsentry/sentry-java/pull/6146)) diff --git a/sentry-android-integration-tests/test-app-size/proguard-rules.pro b/sentry-android-integration-tests/test-app-size/proguard-rules.pro index 4db0031ab00..e26a4ca5136 100644 --- a/sentry-android-integration-tests/test-app-size/proguard-rules.pro +++ b/sentry-android-integration-tests/test-app-size/proguard-rules.pro @@ -47,5 +47,7 @@ -dontwarn kotlin.math.MathKt -dontwarn okhttp3.EventListener -dontwarn okhttp3.Interceptor +-dontwarn okhttp3.ResponseBody +-dontwarn okio.ForwardingSource # Assume all classes are used to not strip them out, e.g. integrations like Compose or Sqlite -keep class io.sentry.** diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverter.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverter.kt index 5405b33eeac..7e6771f00eb 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverter.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverter.kt @@ -233,10 +233,14 @@ public open class DefaultReplayBreadcrumbConverter() : ReplayBreadcrumbConverter // Add Network Details data when available networkDetailData?.let { networkData -> + // One snapshot of the response: an integration that captures a streamed body may fill it in + // while this runs, and the status code must then belong to the body next to it. + val responseDetails = networkData.responseDetails + networkData.method?.let { breadcrumbData["method"] = it } - networkData.statusCode?.let { breadcrumbData["statusCode"] = it } + responseDetails?.let { breadcrumbData["statusCode"] = it.statusCode } networkData.requestBodySize?.let { breadcrumbData["requestBodySize"] = it } - networkData.responseBodySize?.let { breadcrumbData["responseBodySize"] = it } + responseDetails?.response?.size?.let { breadcrumbData["responseBodySize"] = it } networkData.request?.let { request -> val requestData = mutableMapOf() @@ -257,7 +261,7 @@ public open class DefaultReplayBreadcrumbConverter() : ReplayBreadcrumbConverter } } - networkData.response?.let { response -> + responseDetails?.response?.let { response -> val responseData = mutableMapOf() response.size?.let { responseData["size"] = it } response.body?.let { diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverterTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverterTest.kt index 749d3496698..ee5944283c1 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverterTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/DefaultReplayBreadcrumbConverterTest.kt @@ -429,12 +429,14 @@ class DefaultReplayBreadcrumbConverterTest { ) ) fakeOkHttpNetworkDetails.setResponseDetails( - 200, - ReplayNetworkRequestOrResponse( - 500L, - NetworkBody(mapOf("status" to "success", "message" to "OK")), - mapOf("Content-Type" to "text/plain"), - ), + NetworkRequestData.ResponseDetails( + 200, + ReplayNetworkRequestOrResponse( + 500L, + NetworkBody(mapOf("status" to "success", "message" to "OK")), + mapOf("Content-Type" to "text/plain"), + ), + ) ) val hintWithFakeOKHttpNetworkDetails = Hint() hintWithFakeOKHttpNetworkDetails.set(SENTRY_REPLAY_NETWORK_DETAILS, fakeOkHttpNetworkDetails) @@ -491,12 +493,14 @@ class DefaultReplayBreadcrumbConverterTest { ) ) fakeOkHttpNetworkDetails.setResponseDetails( - 404, - ReplayNetworkRequestOrResponse( - 550L, - NetworkBody(mapOf("status" to "success", "message" to "OK")), - mapOf("Content-Type" to "text/plain"), - ), + NetworkRequestData.ResponseDetails( + 404, + ReplayNetworkRequestOrResponse( + 550L, + NetworkBody(mapOf("status" to "success", "message" to "OK")), + mapOf("Content-Type" to "text/plain"), + ), + ) ) val hintWithFakeOKHttpNetworkDetails = Hint() hintWithFakeOKHttpNetworkDetails.set(SENTRY_REPLAY_NETWORK_DETAILS, fakeOkHttpNetworkDetails) @@ -546,12 +550,14 @@ class DefaultReplayBreadcrumbConverterTest { ) ) networkRequestData.setResponseDetails( - 200, - ReplayNetworkRequestOrResponse( - 100L, - NetworkBody("response body content"), - mapOf("Content-Type" to "application/json"), - ), + NetworkRequestData.ResponseDetails( + 200, + ReplayNetworkRequestOrResponse( + 100L, + NetworkBody("response body content"), + mapOf("Content-Type" to "application/json"), + ), + ) ) hint.set(SENTRY_REPLAY_NETWORK_DETAILS, networkRequestData) diff --git a/sentry-okhttp/src/main/java/io/sentry/okhttp/CapturedResponseBody.kt b/sentry-okhttp/src/main/java/io/sentry/okhttp/CapturedResponseBody.kt new file mode 100644 index 00000000000..70c282bd243 --- /dev/null +++ b/sentry-okhttp/src/main/java/io/sentry/okhttp/CapturedResponseBody.kt @@ -0,0 +1,114 @@ +package io.sentry.okhttp + +import java.io.IOException +import java.util.concurrent.atomic.AtomicBoolean +import okhttp3.MediaType +import okhttp3.ResponseBody +import okio.Buffer +import okio.BufferedSource +import okio.ForwardingSource +import okio.Source +import okio.buffer + +/** + * A [ResponseBody] that copies the bytes the application reads into a capped buffer. + * + * Reading the body up front instead, with [okhttp3.Response.peekBody], deadlocks on a response of + * unknown length: a peek is `request(byteCount)`, which waits for that many bytes or for the end of + * the stream, and a server-sent-events stream, a long-poll or an open chunked endpoint delivers + * neither. The caller would never receive the response at all. + * + * Nothing is therefore read on the capture's own account. Only what the application reads is + * captured, which also keeps the cost of the instrumentation to a copy of those bytes. + * + * @param delegate the body to capture from. + * @param maxBytes the maximum number of bytes to retain; capture stops once it is reached. + * @param onCaptured invoked at most once, synchronously, on the thread that finishes the body — so + * it must not block. A body that is abandoned without being read to the end or closed never + * reaches it. + */ +internal class CapturedResponseBody( + private val delegate: ResponseBody, + private val maxBytes: Long, + private val onCaptured: (ByteArray) -> Unit, +) : ResponseBody() { + + private val captured = Buffer() + private val reported = AtomicBoolean(false) + + // Buffering the capturing source cannot re-introduce the deadlock above: a BufferedSource read + // takes at most one segment from the source below it and returns with whatever arrived, so a + // short event is still forwarded on its own. Only request/require/readByteArray() wait for a byte + // count, and those are the application's own calls. + private val capturingSource: BufferedSource by lazy { + CapturingSource(delegate.source()).buffer() + } + + override fun contentType(): MediaType? = delegate.contentType() + + override fun contentLength(): Long = delegate.contentLength() + + override fun source(): BufferedSource = capturingSource + + override fun close() { + try { + // Closing the delegate notifies listeners, which may serialize the breadcrumb this capture + // belongs to, so report first. + reportCaptured() + } finally { + delegate.close() + } + } + + private inner class CapturingSource(source: Source) : ForwardingSource(source) { + override fun read(sink: Buffer, byteCount: Long): Long { + val sinkBefore = sink.size + val read = + try { + super.read(sink, byteCount) + } catch (e: IOException) { + // What arrived is the evidence for this very failure, and a caller handling it is not + // obliged to close the body. + reportCaptured() + throw e + } + + if (read > 0L) { + val capFull = + synchronized(captured) { + // the application may not have taken everything in the sink yet, hence the offset + val toTake = minOf(maxBytes - captured.size, sink.size - sinkBefore) + if (toTake > 0L) { + sink.copyTo(captured, sinkBefore, toTake) + } + captured.size >= maxBytes + } + if (capFull) { + reportCaptured() + } + } else if (read == -1L) { + reportCaptured() + } + return read + } + + override fun close() { + try { + reportCaptured() + } finally { + super.close() + } + } + } + + /** + * [captured] is written by the thread reading the body and read here, which [close] may reach + * from another thread — cancelling a stream from elsewhere is ordinary use — and [Buffer] is not + * thread-safe. + */ + private fun reportCaptured() { + if (reported.compareAndSet(false, true)) { + onCaptured(synchronized(captured) { captured.clone().readByteArray() }) + } + } +} diff --git a/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt b/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt index 71a43590e52..820dc8b044e 100644 --- a/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt @@ -50,6 +50,7 @@ import org.jetbrains.annotations.VisibleForTesting * @param failedRequestTargets The SDK will only capture HTTP Client errors if the HTTP Request URL * is a match for any of the defined targets. */ +@Suppress("TooManyFunctions") // one function per concern of the interception pipeline public open class SentryOkHttpInterceptor( private val scopes: IScopes = ScopesAdapter.getInstance(), private val beforeSpan: BeforeSpanCallback? = null, @@ -171,27 +172,32 @@ public open class SentryOkHttpInterceptor( ) request = requestBuilder.build() - response = chain.proceed(request) - code = response.code + val rawResponse = chain.proceed(request) + response = rawResponse + code = rawResponse.code span?.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, code) span?.status = SpanStatus.fromHttpStatusCode(code) // OkHttp errors (4xx, 5xx) don't throw, so it's safe to call within this block. // breadcrumbs are added on the finally block because we'd like to know if the device // had an unstable connection or something similar - if (shouldCaptureClientError(request, response)) { + if (shouldCaptureClientError(request, rawResponse)) { // If we capture the client error directly, it could be associated with the // currently running span by the backend. In case the listener is in use, that is // an inner span. So, if the listener is in use, we let it capture the client // error, to shown it in the http root call span in the dashboard. if (isFromEventListener && okHttpEvent != null) { - okHttpEvent.setClientErrorResponse(response) + okHttpEvent.setClientErrorResponse(rawResponse) } else { - SentryOkHttpUtils.captureClientError(scopes, request, response) + SentryOkHttpUtils.captureClientError(scopes, request, rawResponse) } } - return response + // Wrapping comes last, after the client error was captured from the untouched response. The + // returned value is computed here, so the finally block below only reads it. + val capturingResponse = rawResponse.withNetworkBodyCapture(networkDetailData) + response = capturingResponse + return capturingResponse } catch (e: IOException) { span?.apply { this.throwable = e @@ -203,20 +209,6 @@ public open class SentryOkHttpInterceptor( // this only works correctly if SentryOkHttpInterceptor is the last one in the chain okHttpEvent?.setRequest(request) - response?.let { - networkDetailData?.setResponseDetails( - it.code, - NetworkDetailCaptureUtils.createResponse( - it, - it.body?.contentLength(), - scopes.options.sessionReplay.isNetworkCaptureBodies, - { resp: Response -> resp.extractResponseBody(scopes.options.logger) }, - scopes.options.sessionReplay.networkResponseHeaders, - { resp: Response -> resp.headers.toMap() }, - ), - ) - } - // Set network details on the OkHttpEvent so it can include them in the breadcrumb hint okHttpEvent?.setNetworkDetails(networkDetailData) @@ -322,36 +314,90 @@ public open class SentryOkHttpInterceptor( } } - /** Extracts the body content from an OkHttp Response safely */ - private fun Response.extractResponseBody(logger: ILogger): NetworkBody? { - return body?.let { responseBody -> - try { - val contentType = responseBody.contentType() - val contentTypeString = contentType?.toString() - val maxBodySize = SentryReplayOptions.MAX_NETWORK_BODY_SIZE - - // Peek at the body (doesn't consume it) - // We +1 here in order to properly truncate within NetworkBodyParser.fromBytes - // and be able to distinguish from an oversized request and a request matching maxBodySize - val peekBody = peekBody(maxBodySize.toLong() + 1) - val bodyBytes = peekBody.bytes() - - val charset = contentType?.charset(Charsets.UTF_8)?.name() ?: "UTF-8" - return NetworkBodyParser.fromBytes( - bodyBytes, - contentTypeString, - charset, - maxBodySize, - logger, - ) - } catch (e: Exception) { - logger.log( - io.sentry.SentryLevel.ERROR, - "Failed to read http response body for Network Details: ${e.message}", - ) - null - } + /** + * Returns this response with a body whose content is captured for replay while the application + * reads it. + * + * Reading the body up front, as [Response.peekBody] does, deadlocks on a response of unknown + * length: server-sent events, a long-poll, a chunked endpoint that stays open, and every response + * OkHttp decompressed itself. The caller would never receive the response at all. + * + * Every body therefore goes through [CapturedResponseBody], which captures only what the + * application reads. A body nobody reads leaves the status code and the headers recorded below, + * and no body. + */ + private fun Response.withNetworkBodyCapture(networkDetailData: NetworkRequestData?): Response { + val responseBody = body + if (networkDetailData == null || responseBody == null) { + return this } + + val knownBodySize = responseBody.contentLength().takeIf { it >= 0L } + + // The status code and the headers are known now, so record them before anything is consumed. A + // stream that stays open for minutes never gets further than this. + recordResponse(networkDetailData, knownBodySize) { null } + + if (!scopes.options.sessionReplay.isNetworkCaptureBodies) { + return this + } + + val logger = scopes.options.logger + // One byte above the limit, so NetworkBodyParser.fromBytes can tell a truncated body from one + // that happens to match the limit exactly. + val captureCap = SentryReplayOptions.MAX_NETWORK_BODY_SIZE.toLong() + 1 + val contentType = responseBody.contentType() + val contentTypeString = contentType?.toString() + val charset = contentType?.charset(Charsets.UTF_8)?.name() ?: "UTF-8" + + return newBuilder() + .body( + CapturedResponseBody(responseBody, captureCap) { capturedBytes -> + // This runs on the thread consuming the body, inside its read or close, so a failure in + // here must stay in here. + try { + recordResponse(networkDetailData, knownBodySize ?: capturedBytes.size.toLong()) { + NetworkBodyParser.fromBytes( + capturedBytes, + contentTypeString, + charset, + SentryReplayOptions.MAX_NETWORK_BODY_SIZE, + logger, + ) + } + } catch (e: Exception) { + logger.log( + io.sentry.SentryLevel.ERROR, + "Failed to capture the http response body for Network Details: ${e.message}", + ) + } + } + ) + .build() + } + + /** + * Records what is known about this response, replacing anything recorded for it before. Called + * once before the body is consumed and once with the captured body. + */ + private fun Response.recordResponse( + networkDetailData: NetworkRequestData, + bodySize: Long?, + body: () -> NetworkBody?, + ) { + networkDetailData.setResponseDetails( + NetworkRequestData.ResponseDetails( + code, + NetworkDetailCaptureUtils.createResponse( + this, + bodySize, + true, + { body() }, + scopes.options.sessionReplay.networkResponseHeaders, + { resp: Response -> resp.headers.toMap() }, + ), + ) + ) } private fun finishSpan( diff --git a/sentry-okhttp/src/test/java/io/sentry/okhttp/CapturedResponseBodyTest.kt b/sentry-okhttp/src/test/java/io/sentry/okhttp/CapturedResponseBodyTest.kt new file mode 100644 index 00000000000..9b740f1789f --- /dev/null +++ b/sentry-okhttp/src/test/java/io/sentry/okhttp/CapturedResponseBodyTest.kt @@ -0,0 +1,341 @@ +package io.sentry.okhttp + +import java.io.IOException +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import okhttp3.MediaType +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.ResponseBody +import okio.Buffer +import okio.BufferedSource +import okio.Source +import okio.Timeout +import okio.buffer + +class CapturedResponseBodyTest { + + /** Emits one chunk per read, then EOF. Records how many reads happened. */ + private class ChunkedSource(private val chunks: List) : Source { + var reads = 0 + private set + + private var index = 0 + private var closed = false + + override fun read(sink: Buffer, byteCount: Long): Long { + reads++ + if (index >= chunks.size) return -1L + val chunk = chunks[index++] + sink.write(chunk) + return chunk.size.toLong() + } + + override fun timeout(): Timeout = Timeout.NONE + + override fun close() { + closed = true + } + + val isClosed: Boolean + get() = closed + } + + private class FailingSource(private val bytesBeforeFailure: Int) : Source { + var isClosed = false + private set + + private var written = 0 + + override fun read(sink: Buffer, byteCount: Long): Long { + if (written >= bytesBeforeFailure) throw IOException("connection reset") + sink.write(ByteArray(bytesBeforeFailure) { 'x'.code.toByte() }) + written += bytesBeforeFailure + return bytesBeforeFailure.toLong() + } + + override fun timeout(): Timeout = Timeout.NONE + + override fun close() { + isClosed = true + } + } + + /** Emits one chunk, then parks until it is closed, like a stream that has gone quiet. */ + private class ParkingSource(private val chunk: ByteArray) : Source { + val emitted = CountDownLatch(1) + private val closed = CountDownLatch(1) + private var sent = false + + override fun read(sink: Buffer, byteCount: Long): Long { + if (!sent) { + sent = true + sink.write(chunk) + emitted.countDown() + return chunk.size.toLong() + } + closed.await(10, TimeUnit.SECONDS) + return -1L + } + + override fun timeout(): Timeout = Timeout.NONE + + override fun close() { + closed.countDown() + } + } + + private fun bodyOf( + source: Source, + type: String? = "text/plain", + length: Long = -1L, + ): ResponseBody = + object : ResponseBody() { + override fun contentType(): MediaType? = type?.toMediaType() + + override fun contentLength(): Long = length + + override fun source(): BufferedSource = source.buffer() + } + + private fun capture( + source: Source, + maxBytes: Long, + type: String? = "text/plain", + length: Long = -1L, + ): Pair> { + val captured = mutableListOf() + val wrapper = + CapturedResponseBody(bodyOf(source, type, length), maxBytes) { + captured.add(it) + } + return wrapper to captured + } + + @Test + fun `does not read anything before the application does`() { + val source = ChunkedSource(listOf("hello".toByteArray())) + val (wrapper, captured) = capture(source, 1024) + + assertEquals(0, source.reads, "the wrapper must not read the body up front") + assertTrue(captured.isEmpty(), "nothing can be captured before the application reads") + assertEquals("text/plain", wrapper.contentType()?.toString(), "content type is delegated") + } + + @Test + fun `forwards every byte to the application unchanged`() { + val payload = "data: event-0\n\ndata: event-1\n\n".toByteArray() + val (wrapper, _) = capture(ChunkedSource(listOf(payload)), 1024) + + assertContentEquals(payload, wrapper.source().readByteArray()) + } + + @Test + fun `captures the whole body when it is consumed and ends`() { + val (wrapper, captured) = + capture( + ChunkedSource(listOf("hello ".toByteArray(), "world".toByteArray())), + 1024, + ) + + wrapper.source().readByteArray() + + assertEquals(1, captured.size, "the capture must be reported exactly once") + assertEquals("hello world", captured.single()?.decodeToString()) + } + + @Test + fun `passes small events through as they arrive and captures them on close`() { + val events = (0 until 3).map { "data: event-$it\n\n".toByteArray() } + val source = ChunkedSource(events) + val (wrapper, captured) = capture(source, 1024) + + val application = wrapper.source() + for (expected in events) { + assertContentEquals(expected, application.readByteArray(expected.size.toLong())) + } + + assertTrue(captured.isEmpty(), "an open stream has nothing final to report yet") + + wrapper.close() + + assertEquals(1, captured.size) + assertEquals( + "data: event-0\n\ndata: event-1\n\ndata: event-2\n\n", + captured.single()?.decodeToString(), + ) + } + + @Test + fun `captures what was read when the body is closed early`() { + val (wrapper, captured) = + capture(ChunkedSource(listOf("first ".toByteArray(), "second".toByteArray())), 1024) + + val application = wrapper.source() + assertEquals("first ", application.readUtf8(6)) + + wrapper.close() + + assertEquals(1, captured.size) + assertEquals("first ", captured.single()?.decodeToString()) + } + + @Test + fun `does not read a body the application never read`() { + val source = ChunkedSource(listOf("failure".toByteArray())) + val (wrapper, captured) = capture(source, 1024, length = 7L) + + wrapper.close() + + assertEquals(0, source.reads, "the capture must not read on its own account") + assertTrue(captured.single()!!.isEmpty()) + } + + @Test + fun `reports an empty capture for an empty body`() { + val (wrapper, captured) = capture(ChunkedSource(emptyList()), 1024) + + wrapper.source().readByteArray() + wrapper.close() + + assertEquals(1, captured.size) + assertEquals(0, captured.single()?.size) + } + + @Test + fun `never captures more than the cap but still delivers the whole body`() { + val payload = "0123456789abcdefghij".toByteArray() + val (wrapper, captured) = capture(ChunkedSource(listOf(payload)), 8) + + assertContentEquals( + payload, + wrapper.source().readByteArray(), + "the application is not truncated", + ) + + assertEquals(1, captured.size, "reaching the cap is final, so it is reported once") + assertEquals("01234567", captured.single()?.decodeToString()) + } + + @Test + fun `captures every byte when the application reads less than a chunk at a time`() { + // The capture copies out of the application's sink at an offset, because a BufferedSource keeps + // what the application did not take yet in that same sink. + val chunks = (0 until 5).map { i -> "chunk-$i--".toByteArray() } + val whole = chunks.joinToString("") { it.decodeToString() } + val (wrapper, captured) = capture(ChunkedSource(chunks), 1024) + + val source = wrapper.source() + val read = StringBuilder() + while (!source.exhausted()) { + read.append(source.readUtf8(minOf(3L, source.buffer.size.coerceAtLeast(1L)))) + } + + assertEquals(whole, read.toString(), "the application receives the whole body") + assertEquals(whole, captured.single()?.decodeToString(), "and the capture holds the same bytes") + } + + @Test + fun `a failing capture callback does not reach the application`() { + val source = ChunkedSource(listOf("payload".toByteArray())) + val body = bodyOf(source) + val wrapper = CapturedResponseBody(body, 1024) { throw IllegalStateException("parse failed") } + + // The interceptor guards its own callback; the wrapper must not swallow the failure silently + // while leaving the delegate open. + assertFailsWith { wrapper.close() } + assertTrue(source.isClosed, "the delegate is closed even though the callback threw") + } + + @Test + fun `reports once when the body is closed while another thread is reading it`() { + // Cancelling a stream from elsewhere closes the body from a thread other than the consumer, so + // the capture is read and written at the same time. + val chunk = "data: event-0\n\n".toByteArray() + val source = ParkingSource(chunk) + val (wrapper, captured) = capture(source, 1024) + val readDone = CountDownLatch(1) + val reader = Thread { + wrapper.source().readByteArray() + readDone.countDown() + } + reader.isDaemon = true + reader.start() + + assertTrue(source.emitted.await(10, TimeUnit.SECONDS), "the reader must have taken the chunk") + wrapper.close() + + assertTrue( + readDone.await(10, TimeUnit.SECONDS), + "the reader must finish once the body is closed", + ) + assertEquals( + 1, + captured.size, + "the capture is reported once, by whichever thread got there first", + ) + assertContentEquals(chunk, captured.single()) + } + + @Test + fun `reports only once when the body ends and is then closed`() { + val (wrapper, captured) = capture(ChunkedSource(listOf("done".toByteArray())), 1024) + + wrapper.source().readByteArray() + wrapper.close() + wrapper.source().close() + + assertEquals(1, captured.size, "the capture is reported exactly once") + } + + @Test + fun `delegates content type and content length`() { + val type = "text/event-stream".toMediaType() + val delegate = + object : ResponseBody() { + override fun contentType(): MediaType? = type + + override fun contentLength(): Long = -1L + + override fun source(): BufferedSource = ChunkedSource(emptyList()).buffer() + } + + val wrapper = CapturedResponseBody(delegate, 16) {} + + assertEquals(type, wrapper.contentType()) + assertEquals(-1L, wrapper.contentLength()) + } + + @Test + fun `propagates read failures to the application`() { + val source = FailingSource(bytesBeforeFailure = 4) + val captured = mutableListOf() + val wrapper = CapturedResponseBody(bodyOf(source), 1024) { captured.add(it) } + + assertFailsWith { wrapper.source().readByteArray() } + assertEquals( + "xxxx", + captured.single()?.decodeToString(), + "what arrived before the failure is reported, since nothing more can arrive", + ) + + wrapper.close() + assertEquals(1, captured.size, "closing afterwards does not report a second time") + assertTrue(source.isClosed, "the connection must still be releasable after a failure") + } + + @Test + fun `closes the delegate body`() { + val source = ChunkedSource(listOf("x".toByteArray())) + val (wrapper, _) = capture(source, 1024) + + assertFalse(source.isClosed) + wrapper.close() + assertTrue(source.isClosed, "the application must still be able to release the connection") + } +} diff --git a/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorUnknownContentLengthTest.kt b/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorUnknownContentLengthTest.kt new file mode 100644 index 00000000000..fb21bebb626 --- /dev/null +++ b/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorUnknownContentLengthTest.kt @@ -0,0 +1,536 @@ +package io.sentry.okhttp + +import io.sentry.Hint +import io.sentry.IScopes +import io.sentry.SentryOptions +import io.sentry.TypeCheckHint +import io.sentry.util.network.NetworkBody +import io.sentry.util.network.NetworkRequestData +import java.io.ByteArrayOutputStream +import java.io.Closeable +import java.io.IOException +import java.net.InetAddress +import java.net.ServerSocket +import java.net.Socket +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue +import okhttp3.Call +import okhttp3.Callback +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okio.Buffer +import okio.GzipSink +import okio.buffer +import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +/** + * Response bodies of unknown length: server-sent events, long-poll, chunked endpoints that stay + * open. + * + * These used to hang. Capturing the body means peeking it, and OkHttp implements a peek as "keep + * reading until the peek size is reached or the stream ends". For these responses neither happens, + * so the thread that called `execute()` never received the response at all. + */ +class SentryOkHttpInterceptorUnknownContentLengthTest { + + private val scopes = mock() + private lateinit var options: SentryOptions + private lateinit var sut: OkHttpClient + + private fun setUpSut(captureBodies: Boolean = true) { + options = + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + sessionReplay.setNetworkDetailAllowUrls(listOf(".*")) + sessionReplay.setNetworkCaptureBodies(captureBodies) + } + whenever(scopes.options).thenReturn(options) + sut = OkHttpClient.Builder().addInterceptor(SentryOkHttpInterceptor(scopes)).build() + } + + private fun requestTo(url: String) = Request.Builder().url(url).build() + + /** The network details the breadcrumb points at; read after the body has been consumed. */ + private fun networkDetails(): NetworkRequestData { + val hint = argumentCaptor() + verify(scopes).addBreadcrumb(any(), hint.capture()) + return assertNotNull( + hint.firstValue.getAs( + TypeCheckHint.SENTRY_REPLAY_NETWORK_DETAILS, + NetworkRequestData::class.java, + ) + ) + } + + private fun capturedBody(): String? = networkDetails().response?.body?.body as String? + + private fun capturedBodyValue(): Any? = networkDetails().response?.body?.body + + // --------------------------------------------------------------------------------------- + // the regression + // --------------------------------------------------------------------------------------- + + @Test + fun `returns the response even though the body never ends`() { + setUpSut() + EventStreamServer(listOf("data: event-0\n\n"), closeStream = false).use { stream -> + val executor = Executors.newSingleThreadExecutor { r -> Thread(r).apply { isDaemon = true } } + val call: Call = sut.newCall(requestTo(stream.url)) + try { + val response = executor.submit { call.execute() }.get(5, TimeUnit.SECONDS) + assertEquals(200, response.code) + + // The stream is still open and nothing has been consumed, so the capture cannot have + // happened yet. Everything that is already known must still reach the breadcrumb. + val details = networkDetails() + assertEquals(200, details.statusCode) + assertNotNull(details.response, "the response is recorded before the body is consumed") + assertNull(details.response?.body, "but its body is not known yet") + + response.close() + } finally { + executor.shutdownNow() + } + } + } + + @Test + fun `delivers each small event to the application as it arrives`() { + setUpSut() + val events = (0 until 3).map { "data: event-$it\n\n" } + EventStreamServer(events, closeStream = true).use { stream -> + sut.newCall(requestTo(stream.url)).execute().use { response -> + val source = assertNotNull(response.body).source() + for (event in events) { + assertEquals( + event, + source.readUtf8(event.length.toLong()), + "the application must see every event", + ) + } + assertTrue(source.exhausted(), "the stream ended cleanly") + } + } + } + + // --------------------------------------------------------------------------------------- + // what ends up captured + // --------------------------------------------------------------------------------------- + + @Test + fun `captures a body of unknown length once the stream ends`() { + setUpSut() + val events = listOf("data: event-0\n\n", "data: event-1\n\n") + EventStreamServer(events, closeStream = true).use { stream -> + sut.newCall(requestTo(stream.url)).execute().use { it.body?.string() } + val details = networkDetails() + assertEquals(200, details.statusCode) + assertEquals(events.joinToString(""), capturedBody()) + assertEquals(events.joinToString("").length.toLong(), details.responseBodySize) + } + } + + @Test + fun `captures a body of unknown length when the application closes it before the stream ends`() { + setUpSut() + EventStreamServer(listOf("data: event-0\n\n", "data: event-1\n\n"), closeStream = false).use { + stream -> + sut.newCall(requestTo(stream.url)).execute().use { response -> + assertEquals("data: event-0\n\n", assertNotNull(response.body).source().readUtf8(15)) + } + + assertEquals("data: event-0\n\n", capturedBody()) + } + } + + @Test + fun `records the response of a body of unknown length the application never reads`() { + setUpSut() + EventStreamServer(listOf("data: event-0\n\n"), closeStream = true).use { stream -> + sut.newCall(requestTo(stream.url)).execute().close() + + val details = networkDetails() + assertEquals(200, details.statusCode) + assertNull( + capturedBody(), + "an unread stream is never peeked, so there is nothing to report; the response is still recorded", + ) + } + } + + @Test + fun `captures an error response body of unknown length`() { + setUpSut() + EventStreamServer(listOf("data: boom\n\n"), closeStream = true, statusCode = 500).use { stream + -> + sut.newCall(requestTo(stream.url)).execute().use { it.body?.string() } + + val details = networkDetails() + assertEquals(500, details.statusCode) + assertEquals("data: boom\n\n", capturedBody()) + } + } + + @Test + fun `caps the captured body without truncating the body the application reads`() { + setUpSut() + val events = (0 until 200).map { "data: " + "x".repeat(1000) + "\n\n" } + val whole = events.joinToString("") + EventStreamServer(events, closeStream = true).use { stream -> + val received = sut.newCall(requestTo(stream.url)).execute().use { it.body?.string() } + + assertEquals(whole.length, received?.length, "the application must receive the whole stream") + val captured = assertNotNull(capturedBody()) + assertEquals( + io.sentry.SentryReplayOptions.MAX_NETWORK_BODY_SIZE, + captured.length, + "the capture must stop at the cap", + ) + assertEquals(whole.substring(0, captured.length), captured) + assertEquals( + listOf(NetworkBody.NetworkBodyWarning.TEXT_TRUNCATED), + networkDetails().response?.body?.warnings, + "a capped capture must be reported as truncated, not as a complete body", + ) + } + } + + @Test + fun `does not capture bodies when network body capture is turned off`() { + setUpSut(captureBodies = false) + EventStreamServer(listOf("data: event-0\n\n"), closeStream = true).use { stream -> + sut.newCall(requestTo(stream.url)).execute().use { it.body?.string() } + + assertEquals(200, networkDetails().statusCode) + assertNull(capturedBody()) + } + } + + // --------------------------------------------------------------------------------------- + // how the body is actually consumed + // --------------------------------------------------------------------------------------- + + @Test + fun `captures a body consumed from the callback of an asynchronous call`() { + // enqueue() hands the response to a dispatcher thread, so the capture is completed by a thread + // other than the one that started the call. That is the publication NetworkRequestData guards. + setUpSut() + val events = (0 until 3).map { "data: event-$it\n\n" } + EventStreamServer(events, closeStream = true).use { stream -> + val body = AtomicReference() + val thread = AtomicReference() + val done = CountDownLatch(1) + + sut + .newCall(requestTo(stream.url)) + .enqueue( + object : Callback { + override fun onFailure(call: Call, e: IOException) = done.countDown() + + override fun onResponse(call: Call, response: Response) { + response.use { body.set(it.body?.string()) } + thread.set(Thread.currentThread().name) + done.countDown() + } + } + ) + + assertTrue(done.await(10, TimeUnit.SECONDS), "the callback must be reached") + assertEquals(events.joinToString(""), body.get()) + assertNotEquals( + Thread.currentThread().name, + thread.get(), + "the body is consumed off the calling thread", + ) + assertEquals(events.joinToString(""), capturedBody()) + assertEquals(200, networkDetails().statusCode) + } + } + + @Test + fun `captures what arrived when a stream is cancelled while a reader is parked on it`() { + // How an SSE client consumes a stream: line by line on its own thread, until the call is + // cancelled. Nothing closes the body afterwards, so the broken read is what completes the + // capture, with the events that did arrive. + setUpSut() + val events = (0 until 3).map { "data: event-$it\n\n" } + EventStreamServer(events, closeStream = false).use { stream -> + val call = sut.newCall(requestTo(stream.url)) + val response = call.execute() + val readThree = CountDownLatch(3) + val failed = CountDownLatch(1) + val reader = Thread { + @Suppress("SwallowedException") // cancelling the call is how this read is meant to end + try { + val source = response.body!!.source() + while (true) { + val line = source.readUtf8Line() ?: break + if (line.startsWith("data:")) readThree.countDown() + } + } catch (e: IOException) { + failed.countDown() + } + } + reader.isDaemon = true + reader.start() + + assertTrue(readThree.await(10, TimeUnit.SECONDS), "the reader must receive the events") + call.cancel() + + assertTrue(failed.await(10, TimeUnit.SECONDS), "the parked read must end with an IOException") + assertEquals(200, networkDetails().statusCode) + assertEquals(events.joinToString(""), capturedBody()) + } + } + + @Test + fun `captures what arrived when the server drops the connection mid stream`() { + setUpSut() + val events = (0 until 3).map { "data: event-$it\n\n" } + EventStreamServer(events, closeStream = false, dropAfterEvents = true).use { stream -> + val response = sut.newCall(requestTo(stream.url)).execute() + + assertFailsWith { response.body?.string() } + + assertEquals(200, networkDetails().statusCode) + assertEquals(events.joinToString(""), capturedBody()) + } + } + + @Test + fun `captures a body of unknown length over HTTP2`() { + // HTTP/2 has no chunked encoding and ends a body with an empty DATA frame, so it reaches the + // wrapper by a different route than the HTTP/1.1 tests above. + setUpSut() + val payload = "data: over-h2\n\n" + MockWebServer().use { server -> + server.protocols = listOf(Protocol.H2_PRIOR_KNOWLEDGE) + server.enqueue( + MockResponse() + .setBody(payload) + .removeHeader("Content-Length") + .setHeader("Content-Type", "text/event-stream") + ) + sut = sut.newBuilder().protocols(listOf(Protocol.H2_PRIOR_KNOWLEDGE)).build() + + val response = sut.newCall(requestTo(server.url("/events").toString())).execute() + assertEquals(payload, response.use { it.body?.string() }) + + assertEquals(Protocol.H2_PRIOR_KNOWLEDGE, response.protocol) + assertEquals(-1L, response.body?.contentLength(), "no known length on the h2 body") + assertEquals(payload, capturedBody()) + assertEquals(200, networkDetails().statusCode) + } + } + + // --------------------------------------------------------------------------------------- + // responses with a known length + // --------------------------------------------------------------------------------------- + + @Test + fun `captures a gzipped body, which okhttp hands over without a known length`() { + // OkHttp asks for gzip itself and decompresses transparently, dropping Content-Length on the + // way, so an application interceptor sees -1 for an ordinary gzipped JSON response. + setUpSut() + val json = """{"hello":"world"}""" + val gzipped = Buffer() + GzipSink(gzipped).buffer().use { it.writeUtf8(json) } + MockWebServer().use { server -> + server.enqueue( + MockResponse() + .setBody(gzipped) + .setHeader("Content-Encoding", "gzip") + .setHeader("Content-Type", "application/json") + ) + + val response = sut.newCall(requestTo(server.url("/json").toString())).execute() + assertEquals(json, response.use { it.body?.string() }) + + assertEquals( + -1L, + response.body?.contentLength(), + "okhttp reports no length for a gzipped body", + ) + assertEquals(mapOf("hello" to "world"), capturedBodyValue()) + assertEquals(200, networkDetails().statusCode) + } + } + + @Test + fun `captures a body with a known length as the application reads it`() { + setUpSut() + MockWebServer().use { server -> + server.enqueue(MockResponse().setBody("response body").setResponseCode(200)) + + sut.newCall(requestTo(server.url("/hello").toString())).execute().use { it.body?.string() } + + assertEquals("response body", capturedBody()) + } + } + + @Test + fun `records an error response the application never reads, without its body`() { + // Nothing is read on the capture's own account, not even a body that would end on its own, so + // the interceptor costs a consumer no more than the bytes it asked for itself. + setUpSut() + MockWebServer().use { server -> + server.enqueue(MockResponse().setBody("failure").setResponseCode(500)) + + sut.newCall(requestTo(server.url("/hello").toString())).execute().close() + + assertEquals(500, networkDetails().statusCode) + assertNull(capturedBody()) + } + } + + @Test + fun `handles a zero length body`() { + setUpSut() + MockWebServer().use { server -> + server.enqueue(MockResponse().setResponseCode(204)) + + sut.newCall(requestTo(server.url("/hello").toString())).execute().use { response -> + assertEquals(204, response.code) + assertEquals(0, assertNotNull(response.body).contentLength()) + } + + assertEquals(204, networkDetails().statusCode) + assertNull(capturedBody()) + } + } + + @Test + fun `handles a zero length body of unknown length`() { + setUpSut() + EventStreamServer(emptyList(), closeStream = true).use { stream -> + sut.newCall(requestTo(stream.url)).execute().use { response -> + assertEquals("", response.body?.string()) + } + + assertEquals(200, networkDetails().statusCode) + assertNull(capturedBody()) + } + } + + @Test + fun `keeps the connection usable after a response of unknown length is closed`() { + setUpSut() + EventStreamServer(listOf("data: event-0\n\n"), closeStream = true).use { stream -> + // a second call on the same client proves the connection was released properly + repeat(3) { + sut.newCall(requestTo(stream.url)).execute().use { assertEquals(200, it.code) } + } + } + } + + /** + * Minimal HTTP/1.1 origin that streams one chunk per event and can leave the stream open, so the + * response body never ends. + */ + private class EventStreamServer( + private val events: List, + private val closeStream: Boolean, + private val statusCode: Int = 200, + private val dropAfterEvents: Boolean = false, + ) : Closeable { + private val server = ServerSocket(0, 8, InetAddress.getLoopbackAddress()) + private val connections = CopyOnWriteArrayList() + + init { + Thread({ acceptLoop() }, "event-stream-server").apply { isDaemon = true }.start() + } + + val url: String + get() = "http://127.0.0.1:${server.localPort}/events" + + @Suppress("SwallowedException") // the server socket is closed when the test finishes + private fun acceptLoop() { + while (!server.isClosed) { + val socket = + try { + server.accept() + } catch (e: IOException) { + return + } + connections.add(socket) + Thread({ serve(socket) }, "event-stream-connection").apply { isDaemon = true }.start() + } + } + + @Suppress("SwallowedException") // a client that hangs up mid-stream is normal here + private fun serve(socket: Socket) { + try { + socket.use { + readRequestHead(it.getInputStream()) + val output = it.getOutputStream() + val headers = + "HTTP/1.1 $statusCode OK\r\n" + + "Content-Type: text/event-stream\r\n" + + "Cache-Control: no-cache\r\n" + + "Transfer-Encoding: chunked\r\n\r\n" + output.write(headers.toByteArray()) + output.flush() + for (event in events) { + val bytes = event.toByteArray() + output.write("${bytes.size.toString(16)}\r\n".toByteArray()) + output.write(bytes) + output.write("\r\n".toByteArray()) + output.flush() + } + if (dropAfterEvents) { + // hang up without the terminating chunk, the way a mobile connection dies + return + } + if (closeStream) { + output.write("0\r\n\r\n".toByteArray()) + output.flush() + } else { + // hold the connection open: the body stays open ended + Thread.sleep(30_000) + } + } + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + } catch (e: IOException) { + // the client went away + } + } + + private fun readRequestHead(input: java.io.InputStream) { + val head = ByteArrayOutputStream() + while (true) { + val byte = input.read() + if (byte == -1) { + return + } + head.write(byte) + if (head.size() >= 4 && head.toString(Charsets.ISO_8859_1).endsWith("\r\n\r\n")) { + return + } + } + } + + override fun close() { + runCatching { server.close() } + connections.forEach { runCatching { it.close() } } + } + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 01b068ee689..8a53d2ee768 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -8311,9 +8311,17 @@ public final class io/sentry/util/network/NetworkRequestData { public fun getRequestBodySize ()Ljava/lang/Long; public fun getResponse ()Lio/sentry/util/network/ReplayNetworkRequestOrResponse; public fun getResponseBodySize ()Ljava/lang/Long; + public fun getResponseDetails ()Lio/sentry/util/network/NetworkRequestData$ResponseDetails; public fun getStatusCode ()Ljava/lang/Integer; public fun setRequestDetails (Lio/sentry/util/network/ReplayNetworkRequestOrResponse;)V - public fun setResponseDetails (ILio/sentry/util/network/ReplayNetworkRequestOrResponse;)V + public fun setResponseDetails (Lio/sentry/util/network/NetworkRequestData$ResponseDetails;)V + public fun toString ()Ljava/lang/String; +} + +public final class io/sentry/util/network/NetworkRequestData$ResponseDetails { + public fun (ILio/sentry/util/network/ReplayNetworkRequestOrResponse;)V + public fun getResponse ()Lio/sentry/util/network/ReplayNetworkRequestOrResponse; + public fun getStatusCode ()I public fun toString ()Ljava/lang/String; } diff --git a/sentry/src/main/java/io/sentry/util/network/NetworkRequestData.java b/sentry/src/main/java/io/sentry/util/network/NetworkRequestData.java index 997e001a35d..9a55c0f0bb3 100644 --- a/sentry/src/main/java/io/sentry/util/network/NetworkRequestData.java +++ b/sentry/src/main/java/io/sentry/util/network/NetworkRequestData.java @@ -13,11 +13,14 @@ @ApiStatus.Internal public final class NetworkRequestData { private @Nullable final String method; - private @Nullable Integer statusCode; - private @Nullable Long requestBodySize; - private @Nullable Long responseBodySize; - private @Nullable ReplayNetworkRequestOrResponse request; - private @Nullable ReplayNetworkRequestOrResponse response; + + // Both sides are filled in by the thread running the http call and read by the replay thread, so + // both are published through a volatile write. The response side needs it most: an integration + // that captures a body of unknown length only knows the response once the body has been consumed, + // which can be after this instance was handed to the scope. Keeping its values behind one + // reference to an immutable object means a reader sees either nothing or the complete set. + private volatile @Nullable ReplayNetworkRequestOrResponse request; + private volatile @Nullable ResponseDetails responseDetails; public NetworkRequestData(@Nullable final String method) { this.method = method; @@ -28,15 +31,18 @@ public NetworkRequestData(@Nullable final String method) { } public @Nullable Integer getStatusCode() { - return statusCode; + final ResponseDetails details = responseDetails; + return details == null ? null : details.getStatusCode(); } public @Nullable Long getRequestBodySize() { - return requestBodySize; + final ReplayNetworkRequestOrResponse requestData = request; + return requestData == null ? null : requestData.getSize(); } public @Nullable Long getResponseBodySize() { - return responseBodySize; + final ResponseDetails details = responseDetails; + return details == null ? null : details.getResponse().getSize(); } public @Nullable ReplayNetworkRequestOrResponse getRequest() { @@ -44,7 +50,8 @@ public NetworkRequestData(@Nullable final String method) { } public @Nullable ReplayNetworkRequestOrResponse getResponse() { - return response; + final ResponseDetails details = responseDetails; + return details == null ? null : details.getResponse(); } /** @@ -53,18 +60,29 @@ public NetworkRequestData(@Nullable final String method) { */ public void setRequestDetails(@NotNull final ReplayNetworkRequestOrResponse requestData) { this.request = requestData; - this.requestBodySize = requestData.getSize(); } /** - * Populates this instance with request details obtained via {@link - * NetworkDetailCaptureUtils#createResponse} + * The response details as one snapshot, or {@code null} while the response is not known yet. + * + *

Prefer this over {@link #getStatusCode()}, {@link #getResponseBodySize()} and {@link + * #getResponse()} when more than one of them is needed: the details may be replaced between two + * of those calls, which would mix one response with the next. */ - public void setResponseDetails( - final int statusCode, @NotNull final ReplayNetworkRequestOrResponse responseData) { - this.statusCode = statusCode; - this.response = responseData; - this.responseBodySize = responseData.getSize(); + public @Nullable ResponseDetails getResponseDetails() { + return responseDetails; + } + + /** + * Populates this instance with the response details assembled by the caller. + * + *

May be called from another thread than the one that created this instance, and after the + * instance was handed to the scope. A later call replaces the details of an earlier one, so an + * integration can record the status code and the headers as soon as the response arrives and add + * the body once it has been consumed. + */ + public void setResponseDetails(@NotNull final ResponseDetails details) { + this.responseDetails = details; } @Override @@ -73,16 +91,42 @@ public String toString() { + "method='" + method + '\'' - + ", statusCode=" - + statusCode - + ", requestBodySize=" - + requestBodySize - + ", responseBodySize=" - + responseBodySize + ", request=" + request - + ", response=" - + response + + ", responseDetails=" + + responseDetails + '}'; } + + /** + * The response side of a {@link NetworkRequestData}, immutable so one reference publishes all. + */ + public static final class ResponseDetails { + private final int statusCode; + private final @NotNull ReplayNetworkRequestOrResponse response; + + /** + * @param statusCode the HTTP status code of the response. + * @param response the response details obtained via {@link + * NetworkDetailCaptureUtils#createResponse} + */ + public ResponseDetails( + final int statusCode, final @NotNull ReplayNetworkRequestOrResponse response) { + this.statusCode = statusCode; + this.response = response; + } + + public int getStatusCode() { + return statusCode; + } + + public @NotNull ReplayNetworkRequestOrResponse getResponse() { + return response; + } + + @Override + public String toString() { + return "ResponseDetails{statusCode=" + statusCode + ", response=" + response + '}'; + } + } } diff --git a/sentry/src/test/java/io/sentry/util/network/NetworkRequestDataTest.kt b/sentry/src/test/java/io/sentry/util/network/NetworkRequestDataTest.kt new file mode 100644 index 00000000000..3f42e1cbae1 --- /dev/null +++ b/sentry/src/test/java/io/sentry/util/network/NetworkRequestDataTest.kt @@ -0,0 +1,79 @@ +package io.sentry.util.network + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +class NetworkRequestDataTest { + + private fun responseDetails(statusCode: Int, size: Long): NetworkRequestData.ResponseDetails = + NetworkRequestData.ResponseDetails( + statusCode, + ReplayNetworkRequestOrResponse(size, null, emptyMap()), + ) + + @Test + fun `response details are empty until they are set`() { + val data = NetworkRequestData("GET") + + assertNull(data.getStatusCode()) + assertNull(data.getResponse()) + assertNull(data.getResponseBodySize()) + } + + @Test + fun `response details are all readable once set`() { + val data = NetworkRequestData("GET") + + data.setResponseDetails(responseDetails(200, 42L)) + + assertEquals(200, data.getStatusCode()) + assertEquals(42L, data.getResponseBodySize()) + assertEquals(42L, data.getResponse()?.getSize()) + } + + @Test + fun `response details can be filled in after the instance was published`() { + // A body of unknown length is only known once it has been consumed, which can be after the + // breadcrumb + // holding this instance already reached the scope, so the values must stay consistent for a + // reader that arrives late. + val data = NetworkRequestData("GET") + assertNull(data.getStatusCode()) + assertNull(data.getResponse()) + + data.setResponseDetails(responseDetails(500, 7L)) + + assertEquals(500, data.getStatusCode()) + assertEquals(7L, data.getResponseBodySize()) + assertEquals(7L, data.getResponse()?.getSize()) + } + + @Test + fun `the response details snapshot keeps the status code and the response together`() { + val data = NetworkRequestData("GET") + assertNull(data.responseDetails) + + data.setResponseDetails(responseDetails(200, 42L)) + val first = assertNotNull(data.responseDetails) + + data.setResponseDetails(responseDetails(500, 7L)) + + assertEquals(200, first.statusCode, "a snapshot is unaffected by a later call") + assertEquals(42L, first.response.size) + assertEquals(500, data.responseDetails?.statusCode) + assertEquals(7L, data.responseDetails?.response?.size) + } + + @Test + fun `request details are unaffected`() { + val data = NetworkRequestData("GET") + val request = ReplayNetworkRequestOrResponse(11L, null, mapOf("Accept" to "application/json")) + + data.setRequestDetails(request) + + assertEquals(11L, data.getRequestBodySize()) + assertEquals(request, data.getRequest()) + } +}