Only rebuild response in OkHttp interceptor if we modify the body (#33)

## Goal We were rebuilding the response even if we didn't change it, which is unnecessary work. As such, this change simply bypasses and reuses the response that we get initially from calling `chain.proceed(request)` ## Testing Existing tests pass
embrace-io · Oct 30, 2023 · b183026 · b183026
2 parents 8cf8db4 + 6be995f
commit b183026
Showing 1 changed file with 73 additions and 51 deletions.
diff --git a/...3/src/main/java/io/embrace/android/embracesdk/okhttp3/EmbraceOkHttp3NetworkInterceptor.kt b/...3/src/main/java/io/embrace/android/embracesdk/okhttp3/EmbraceOkHttp3NetworkInterceptor.kt
@@ -40,81 +40,69 @@ public class EmbraceOkHttp3NetworkInterceptor internal constructor(
 ) : Interceptor {
     public constructor() : this(Embrace.getInstance(), Clock { System.currentTimeMillis() })
 
-    @Suppress("ComplexMethod")
     @Throws(IOException::class)
     override fun intercept(chain: Interceptor.Chain): Response {
+        // If the SDK has not started, don't do anything
         val originalRequest: Request = chain.request()
         if (!embrace.isStarted || embrace.internalInterface.isInternalNetworkCaptureDisabled()) {
             return chain.proceed(originalRequest)
         }
-        val offset = sdkClockOffset()
+
         val networkSpanForwardingEnabled = embrace.internalInterface.isNetworkSpanForwardingEnabled()
         var traceparent: String? = null
         if (networkSpanForwardingEnabled && originalRequest.header(TRACEPARENT_HEADER_NAME) == null) {
             traceparent = embrace.generateW3cTraceparent()
         }
         val request =
             if (traceparent == null) originalRequest else originalRequest.newBuilder().header(TRACEPARENT_HEADER_NAME, traceparent).build()
+
+        // Take a snapshot of the difference in the system and SDK clocks and send the request along the chain
+        val offset = sdkClockOffset()
         val networkResponse: Response = chain.proceed(request)
-        val responseBuilder: Response.Builder = networkResponse.newBuilder().request(request)
-        var contentLength: Long? = null
-        // Try to get the content length from the header
-        val contentLengthHeaderValue = networkResponse.header(CONTENT_LENGTH_HEADER_NAME)
-        if (contentLengthHeaderValue != null) {
-            try {
-                contentLength = contentLengthHeaderValue.toLong()
-            } catch (ex: Exception) {
-                // Ignore
-            }
-        }
 
-        // If we get the body for a server-sent events stream, then we will wait forever
-        val contentType = networkResponse.header(CONTENT_TYPE_HEADER_NAME)
+        // Get response and determine the size of the body
+        var contentLength: Long? = getContentLengthFromHeader(networkResponse)
 
-        // Tolerant of a charset specified in header,
-        // e.g. Content-Type: text/event-stream;charset=UTF-8
-        val serverSentEvent = contentType != null && contentType.startsWith(CONTENT_TYPE_EVENT_STREAM)
-        if (!serverSentEvent && contentLength == null) {
-            try {
-                val body = networkResponse.body
-                if (body != null) {
-                    val source = body.source()
-                    source.request(Long.MAX_VALUE)
-                    contentLength = source.buffer.size
-                }
-            } catch (ex: Exception) {
-                // Ignore
-            }
+        if (contentLength == null) {
+            // If we get the body for a server-sent events stream, then we will wait forever
+            contentLength = getContentLengthFromBody(networkResponse, networkResponse.header(CONTENT_TYPE_HEADER_NAME))
         }
+
         if (contentLength == null) {
-            // Otherwise default to zero
+            // Set the content length to 0 if we can't determine it
             contentLength = 0L
         }
-        val shouldCaptureNetworkData = embrace.internalInterface.shouldCaptureNetworkBody(request.url.toString(), request.method)
-        if (shouldCaptureNetworkData &&
-            ENCODING_GZIP.equals(networkResponse.header(CONTENT_ENCODING_HEADER_NAME), ignoreCase = true) &&
-            networkResponse.promisesBody()
-        ) {
-            val body = networkResponse.body
-            if (body != null) {
-                val strippedHeaders = networkResponse.headers.newBuilder()
-                    .removeAll(CONTENT_ENCODING_HEADER_NAME)
-                    .removeAll(CONTENT_LENGTH_HEADER_NAME)
-                    .build()
-                val realResponseBody = RealResponseBody(
-                    contentType,
-                    -1L,
-                    GzipSource(body.source()).buffer()
-                )
-                responseBuilder.headers(strippedHeaders)
-                responseBuilder.body(realResponseBody)
-            }
-        }
-        val response: Response = responseBuilder.build()
+
+        var response: Response = networkResponse
         var networkCaptureData: NetworkCaptureData? = null
+        val shouldCaptureNetworkData = embrace.internalInterface.shouldCaptureNetworkBody(request.url.toString(), request.method)
+
+        // If we need to capture the network response body,
         if (shouldCaptureNetworkData) {
+            if (ENCODING_GZIP.equals(networkResponse.header(CONTENT_ENCODING_HEADER_NAME), ignoreCase = true) &&
+                networkResponse.promisesBody()
+            ) {
+                val body = networkResponse.body
+                if (body != null) {
+                    val strippedHeaders = networkResponse.headers.newBuilder()
+                        .removeAll(CONTENT_ENCODING_HEADER_NAME)
+                        .removeAll(CONTENT_LENGTH_HEADER_NAME)
+                        .build()
+                    val realResponseBody = RealResponseBody(
+                        networkResponse.header(CONTENT_TYPE_HEADER_NAME),
+                        -1L,
+                        GzipSource(body.source()).buffer()
+                    )
+                    val responseBuilder = networkResponse.newBuilder().request(request)
+                    responseBuilder.headers(strippedHeaders)
+                    responseBuilder.body(realResponseBody)
+                    response = responseBuilder.build()
+                }
+            }
+
             networkCaptureData = getNetworkCaptureData(request, response)
         }
+
         embrace.recordNetworkRequest(
             EmbraceNetworkRequest.fromCompletedRequest(
                 EmbraceHttpPathOverride.getURLString(EmbraceOkHttp3PathOverrideRequest(request)),
@@ -132,6 +120,40 @@ public class EmbraceOkHttp3NetworkInterceptor internal constructor(
         return response
     }
 
+    private fun getContentLengthFromHeader(networkResponse: Response): Long? {
+        var contentLength: Long? = null
+        val contentLengthHeaderValue = networkResponse.header(CONTENT_LENGTH_HEADER_NAME)
+        if (contentLengthHeaderValue != null) {
+            try {
+                contentLength = contentLengthHeaderValue.toLong()
+            } catch (ex: Exception) {
+                // Ignore
+            }
+        }
+        return contentLength
+    }
+
+    private fun getContentLengthFromBody(networkResponse: Response, contentType: String?): Long? {
+        var contentLength: Long? = null
+
+        // Tolerant of a charset specified in header, e.g. Content-Type: text/event-stream;charset=UTF-8
+        val serverSentEvent = contentType != null && contentType.startsWith(CONTENT_TYPE_EVENT_STREAM)
+        if (!serverSentEvent) {
+            try {
+                val body = networkResponse.body
+                if (body != null) {
+                    val source = body.source()
+                    source.request(Long.MAX_VALUE)
+                    contentLength = source.buffer.size
+                }
+            } catch (ex: Exception) {
+                // Ignore
+            }
+        }
+
+        return contentLength
+    }
+
     private fun getNetworkCaptureData(request: Request, response: Response): NetworkCaptureData {
         var requestHeaders: Map<String, String>? = null
         var requestQueryParams: String? = null