Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capture event for HTTP requests resulted in server error #2287

Merged
merged 43 commits into from Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
4adc744
test
marandaneto Oct 5, 2022
371db9a
HTTP client errors for OkHttp
marandaneto Oct 10, 2022
090a7df
fix
marandaneto Oct 10, 2022
20f8b09
Format code
getsentry-bot Oct 10, 2022
76c4567
fix
marandaneto Oct 10, 2022
a664017
Merge branch 'main' into feat/httpclienterrors
marandaneto Oct 10, 2022
d046806
Merge branch 'feat/httpclienterrors' of https://github.com/getsentry/…
marandaneto Oct 10, 2022
54cac05
implement review
marandaneto Oct 12, 2022
3804a5c
Format code
getsentry-bot Oct 12, 2022
d970ff9
move the propagation targets to utils
marandaneto Oct 12, 2022
2c113a9
Merge branch 'feat/httpclienterrors' of https://github.com/getsentry/…
marandaneto Oct 12, 2022
bee4510
Format code
getsentry-bot Oct 12, 2022
1541330
ref
marandaneto Oct 12, 2022
cf81c16
merge
marandaneto Oct 12, 2022
412bc0e
remove tags
marandaneto Oct 12, 2022
9136b37
fix
marandaneto Oct 12, 2022
2284f88
fix api
marandaneto Oct 12, 2022
6268a19
fix build
marandaneto Oct 12, 2022
566d70e
Format code
getsentry-bot Oct 12, 2022
9b5e6bb
Merge branch 'main' into feat/httpclienterrors
marandaneto Oct 13, 2022
176e7ef
fix
marandaneto Oct 14, 2022
e896271
merge
marandaneto Oct 14, 2022
67fc1c1
fix conflict
marandaneto Oct 14, 2022
0e27884
add body size to request field
marandaneto Oct 14, 2022
a6211b8
api
marandaneto Oct 14, 2022
2f10f79
fix naming
marandaneto Oct 14, 2022
a26cde4
add tests
marandaneto Oct 14, 2022
d452d9d
add changelog
marandaneto Oct 14, 2022
d27d2c2
review
marandaneto Oct 17, 2022
e3bfc1f
Merge branch 'main' into feat/httpclienterrors
marandaneto Oct 17, 2022
c06174f
Format code
getsentry-bot Oct 17, 2022
76168e0
fix
marandaneto Oct 17, 2022
3fd6b01
Merge branch 'feat/httpclienterrors' of https://github.com/getsentry/…
marandaneto Oct 17, 2022
7572c8a
tests
marandaneto Oct 18, 2022
68408b5
fix test
marandaneto Oct 18, 2022
99b62a3
Merge branch 'main' into feat/httpclienterrors
marandaneto Oct 18, 2022
f0a6294
fix todos
marandaneto Oct 18, 2022
a3ca145
fixm essage
marandaneto Oct 19, 2022
38aeba2
Merge branch 'main' into feat/httpclienterrors
marandaneto Oct 19, 2022
6a4013b
review
marandaneto Oct 20, 2022
57d3776
fix
marandaneto Oct 20, 2022
636ed7e
Merge branch 'main' into feat/httpclienterrors
marandaneto Oct 20, 2022
b79a165
review
marandaneto Oct 20, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -15,6 +15,10 @@
- Add captureProfile method to hub and client ([#2290](https://github.com/getsentry/sentry-java/pull/2290))
- Report Startup Crashes ([#2277](https://github.com/getsentry/sentry-java/pull/2277))

### Features

- HTTP Client errors for OkHttp ([#2287](https://github.com/getsentry/sentry-java/pull/2287))

## 6.5.0

### Fixes
Expand Down
Expand Up @@ -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() {}

Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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)
}
}
4 changes: 2 additions & 2 deletions sentry-android-okhttp/api/sentry-android-okhttp.api
Expand Up @@ -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;
}
Expand Down
Expand Up @@ -3,23 +3,36 @@ 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(
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
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(".*")
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
) : Interceptor {

constructor() : this(HubAdapter.getInstance())
constructor(hub: IHub) : this(hub, null)
constructor(beforeSpan: BeforeSpanCallback) : this(HubAdapter.getInstance(), beforeSpan)

Expand All @@ -38,7 +51,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)
Expand All @@ -53,6 +66,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 {
Expand Down Expand Up @@ -104,6 +123,107 @@ 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 ||
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
!containsStatusCode(response.code) ||
!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
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could be called something like shouldStatusCodeCauseSentryEvent or similar?

Also should it be extracted into some util so it can be reused from other integrations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, isInRange already can be reused.
In case the language has a built-in type for Range instead of HttpStatusCodeRange, they will need to implement this anyway.
Java does not have Range obj as part of the language.

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
*/
Expand Down