Capture event for HTTP requests resulted in server error (#2287)

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>

CHANGELOG.md

@@ -15,6 +15,7 @@
 - Profile envelopes are sent directly from profiler ([#2298](https://github.com/getsentry/sentry-java/pull/2298))
 - Add support for using Encoder with logback.SentryAppender ([#2246](https://github.com/getsentry/sentry-java/pull/2246))
 - Report Startup Crashes ([#2277](https://github.com/getsentry/sentry-java/pull/2277))
+- HTTP Client errors for OkHttp ([#2287](https://github.com/getsentry/sentry-java/pull/2287))
 
 ### Dependencies
 

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,50 @@ 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
 
+/**
+ * The Sentry's [SentryOkHttpInterceptor], it will automatically add a breadcrumb and start a span
+ * out of the active span bound to the scope for each HTTP Request.
+ * If [captureFailedRequests] is enabled, the SDK will capture HTTP Client errors as well.
+ *
+ * @param hub The [IHub], internal and only used for testing.
+ * @param beforeSpan The [ISpan] can be customized or dropped with the [BeforeSpanCallback].
+ * @param captureFailedRequests The SDK will only capture HTTP Client errors if it is enabled,
+ * Defaults to false.
+ * @param failedRequestStatusCodes The SDK will only capture HTTP Client errors if the HTTP Response
+ * status code is within the defined ranges.
+ * @param failedRequestTargets The SDK will only capture HTTP Client errors if the HTTP Request URL
+ * is a match for any of the defined targets.
+ */
 class SentryOkHttpInterceptor(
  private val hub: IHub = HubAdapter.getInstance(),
- private val beforeSpan: BeforeSpanCallback? = null
+ private val beforeSpan: BeforeSpanCallback? = null,
+ 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 +65,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 +80,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 +137,110 @@ class SentryOkHttpInterceptor(
  }
  }
 
+ private fun captureEvent(request: Request, response: Response) {
+ // return if the feature is disabled or its not within the range
+ if (!captureFailedRequests || !containsStatusCode(response.code)) {
+ return
+ }
+
+ // 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", "")
+ }
+
+ // return if its not a target match
+ if (!PropagationTargetsUtils.contain(failedRequestTargets, requestUrl)) {
+ return
+ }
+
+ val mechanism = Mechanism().apply {
+ type = "SentryOkHttpInterceptor"
+ }
+ val exception = SentryHttpClientException(
+ "HTTP Client Error with status code: ${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)
+
+ 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 {
+ 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.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
  */