Merge e3bfc1f into afa17ae

CHANGELOG.md

@@ -13,6 +13,10 @@
 - Add support for using Encoder with logback.SentryAppender ([#2246](https://github.com/getsentry/sentry-java/pull/2246))
 - Add captureProfile method to hub and client ([#2290](https://github.com/getsentry/sentry-java/pull/2290))
 
+### Features
+
+- HTTP Client errors for OkHttp ([#2287](https://github.com/getsentry/sentry-java/pull/2287))
+
 ## 6.5.0
 
 ### Fixes

sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java

@@ -78,6 +78,8 @@ final class ManifestMetadataReader {
   static final String CLIENT_REPORTS_ENABLE = "io.sentry.send-client-reports";
   static final String COLLECT_ADDITIONAL_CONTEXT = "io.sentry.additional-context";
 
+  static final String SEND_DEFAULT_PII = "io.sentry.send-default-pii";
+
   /** ManifestMetadataReader ctor */
   private ManifestMetadataReader() {}
 
@@ -297,6 +299,9 @@ static void applyMetadata(
         sdkInfo.setName(readStringNotNull(metadata, logger, SDK_NAME, sdkInfo.getName()));
         sdkInfo.setVersion(readStringNotNull(metadata, logger, SDK_VERSION, sdkInfo.getVersion()));
         options.setSdkVersion(sdkInfo);
+
+        options.setSendDefaultPii(
+            readBool(metadata, logger, SEND_DEFAULT_PII, options.isSendDefaultPii()));
       }
 
       options

sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt

@@ -1061,4 +1061,29 @@ class ManifestMetadataReaderTest {
         // Assert
         assertTrue(fixture.options.isCollectAdditionalContext)
     }
+
+    @Test
+    fun `applyMetadata reads send default pii and keep default value if not found`() {
+        // Arrange
+        val context = fixture.getContext()
+
+        // Act
+        ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)
+
+        // Assert
+        assertFalse(fixture.options.isSendDefaultPii)
+    }
+
+    @Test
+    fun `applyMetadata reads send default pii to options`() {
+        // Arrange
+        val bundle = bundleOf(ManifestMetadataReader.SEND_DEFAULT_PII to true)
+        val context = fixture.getContext(metaData = bundle)
+
+        // Act
+        ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)
+
+        // Assert
+        assertTrue(fixture.options.isSendDefaultPii)
+    }
 }

sentry-android-okhttp/api/sentry-android-okhttp.api

@@ -9,8 +9,8 @@ public final class io/sentry/android/okhttp/BuildConfig {
 public final class io/sentry/android/okhttp/SentryOkHttpInterceptor : okhttp3/Interceptor {
 	public fun <init> ()V
 	public fun <init> (Lio/sentry/IHub;)V
-	public fun <init> (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;)V
-	public synthetic fun <init> (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public fun <init> (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;ZLjava/util/List;Ljava/util/List;)V
+	public synthetic fun <init> (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;ZLjava/util/List;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun <init> (Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;)V
 	public fun intercept (Lokhttp3/Interceptor$Chain;)Lokhttp3/Response;
 }

sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt

@@ -3,23 +3,37 @@ package io.sentry.android.okhttp
 import io.sentry.BaggageHeader
 import io.sentry.Breadcrumb
 import io.sentry.Hint
+import io.sentry.HttpStatusCodeRange
 import io.sentry.HubAdapter
 import io.sentry.IHub
 import io.sentry.ISpan
+import io.sentry.SentryEvent
 import io.sentry.SpanStatus
-import io.sentry.TracePropagationTargets
 import io.sentry.TypeCheckHint.OKHTTP_REQUEST
 import io.sentry.TypeCheckHint.OKHTTP_RESPONSE
+import io.sentry.exception.ExceptionMechanismException
+import io.sentry.exception.SentryHttpClientException
+import io.sentry.protocol.Mechanism
+import io.sentry.util.HttpUtils
+import io.sentry.util.PropagationTargetsUtils
+import okhttp3.Headers
 import okhttp3.Interceptor
 import okhttp3.Request
 import okhttp3.Response
 import java.io.IOException
 
 class SentryOkHttpInterceptor(
     private val hub: IHub = HubAdapter.getInstance(),
-    private val beforeSpan: BeforeSpanCallback? = null
+    private val beforeSpan: BeforeSpanCallback? = null,
+    // should this be under the options or here? also define the names
+    private val captureFailedRequests: Boolean = false,
+    private val failedRequestStatusCodes: List<HttpStatusCodeRange> = listOf(
+        HttpStatusCodeRange(HttpStatusCodeRange.DEFAULT_MIN, HttpStatusCodeRange.DEFAULT_MAX)
+    ),
+    private val failedRequestTargets: List<String> = listOf(".*")
 ) : Interceptor {
 
+    constructor() : this(HubAdapter.getInstance())
     constructor(hub: IHub) : this(hub, null)
     constructor(beforeSpan: BeforeSpanCallback) : this(HubAdapter.getInstance(), beforeSpan)
 
@@ -38,7 +52,7 @@ class SentryOkHttpInterceptor(
         try {
             val requestBuilder = request.newBuilder()
             if (span != null &&
-                TracePropagationTargets.contain(hub.options.tracePropagationTargets, request.url.toString())
+                PropagationTargetsUtils.contain(hub.options.tracePropagationTargets, request.url.toString())
             ) {
                 span.toSentryTrace().let {
                     requestBuilder.addHeader(it.name, it.value)
@@ -53,6 +67,12 @@ class SentryOkHttpInterceptor(
             response = chain.proceed(request)
             code = response.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
+            captureEvent(request, response)
+
             return response
         } catch (e: IOException) {
             span?.apply {
@@ -104,6 +124,115 @@ class SentryOkHttpInterceptor(
         }
     }
 
+    private fun captureEvent(request: Request, response: Response) {
+        // not possible to get a parameterized url, but we remove at least the
+        // query string and the fragment.
+        // url example: https://api.github.com/users/getsentry/repos/#fragment?query=query
+        // url will be: https://api.github.com/users/getsentry/repos/
+        // ideally we'd like a parameterized url: https://api.github.com/users/{user}/repos/
+        // but that's not possible
+        var requestUrl = request.url.toString()
+
+        val query = request.url.query
+        if (!query.isNullOrEmpty()) {
+            requestUrl = requestUrl.replace("?$query", "")
+        }
+
+        val urlFragment = request.url.fragment
+        if (!urlFragment.isNullOrEmpty()) {
+            requestUrl = requestUrl.replace("#$urlFragment", "")
+        }
+
+        if (!captureFailedRequests ||
+            !containsStatusCode(response.code) ||
+            !PropagationTargetsUtils.contain(failedRequestTargets, requestUrl)
+        ) {
+            return
+        }
+
+        val mechanism = Mechanism().apply {
+            type = "SentryOkHttpInterceptor"
+        }
+        val exception = SentryHttpClientException(
+            "Event was captured because the request status code was ${response.code}"
+        )
+        val mechanismException = ExceptionMechanismException(mechanism, exception, Thread.currentThread(), true)
+        val event = SentryEvent(mechanismException)
+
+        val hint = Hint()
+        hint.set(OKHTTP_REQUEST, request)
+        hint.set(OKHTTP_RESPONSE, response)
+
+        // remove after fields indexed
+//        val tags = mutableMapOf<String, String>()
+//        tags["status_code"] = response.code.toString()
+//        tags["url"] = requestUrl
+
+        val sentryRequest = io.sentry.protocol.Request().apply {
+            url = requestUrl
+            // Cookie is only sent if isSendDefaultPii is enabled
+            cookies = if (hub.options.isSendDefaultPii) request.headers["Cookie"] else null
+            method = request.method
+            queryString = query
+            headers = getHeaders(request.headers)
+            fragment = urlFragment
+
+            request.body?.contentLength().ifHasValidLength {
+                // should be mapped in relay and added to the protocol, right now
+                // relay isn't retaining unmapped fields
+                bodySize = it
+            }
+        }
+
+        val sentryResponse = io.sentry.protocol.Response().apply {
+            // Cookie is only sent if isSendDefaultPii is enabled due to PII
+            cookies = if (hub.options.isSendDefaultPii) response.headers["Cookie"] else null
+            headers = getHeaders(response.headers)
+            statusCode = response.code
+
+            response.body?.contentLength().ifHasValidLength {
+                bodySize = it
+            }
+        }
+
+//        event.tags = tags
+        event.request = sentryRequest
+        event.contexts.setResponse(sentryResponse)
+
+        hub.captureEvent(event, hint)
+    }
+
+    private fun containsStatusCode(statusCode: Int): Boolean {
+        for (item in failedRequestStatusCodes) {
+            if (item.isInRange(statusCode)) {
+                return true
+            }
+        }
+        return false
+    }
+
+    private fun getHeaders(requestHeaders: Headers): MutableMap<String, String>? {
+        // Headers are only sent if isSendDefaultPii is enabled due to PII
+        if (!hub.options.isSendDefaultPii) {
+            return null
+        }
+
+        val headers = mutableMapOf<String, String>()
+
+        for (i in 0 until requestHeaders.size) {
+            val name = requestHeaders.name(i)
+
+            // header is only sent if isn't sensitive
+            if (HttpUtils.containsSensitiveHeader(name)) {
+                continue
+            }
+
+            val value = requestHeaders.value(i)
+            headers[name] = value
+        }
+        return headers
+    }
+
     /**
      * The BeforeSpan callback
      */

sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt

@@ -15,8 +15,8 @@ import io.sentry.IHub
 import io.sentry.ISpan
 import io.sentry.SentryLevel
 import io.sentry.SpanStatus
-import io.sentry.TracePropagationTargets
 import io.sentry.TypeCheckHint
+import io.sentry.util.PropagationTargetsUtils
 
 class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IHub = HubAdapter.getInstance(), private val beforeSpan: BeforeSpanCallback? = null) :
     HttpInterceptor {
@@ -33,7 +33,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH
 
             var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList()
 
-            if (TracePropagationTargets.contain(hub.options.tracePropagationTargets, request.url)) {
+            if (PropagationTargetsUtils.contain(hub.options.tracePropagationTargets, request.url)) {
                 val sentryTraceHeader = span.toSentryTrace()
                 val baggageHeader = span.toBaggageHeader(request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value })
                 cleanedHeaders.add(HttpHeader(sentryTraceHeader.name, sentryTraceHeader.value))

sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java

@@ -13,8 +13,8 @@
 import io.sentry.ISpan;
 import io.sentry.SentryTraceHeader;
 import io.sentry.SpanStatus;
-import io.sentry.TracePropagationTargets;
 import io.sentry.util.Objects;
+import io.sentry.util.PropagationTargetsUtils;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -55,7 +55,7 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O
 
       final RequestWrapper requestWrapper = new RequestWrapper(request);
 
-      if (TracePropagationTargets.contain(hub.getOptions().getTracePropagationTargets(), url)) {
+      if (PropagationTargetsUtils.contain(hub.getOptions().getTracePropagationTargets(), url)) {
         final SentryTraceHeader sentryTraceHeader = span.toSentryTrace();
         final @Nullable Collection<String> requestBaggageHeader =
             request.headers().get(BaggageHeader.BAGGAGE_HEADER);

sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml

@@ -135,5 +135,8 @@
         <!--    how to enable the attach screenshot feature-->
         <meta-data android:name="io.sentry.attach-screenshot" android:value="true" />
 
+        <!--    how to enable the send default pii-->
+        <meta-data android:name="io.sentry.send-default-pii" android:value="true" />
+
     </application>
 </manifest>

sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt

@@ -10,7 +10,7 @@ interface GitHubService {
     @GET("users/{user}/repos")
     fun listRepos(@Path("user") user: String): Call<List<Repo>>
 
-    @GET("users/{user}/repos")
+    @GET("users/{user}/repos/")
     suspend fun listReposAsync(@Path("user") user: String, @Query("per_page") perPage: Int): List<Repo>
 }
 

sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt

@@ -1,13 +1,22 @@
 package io.sentry.samples.android
 
+import io.sentry.HttpStatusCodeRange
 import io.sentry.android.okhttp.SentryOkHttpInterceptor
 import okhttp3.OkHttpClient
 import retrofit2.Retrofit
 import retrofit2.converter.gson.GsonConverterFactory
 
 object GithubAPI {
 
-    private val client = OkHttpClient.Builder().addInterceptor(SentryOkHttpInterceptor()).build()
+    private val client = OkHttpClient.Builder().addInterceptor(
+        SentryOkHttpInterceptor(
+            captureFailedRequests = true,
+            // TODO: 200 just for testing
+            failedRequestStatusCodes = listOf(
+                HttpStatusCodeRange(200, 599)
+            )
+        )
+    ).build()
 
     private val retrofit = Retrofit.Builder()
         .baseUrl("https://api.github.com/")

sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt

@@ -90,7 +90,12 @@ fun Github(
     val scope = rememberCoroutineScope()
 
     LaunchedEffect(perPage) {
-        result = GithubAPI.service.listReposAsync(user.text, perPage).random().full_name
+        try {
+            result = GithubAPI.service.listReposAsync(user.text, perPage).random().full_name
+        } catch (e: Throwable) {
+            // TODO: event processor that converts retrofit HttpException to a proper sentry event
+            Sentry.captureException(e)
+        }
     }
 
     Column(
@@ -108,7 +113,12 @@ fun Github(
         Button(
             onClick = {
                 scope.launch {
-                    result = GithubAPI.service.listReposAsync(user.text, perPage).random().full_name
+                    try {
+                        result =
+                            GithubAPI.service.listReposAsync(user.text, perPage).random().full_name
+                    } catch (e: Throwable) {
+                        Sentry.captureException(e)
+                    }
                 }
             },
             modifier = Modifier.padding(top = 32.dp)

sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java

@@ -3,23 +3,18 @@
 import com.jakewharton.nopen.annotation.Open;
 import io.sentry.IHub;
 import io.sentry.protocol.Request;
+import io.sentry.util.HttpUtils;
 import io.sentry.util.Objects;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashMap;
-import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import javax.servlet.http.HttpServletRequest;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 @Open
 public class SentryRequestResolver {
-  private static final List<String> SENSITIVE_HEADERS =
-      Arrays.asList("X-FORWARDED-FOR", "AUTHORIZATION", "COOKIE");
-
   private final @NotNull IHub hub;
 
   public SentryRequestResolver(final @NotNull IHub hub) {
@@ -46,8 +41,7 @@ Map<String, String> resolveHeadersMap(final @NotNull HttpServletRequest request)
     final Map<String, String> headersMap = new HashMap<>();
     for (String headerName : Collections.list(request.getHeaderNames())) {
       // do not copy personal information identifiable headers
-      if (hub.getOptions().isSendDefaultPii()
-          || !SENSITIVE_HEADERS.contains(headerName.toUpperCase(Locale.ROOT))) {
+      if (hub.getOptions().isSendDefaultPii() || !HttpUtils.containsSensitiveHeader(headerName)) {
         headersMap.put(headerName, toString(request.getHeaders(headerName)));
       }
     }