From 4adc7446d5903701bbc80e02bbe6c444fc12fe33 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 5 Oct 2022 15:25:48 +0200 Subject: [PATCH 01/32] test --- .../io/sentry/android/core/ManifestMetadataReader.java | 4 ++++ .../io/sentry/android/okhttp/SentryHttpClientError.kt | 5 +++++ .../java/io/sentry/android/okhttp/StatusCodeRange.kt | 9 +++++++++ .../sentry-samples-android/src/main/AndroidManifest.xml | 3 +++ .../src/main/java/io/sentry/samples/android/GithubAPI.kt | 5 ++++- 5 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt create mode 100644 sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index b22f4f3bf1..d6bcbb1283 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/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,8 @@ 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 diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt new file mode 100644 index 0000000000..d428cd7ebc --- /dev/null +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt @@ -0,0 +1,5 @@ +package io.sentry.android.okhttp + +class SentryHttpClientError(override val message: String) : RuntimeException(message) { + +} diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt new file mode 100644 index 0000000000..162bab1793 --- /dev/null +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt @@ -0,0 +1,9 @@ +package io.sentry.android.okhttp + +class StatusCodeRange(private val min: Int, private val max: Int) { + constructor(statusCode: Int) : this(statusCode, statusCode) + + fun isInRange(statusCode: Int) : Boolean { + return statusCode in min..max + } +} diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index cc10b895af..9e82bdc8b3 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -135,5 +135,8 @@ + + + diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt index 94c3362420..2a25924812 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt @@ -1,13 +1,16 @@ package io.sentry.samples.android import io.sentry.android.okhttp.SentryOkHttpInterceptor +import io.sentry.android.okhttp.StatusCodeRange 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, failedRequestStatusCodes = listOf( + StatusCodeRange(200, 599) + ))).build() private val retrofit = Retrofit.Builder() .baseUrl("https://api.github.com/") From 371db9aa5fc58b0f042f0237f32d453bc0a3a420 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 10 Oct 2022 17:36:07 +0200 Subject: [PATCH 02/32] HTTP client errors for OkHttp --- .../android/core/ManifestMetadataReader.java | 3 +- sentry-android-okhttp/build.gradle.kts | 1 + .../android/okhttp/SentryHttpClientError.kt | 5 +- .../android/okhttp/SentryOkHttpInterceptor.kt | 132 ++++++++++++- .../sentry/android/okhttp/StatusCodeRange.kt | 5 +- .../okhttp/SentryOkHttpInterceptorTest.kt | 6 +- .../sentry/samples/android/GitHubService.kt | 4 +- .../io/sentry/samples/android/GithubAPI.kt | 11 +- .../android/compose/ComposeActivity.kt | 13 +- .../io/sentry/TracePropagationTargets.java | 2 + .../java/io/sentry/protocol/Contexts.java | 31 ++- .../main/java/io/sentry/protocol/Request.java | 21 ++ .../java/io/sentry/protocol/Response.java | 179 ++++++++++++++++++ 13 files changed, 389 insertions(+), 24 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/protocol/Response.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index d6bcbb1283..08393836fc 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -300,7 +300,8 @@ static void applyMetadata( sdkInfo.setVersion(readStringNotNull(metadata, logger, SDK_VERSION, sdkInfo.getVersion())); options.setSdkVersion(sdkInfo); - options.setSendDefaultPii(readBool(metadata, logger, SEND_DEFAULT_PII, options.isSendDefaultPii())); + options.setSendDefaultPii( + readBool(metadata, logger, SEND_DEFAULT_PII, options.isSendDefaultPii())); } options diff --git a/sentry-android-okhttp/build.gradle.kts b/sentry-android-okhttp/build.gradle.kts index 3b74a72bb1..d4d44ec366 100644 --- a/sentry-android-okhttp/build.gradle.kts +++ b/sentry-android-okhttp/build.gradle.kts @@ -71,6 +71,7 @@ dependencies { compileOnly(Config.Libs.okhttpBom) compileOnly(Config.Libs.okhttp) +// compileOnly(Config.CompileOnly.jetbrainsAnnotations) implementation(kotlin(Config.kotlinStdLib, KotlinCompilerVersion.VERSION)) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt index d428cd7ebc..a34e03230a 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt @@ -1,5 +1,4 @@ package io.sentry.android.okhttp -class SentryHttpClientError(override val message: String) : RuntimeException(message) { - -} +// @ApiStatus.Internal +class SentryHttpClientError(override val message: String) : RuntimeException(message) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index bd51ee0635..b68c3517bb 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -8,8 +8,13 @@ import io.sentry.IHub import io.sentry.ISpan import io.sentry.SpanStatus import io.sentry.TracePropagationTargets +import io.sentry.SentryEvent import io.sentry.TypeCheckHint.OKHTTP_REQUEST import io.sentry.TypeCheckHint.OKHTTP_RESPONSE +import io.sentry.exception.ExceptionMechanismException +import io.sentry.protocol.Mechanism +import io.sentry.protocol.Message +import okhttp3.Headers import okhttp3.Interceptor import okhttp3.Request import okhttp3.Response @@ -17,7 +22,10 @@ import java.io.IOException 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 failedRequestStatusCode: List = listOf(StatusCodeRange(500, 599)), + private val failedRequestsTargets: List = listOf(".*") ) : Interceptor { constructor(hub: IHub) : this(hub, null) @@ -53,6 +61,9 @@ class SentryOkHttpInterceptor( response = chain.proceed(request) code = response.code span?.status = SpanStatus.fromHttpStatusCode(code) + + captureEvent(request, response) + return response } catch (e: IOException) { span?.apply { @@ -104,6 +115,125 @@ class SentryOkHttpInterceptor( } } + // TODO: ignore exceptions of the type UnknownHostException + + 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 || !TracePropagationTargets.contain(failedRequestsTargets, requestUrl) || !containsStatusCode(response.code)) { + return + } + + val mechanism = Mechanism().apply { + type = "SentryOkHttpInterceptor" + +// // with description, mechanism in the UI is buggy +// description = message + + // TODO: should it be synthetic? likely not, mechanism SentryOkHttpInterceptor should be considered for grouping +// synthetic = true + } + val exception = SentryHttpClientError("Event was captured because the request status code was ${response.code}") + // TODO: okhttp thread uses thread pool so its always the same stack trace (okhttp frames only), stacktrace is set as snapshot=true, is that ok? + val mechanismException = ExceptionMechanismException(mechanism, exception, Thread.currentThread(), true) + val event = SentryEvent(mechanismException) + + // TODO: Do we need a message if the request and response already have the info? + val sentryMessage = Message().apply { + message = "HTTP url: %s status: %s" + params = listOf(requestUrl, response.code.toString()) + } + + val hint = Hint() + hint.set("request", request) + hint.set("response", response) + + // TODO: remove after fields indexed + val tags = mutableMapOf() + tags["status_code"] = response.code.toString() + tags["url"] = requestUrl + + val unknownRequestFields = mutableMapOf() + + val sentryRequest = io.sentry.protocol.Request().apply { + url = requestUrl + cookies = request.headers["Cookie"] + method = request.method + queryString = query + headers = getHeaders(request.headers) + fragment = urlFragment + + request.body?.contentLength().ifHasValidLength { + // TODO: should be mapped in relay and added to the protocol + unknownRequestFields["body_size"] = it + } + + unknown = unknownRequestFields.ifEmpty { null } + } + + val sentryResponse = io.sentry.protocol.Response().apply { + cookies = response.headers["Cookie"] + headers = getHeaders(response.headers) + statusCode = response.code + + response.body?.contentLength().ifHasValidLength { + bodySize = it + } + } + + event.tags = tags + event.request = sentryRequest + event.contexts.setResponse(sentryResponse) + event.message = sentryMessage + + hub.captureEvent(event, hint) + + // TODO: event processor that converts retrofit HttpException to a proper sentry event + } + + private fun containsStatusCode(statusCode: Int): Boolean { + for(item in failedRequestStatusCode) { + if (item.isInRange(statusCode)) { + return true + } + } + return false + } + + private fun getHeaders(requestHeaders: Headers): MutableMap? { + // TODO: should it be under isSendDefaultPii? + // Server already does data scrubbing + if (!hub.options.isSendDefaultPii) { + return null + } + + val headers = mutableMapOf() + + for (i in 0 until requestHeaders.size) { + // TODO: should we remove sentry-trace and baggage from headers? + val name = requestHeaders.name(i) + val value = requestHeaders.value(i) + headers[name] = value + } + return headers + } + /** * The BeforeSpan callback */ diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt index 162bab1793..6486592bcb 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt @@ -1,9 +1,12 @@ package io.sentry.android.okhttp +//import org.jetbrains.annotations.ApiStatus + +//@ApiStatus.Internal class StatusCodeRange(private val min: Int, private val max: Int) { constructor(statusCode: Int) : this(statusCode, statusCode) - fun isInRange(statusCode: Int) : Boolean { + fun isInRange(statusCode: Int): Boolean { return statusCode in min..max } } diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index 850d88e590..bac75f8109 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -36,7 +36,7 @@ class SentryOkHttpInterceptorTest { class Fixture { val hub = mock() - var interceptor = SentryOkHttpInterceptor(hub) + var interceptor = SentryOkHttpInterceptor_old(hub) val server = MockWebServer() lateinit var sentryTracer: SentryTracer lateinit var options: SentryOptions @@ -47,7 +47,7 @@ class SentryOkHttpInterceptorTest { httpStatusCode: Int = 201, responseBody: String = "success", socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, - beforeSpan: SentryOkHttpInterceptor.BeforeSpanCallback? = null, + beforeSpan: SentryOkHttpInterceptor_old.BeforeSpanCallback? = null, includeMockServerInTracePropagationTargets: Boolean = true, keepDefaultTracePropagationTargets: Boolean = false, ): OkHttpClient { @@ -75,7 +75,7 @@ class SentryOkHttpInterceptorTest { ) if (beforeSpan != null) { - interceptor = SentryOkHttpInterceptor(hub, beforeSpan) + interceptor = SentryOkHttpInterceptor_old(hub, beforeSpan) } return OkHttpClient.Builder().addInterceptor(interceptor).build() } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt index 36b6a5a495..4957cf7031 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt @@ -10,7 +10,9 @@ interface GitHubService { @GET("users/{user}/repos") fun listRepos(@Path("user") user: String): Call> - @GET("users/{user}/repos") + // TODO: @GET("users/{user}/repos/#test") throws 404 +// @GET("users/{user}/repos/") + @GET("users/{user}/repos/#test") suspend fun listReposAsync(@Path("user") user: String, @Query("per_page") perPage: Int): List } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt index 2a25924812..b77b2e21c4 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt @@ -8,9 +8,14 @@ import retrofit2.converter.gson.GsonConverterFactory object GithubAPI { - private val client = OkHttpClient.Builder().addInterceptor(SentryOkHttpInterceptor(captureFailedRequests = true, failedRequestStatusCodes = listOf( - StatusCodeRange(200, 599) - ))).build() + private val client = OkHttpClient.Builder().addInterceptor( + SentryOkHttpInterceptor( + captureFailedRequests = true, + failedRequestStatusCode = listOf( + StatusCodeRange(200, 599) + ) + ) + ).build() private val retrofit = Retrofit.Builder() .baseUrl("https://api.github.com/") diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt index 74f7cdccfc..d2ff934ac4 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt @@ -90,7 +90,11 @@ 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) { + Sentry.captureException(e) + } } Column( @@ -108,7 +112,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) diff --git a/sentry/src/main/java/io/sentry/TracePropagationTargets.java b/sentry/src/main/java/io/sentry/TracePropagationTargets.java index 3425642854..0047d2bd2c 100644 --- a/sentry/src/main/java/io/sentry/TracePropagationTargets.java +++ b/sentry/src/main/java/io/sentry/TracePropagationTargets.java @@ -4,6 +4,8 @@ import java.util.List; import org.jetbrains.annotations.NotNull; +// TODO: rename class to something more generic + /** * Checks if an URL matches the list of origins to which `sentry-trace` header should be sent in * HTTP integrations. diff --git a/sentry/src/main/java/io/sentry/protocol/Contexts.java b/sentry/src/main/java/io/sentry/protocol/Contexts.java index 842aaa2d9b..f6f179c76b 100644 --- a/sentry/src/main/java/io/sentry/protocol/Contexts.java +++ b/sentry/src/main/java/io/sentry/protocol/Contexts.java @@ -22,9 +22,9 @@ public final class Contexts extends ConcurrentHashMap implements public Contexts() {} public Contexts(final @NotNull Contexts contexts) { - for (Map.Entry entry : contexts.entrySet()) { + for (final Map.Entry entry : contexts.entrySet()) { if (entry != null) { - Object value = entry.getValue(); + final Object value = entry.getValue(); if (App.TYPE.equals(entry.getKey()) && value instanceof App) { this.setApp(new App((App) value)); } else if (Browser.TYPE.equals(entry.getKey()) && value instanceof Browser) { @@ -40,6 +40,8 @@ public Contexts(final @NotNull Contexts contexts) { this.setGpu(new Gpu((Gpu) value)); } else if (SpanContext.TYPE.equals(entry.getKey()) && value instanceof SpanContext) { this.setTrace(new SpanContext((SpanContext) value)); + } else if (Response.TYPE.equals(entry.getKey()) && value instanceof Response) { + this.setResponse(new Response((Response) value)); } else { this.put(entry.getKey(), value); } @@ -109,17 +111,25 @@ public void setGpu(final @NotNull Gpu gpu) { this.put(Gpu.TYPE, gpu); } + public @Nullable Response getResponse() { + return toContextType(Response.TYPE, Response.class); + } + + public void setResponse(final @NotNull Response response) { + this.put(Response.TYPE, response); + } + // region json @Override - public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) + public void serialize(final @NotNull JsonObjectWriter writer, final @NotNull ILogger logger) throws IOException { writer.beginObject(); // Serialize in alphabetical order to keep determinism. - List sortedKeys = Collections.list(keys()); - java.util.Collections.sort(sortedKeys); - for (String key : sortedKeys) { - Object value = get(key); + final List sortedKeys = Collections.list(keys()); + Collections.sort(sortedKeys); + for (final String key : sortedKeys) { + final Object value = get(key); if (value != null) { writer.name(key).value(logger, value); } @@ -130,9 +140,9 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) public static final class Deserializer implements JsonDeserializer { @Override - public @NotNull Contexts deserialize(@NotNull JsonObjectReader reader, @NotNull ILogger logger) + public @NotNull Contexts deserialize(final @NotNull JsonObjectReader reader, final @NotNull ILogger logger) throws Exception { - Contexts contexts = new Contexts(); + final Contexts contexts = new Contexts(); reader.beginObject(); while (reader.peek() == JsonToken.NAME) { final String nextName = reader.nextName(); @@ -159,6 +169,9 @@ public static final class Deserializer implements JsonDeserializer { case SpanContext.TYPE: contexts.setTrace(new SpanContext.Deserializer().deserialize(reader, logger)); break; + case Response.TYPE: + contexts.setResponse(new Response.Deserializer().deserialize(reader, logger)); + break; default: Object object = reader.nextObjectOrNull(); if (object != null) { diff --git a/sentry/src/main/java/io/sentry/protocol/Request.java b/sentry/src/main/java/io/sentry/protocol/Request.java index 9c496d2b78..240756ae66 100644 --- a/sentry/src/main/java/io/sentry/protocol/Request.java +++ b/sentry/src/main/java/io/sentry/protocol/Request.java @@ -102,6 +102,11 @@ public final class Request implements JsonUnknown, JsonSerializable { private @Nullable Map other; + /** + * The fragment (anchor) of the request URL. + */ + private @Nullable String fragment; + @SuppressWarnings("unused") private @Nullable Map unknown; @@ -117,6 +122,7 @@ public Request(final @NotNull Request request) { this.other = CollectionUtils.newConcurrentHashMap(request.other); this.unknown = CollectionUtils.newConcurrentHashMap(request.unknown); this.data = request.data; + this.fragment = request.fragment; } public @Nullable String getUrl() { @@ -196,6 +202,14 @@ public void setUnknown(@Nullable Map unknown) { this.unknown = unknown; } + public @Nullable String getFragment() { + return fragment; + } + + public void setFragment(final @Nullable String fragment) { + this.fragment = fragment; + } + public static final class JsonKeys { public static final String URL = "url"; public static final String METHOD = "method"; @@ -205,6 +219,7 @@ public static final class JsonKeys { public static final String HEADERS = "headers"; public static final String ENV = "env"; public static final String OTHER = "other"; + public static final String FRAGMENT = "fragment"; } @Override @@ -235,6 +250,9 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) if (other != null) { writer.name(JsonKeys.OTHER).value(logger, other); } + if (fragment != null) { + writer.name(JsonKeys.FRAGMENT).value(logger, fragment); + } if (unknown != null) { for (String key : unknown.keySet()) { Object value = unknown.get(key); @@ -290,6 +308,9 @@ public static final class Deserializer implements JsonDeserializer { request.other = CollectionUtils.newConcurrentHashMap(deserializedOther); } break; + case JsonKeys.FRAGMENT: + request.fragment = reader.nextStringOrNull(); + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); diff --git a/sentry/src/main/java/io/sentry/protocol/Response.java b/sentry/src/main/java/io/sentry/protocol/Response.java new file mode 100644 index 0000000000..f4c842947c --- /dev/null +++ b/sentry/src/main/java/io/sentry/protocol/Response.java @@ -0,0 +1,179 @@ +package io.sentry.protocol; + +import io.sentry.ILogger; +import io.sentry.JsonDeserializer; +import io.sentry.JsonObjectReader; +import io.sentry.JsonObjectWriter; +import io.sentry.JsonSerializable; +import io.sentry.JsonUnknown; +import io.sentry.util.CollectionUtils; +import io.sentry.vendor.gson.stream.JsonToken; +import java.io.IOException; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class Response implements JsonUnknown, JsonSerializable { + public static final String TYPE = "response"; + + /** + * The cookie values. + * + *

Can be given unparsed as string, as dictionary, or as a list of tuples. + */ + private @Nullable String cookies; + /** + * A dictionary of response headers. + * + *

If a header appears multiple times it, needs to be merged according to the HTTP standard for + * header merging. Header names are treated case-insensitively by Sentry. + */ + private @Nullable Map headers; + + /** + * The HTTP response status code + */ + private @Nullable Integer statusCode; + + /** + * The body size in bytes + */ + private @Nullable Long bodySize; + + @SuppressWarnings("unused") + private @Nullable Map unknown; + + public Response() {} + + public Response(final @NotNull Response response) { + this.cookies = response.cookies; + this.headers = CollectionUtils.newConcurrentHashMap(response.headers); + this.unknown = CollectionUtils.newConcurrentHashMap(response.unknown); + this.setStatusCode(response.getStatusCode()); + this.setBodySize(response.getBodySize()); + } + + public @Nullable String getCookies() { + return cookies; + } + + public void setCookies(final @Nullable String cookies) { + this.cookies = cookies; + } + + public @Nullable Map getHeaders() { + return headers; + } + + public void setHeaders(final @Nullable Map headers) { + this.headers = CollectionUtils.newConcurrentHashMap(headers); + } + + // region json + + @Nullable + @Override + public Map getUnknown() { + return unknown; + } + + @Override + public void setUnknown(final @Nullable Map unknown) { + this.unknown = unknown; + } + + public @Nullable Integer getStatusCode() { + return statusCode; + } + + public void setStatusCode(final @Nullable Integer statusCode) { + this.statusCode = statusCode; + } + + public @Nullable Long getBodySize() { + return bodySize; + } + + public void setBodySize(final @Nullable Long bodySize) { + this.bodySize = bodySize; + } + + public static final class JsonKeys { + public static final String COOKIES = "cookies"; + public static final String HEADERS = "headers"; + public static final String STATUS_CODE = "status_code"; + public static final String BODY_SIZE = "body_size"; + } + + @Override + public void serialize(final @NotNull JsonObjectWriter writer, final @NotNull ILogger logger) + throws IOException { + writer.beginObject(); + + if (cookies != null) { + writer.name(JsonKeys.COOKIES).value(cookies); + } + if (headers != null) { + writer.name(JsonKeys.HEADERS).value(logger, headers); + } + if (getStatusCode() != null) { + writer.name(JsonKeys.STATUS_CODE).value(logger, getStatusCode()); + } + if (getBodySize() != null) { + writer.name(JsonKeys.BODY_SIZE).value(logger, getBodySize()); + } + + if (unknown != null) { + for (final String key : unknown.keySet()) { + final Object value = unknown.get(key); + writer.name(key); + writer.value(logger, value); + } + } + writer.endObject(); + } + + @SuppressWarnings("unchecked") + public static final class Deserializer implements JsonDeserializer { + @Override + public @NotNull Response deserialize(final @NotNull JsonObjectReader reader, final @NotNull ILogger logger) + throws Exception { + reader.beginObject(); + final Response response = new Response(); + Map unknown = null; + while (reader.peek() == JsonToken.NAME) { + final String nextName = reader.nextName(); + switch (nextName) { + case JsonKeys.COOKIES: + response.cookies = reader.nextStringOrNull(); + break; + case JsonKeys.HEADERS: + final Map deserializedHeaders = + (Map) reader.nextObjectOrNull(); + if (deserializedHeaders != null) { + response.headers = CollectionUtils.newConcurrentHashMap(deserializedHeaders); + } + break; + case JsonKeys.STATUS_CODE: + response.setStatusCode(reader.nextIntegerOrNull()); + break; + case JsonKeys.BODY_SIZE: + response.setBodySize(reader.nextLongOrNull()); + break; + default: + if (unknown == null) { + unknown = new ConcurrentHashMap<>(); + } + reader.nextUnknown(logger, unknown, nextName); + break; + } + } + response.setUnknown(unknown); + reader.endObject(); + return response; + } + } + + // endregion +} From 090a7df65e959ebfeea7965344b4846d56ebe086 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 10 Oct 2022 17:39:34 +0200 Subject: [PATCH 03/32] fix --- .../io/sentry/android/okhttp/SentryOkHttpInterceptor.kt | 2 -- .../io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt | 6 +++--- .../main/java/io/sentry/samples/android/GitHubService.kt | 3 +-- .../src/main/java/io/sentry/samples/android/GithubAPI.kt | 1 + .../io/sentry/samples/android/compose/ComposeActivity.kt | 1 + 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index b68c3517bb..2f12b34423 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -203,8 +203,6 @@ class SentryOkHttpInterceptor( event.message = sentryMessage hub.captureEvent(event, hint) - - // TODO: event processor that converts retrofit HttpException to a proper sentry event } private fun containsStatusCode(statusCode: Int): Boolean { diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index bac75f8109..850d88e590 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -36,7 +36,7 @@ class SentryOkHttpInterceptorTest { class Fixture { val hub = mock() - var interceptor = SentryOkHttpInterceptor_old(hub) + var interceptor = SentryOkHttpInterceptor(hub) val server = MockWebServer() lateinit var sentryTracer: SentryTracer lateinit var options: SentryOptions @@ -47,7 +47,7 @@ class SentryOkHttpInterceptorTest { httpStatusCode: Int = 201, responseBody: String = "success", socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, - beforeSpan: SentryOkHttpInterceptor_old.BeforeSpanCallback? = null, + beforeSpan: SentryOkHttpInterceptor.BeforeSpanCallback? = null, includeMockServerInTracePropagationTargets: Boolean = true, keepDefaultTracePropagationTargets: Boolean = false, ): OkHttpClient { @@ -75,7 +75,7 @@ class SentryOkHttpInterceptorTest { ) if (beforeSpan != null) { - interceptor = SentryOkHttpInterceptor_old(hub, beforeSpan) + interceptor = SentryOkHttpInterceptor(hub, beforeSpan) } return OkHttpClient.Builder().addInterceptor(interceptor).build() } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt index 4957cf7031..635f1c86da 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt @@ -11,8 +11,7 @@ interface GitHubService { fun listRepos(@Path("user") user: String): Call> // TODO: @GET("users/{user}/repos/#test") throws 404 -// @GET("users/{user}/repos/") - @GET("users/{user}/repos/#test") + @GET("users/{user}/repos/") suspend fun listReposAsync(@Path("user") user: String, @Query("per_page") perPage: Int): List } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt index b77b2e21c4..39e8539b77 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt @@ -11,6 +11,7 @@ object GithubAPI { private val client = OkHttpClient.Builder().addInterceptor( SentryOkHttpInterceptor( captureFailedRequests = true, + // TODO: 200 just for testing failedRequestStatusCode = listOf( StatusCodeRange(200, 599) ) diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt index d2ff934ac4..01b765bec7 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt @@ -93,6 +93,7 @@ fun Github( 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) } } From 20f8b095eebacda4cab1fe0c546880a152ae8240 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Mon, 10 Oct 2022 15:42:54 +0000 Subject: [PATCH 04/32] Format code --- .../android/okhttp/SentryOkHttpInterceptor.kt | 4 +- .../sentry/android/okhttp/StatusCodeRange.kt | 4 +- .../java/io/sentry/protocol/Contexts.java | 4 +- .../main/java/io/sentry/protocol/Request.java | 4 +- .../java/io/sentry/protocol/Response.java | 292 +++++++++--------- 5 files changed, 151 insertions(+), 157 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 2f12b34423..a137240a71 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -6,9 +6,9 @@ import io.sentry.Hint 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.SentryEvent import io.sentry.TypeCheckHint.OKHTTP_REQUEST import io.sentry.TypeCheckHint.OKHTTP_RESPONSE import io.sentry.exception.ExceptionMechanismException @@ -206,7 +206,7 @@ class SentryOkHttpInterceptor( } private fun containsStatusCode(statusCode: Int): Boolean { - for(item in failedRequestStatusCode) { + for (item in failedRequestStatusCode) { if (item.isInRange(statusCode)) { return true } diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt index 6486592bcb..5a2aa46452 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt @@ -1,8 +1,8 @@ package io.sentry.android.okhttp -//import org.jetbrains.annotations.ApiStatus +// import org.jetbrains.annotations.ApiStatus -//@ApiStatus.Internal +// @ApiStatus.Internal class StatusCodeRange(private val min: Int, private val max: Int) { constructor(statusCode: Int) : this(statusCode, statusCode) diff --git a/sentry/src/main/java/io/sentry/protocol/Contexts.java b/sentry/src/main/java/io/sentry/protocol/Contexts.java index f6f179c76b..fe72ad0a37 100644 --- a/sentry/src/main/java/io/sentry/protocol/Contexts.java +++ b/sentry/src/main/java/io/sentry/protocol/Contexts.java @@ -140,8 +140,8 @@ public void serialize(final @NotNull JsonObjectWriter writer, final @NotNull ILo public static final class Deserializer implements JsonDeserializer { @Override - public @NotNull Contexts deserialize(final @NotNull JsonObjectReader reader, final @NotNull ILogger logger) - throws Exception { + public @NotNull Contexts deserialize( + final @NotNull JsonObjectReader reader, final @NotNull ILogger logger) throws Exception { final Contexts contexts = new Contexts(); reader.beginObject(); while (reader.peek() == JsonToken.NAME) { diff --git a/sentry/src/main/java/io/sentry/protocol/Request.java b/sentry/src/main/java/io/sentry/protocol/Request.java index 240756ae66..5a69a0a60d 100644 --- a/sentry/src/main/java/io/sentry/protocol/Request.java +++ b/sentry/src/main/java/io/sentry/protocol/Request.java @@ -102,9 +102,7 @@ public final class Request implements JsonUnknown, JsonSerializable { private @Nullable Map other; - /** - * The fragment (anchor) of the request URL. - */ + /** The fragment (anchor) of the request URL. */ private @Nullable String fragment; @SuppressWarnings("unused") diff --git a/sentry/src/main/java/io/sentry/protocol/Response.java b/sentry/src/main/java/io/sentry/protocol/Response.java index f4c842947c..e8820c77ed 100644 --- a/sentry/src/main/java/io/sentry/protocol/Response.java +++ b/sentry/src/main/java/io/sentry/protocol/Response.java @@ -15,165 +15,161 @@ import org.jetbrains.annotations.Nullable; public final class Response implements JsonUnknown, JsonSerializable { - public static final String TYPE = "response"; - - /** - * The cookie values. - * - *

Can be given unparsed as string, as dictionary, or as a list of tuples. - */ - private @Nullable String cookies; - /** - * A dictionary of response headers. - * - *

If a header appears multiple times it, needs to be merged according to the HTTP standard for - * header merging. Header names are treated case-insensitively by Sentry. - */ - private @Nullable Map headers; - - /** - * The HTTP response status code - */ - private @Nullable Integer statusCode; - - /** - * The body size in bytes - */ - private @Nullable Long bodySize; - - @SuppressWarnings("unused") - private @Nullable Map unknown; - - public Response() {} - - public Response(final @NotNull Response response) { - this.cookies = response.cookies; - this.headers = CollectionUtils.newConcurrentHashMap(response.headers); - this.unknown = CollectionUtils.newConcurrentHashMap(response.unknown); - this.setStatusCode(response.getStatusCode()); - this.setBodySize(response.getBodySize()); + public static final String TYPE = "response"; + + /** + * The cookie values. + * + *

Can be given unparsed as string, as dictionary, or as a list of tuples. + */ + private @Nullable String cookies; + /** + * A dictionary of response headers. + * + *

If a header appears multiple times it, needs to be merged according to the HTTP standard for + * header merging. Header names are treated case-insensitively by Sentry. + */ + private @Nullable Map headers; + + /** The HTTP response status code */ + private @Nullable Integer statusCode; + + /** The body size in bytes */ + private @Nullable Long bodySize; + + @SuppressWarnings("unused") + private @Nullable Map unknown; + + public Response() {} + + public Response(final @NotNull Response response) { + this.cookies = response.cookies; + this.headers = CollectionUtils.newConcurrentHashMap(response.headers); + this.unknown = CollectionUtils.newConcurrentHashMap(response.unknown); + this.setStatusCode(response.getStatusCode()); + this.setBodySize(response.getBodySize()); + } + + public @Nullable String getCookies() { + return cookies; + } + + public void setCookies(final @Nullable String cookies) { + this.cookies = cookies; + } + + public @Nullable Map getHeaders() { + return headers; + } + + public void setHeaders(final @Nullable Map headers) { + this.headers = CollectionUtils.newConcurrentHashMap(headers); + } + + // region json + + @Nullable + @Override + public Map getUnknown() { + return unknown; + } + + @Override + public void setUnknown(final @Nullable Map unknown) { + this.unknown = unknown; + } + + public @Nullable Integer getStatusCode() { + return statusCode; + } + + public void setStatusCode(final @Nullable Integer statusCode) { + this.statusCode = statusCode; + } + + public @Nullable Long getBodySize() { + return bodySize; + } + + public void setBodySize(final @Nullable Long bodySize) { + this.bodySize = bodySize; + } + + public static final class JsonKeys { + public static final String COOKIES = "cookies"; + public static final String HEADERS = "headers"; + public static final String STATUS_CODE = "status_code"; + public static final String BODY_SIZE = "body_size"; + } + + @Override + public void serialize(final @NotNull JsonObjectWriter writer, final @NotNull ILogger logger) + throws IOException { + writer.beginObject(); + + if (cookies != null) { + writer.name(JsonKeys.COOKIES).value(cookies); } - - public @Nullable String getCookies() { - return cookies; - } - - public void setCookies(final @Nullable String cookies) { - this.cookies = cookies; + if (headers != null) { + writer.name(JsonKeys.HEADERS).value(logger, headers); } - - public @Nullable Map getHeaders() { - return headers; + if (getStatusCode() != null) { + writer.name(JsonKeys.STATUS_CODE).value(logger, getStatusCode()); } - - public void setHeaders(final @Nullable Map headers) { - this.headers = CollectionUtils.newConcurrentHashMap(headers); + if (getBodySize() != null) { + writer.name(JsonKeys.BODY_SIZE).value(logger, getBodySize()); } - // region json - - @Nullable - @Override - public Map getUnknown() { - return unknown; + if (unknown != null) { + for (final String key : unknown.keySet()) { + final Object value = unknown.get(key); + writer.name(key); + writer.value(logger, value); + } } + writer.endObject(); + } + @SuppressWarnings("unchecked") + public static final class Deserializer implements JsonDeserializer { @Override - public void setUnknown(final @Nullable Map unknown) { - this.unknown = unknown; - } - - public @Nullable Integer getStatusCode() { - return statusCode; - } - - public void setStatusCode(final @Nullable Integer statusCode) { - this.statusCode = statusCode; - } - - public @Nullable Long getBodySize() { - return bodySize; - } - - public void setBodySize(final @Nullable Long bodySize) { - this.bodySize = bodySize; - } - - public static final class JsonKeys { - public static final String COOKIES = "cookies"; - public static final String HEADERS = "headers"; - public static final String STATUS_CODE = "status_code"; - public static final String BODY_SIZE = "body_size"; - } - - @Override - public void serialize(final @NotNull JsonObjectWriter writer, final @NotNull ILogger logger) - throws IOException { - writer.beginObject(); - - if (cookies != null) { - writer.name(JsonKeys.COOKIES).value(cookies); - } - if (headers != null) { - writer.name(JsonKeys.HEADERS).value(logger, headers); - } - if (getStatusCode() != null) { - writer.name(JsonKeys.STATUS_CODE).value(logger, getStatusCode()); - } - if (getBodySize() != null) { - writer.name(JsonKeys.BODY_SIZE).value(logger, getBodySize()); - } - - if (unknown != null) { - for (final String key : unknown.keySet()) { - final Object value = unknown.get(key); - writer.name(key); - writer.value(logger, value); + public @NotNull Response deserialize( + final @NotNull JsonObjectReader reader, final @NotNull ILogger logger) throws Exception { + reader.beginObject(); + final Response response = new Response(); + Map unknown = null; + while (reader.peek() == JsonToken.NAME) { + final String nextName = reader.nextName(); + switch (nextName) { + case JsonKeys.COOKIES: + response.cookies = reader.nextStringOrNull(); + break; + case JsonKeys.HEADERS: + final Map deserializedHeaders = + (Map) reader.nextObjectOrNull(); + if (deserializedHeaders != null) { + response.headers = CollectionUtils.newConcurrentHashMap(deserializedHeaders); } - } - writer.endObject(); - } - - @SuppressWarnings("unchecked") - public static final class Deserializer implements JsonDeserializer { - @Override - public @NotNull Response deserialize(final @NotNull JsonObjectReader reader, final @NotNull ILogger logger) - throws Exception { - reader.beginObject(); - final Response response = new Response(); - Map unknown = null; - while (reader.peek() == JsonToken.NAME) { - final String nextName = reader.nextName(); - switch (nextName) { - case JsonKeys.COOKIES: - response.cookies = reader.nextStringOrNull(); - break; - case JsonKeys.HEADERS: - final Map deserializedHeaders = - (Map) reader.nextObjectOrNull(); - if (deserializedHeaders != null) { - response.headers = CollectionUtils.newConcurrentHashMap(deserializedHeaders); - } - break; - case JsonKeys.STATUS_CODE: - response.setStatusCode(reader.nextIntegerOrNull()); - break; - case JsonKeys.BODY_SIZE: - response.setBodySize(reader.nextLongOrNull()); - break; - default: - if (unknown == null) { - unknown = new ConcurrentHashMap<>(); - } - reader.nextUnknown(logger, unknown, nextName); - break; - } + break; + case JsonKeys.STATUS_CODE: + response.setStatusCode(reader.nextIntegerOrNull()); + break; + case JsonKeys.BODY_SIZE: + response.setBodySize(reader.nextLongOrNull()); + break; + default: + if (unknown == null) { + unknown = new ConcurrentHashMap<>(); } - response.setUnknown(unknown); - reader.endObject(); - return response; + reader.nextUnknown(logger, unknown, nextName); + break; } + } + response.setUnknown(unknown); + reader.endObject(); + return response; } + } - // endregion + // endregion } From 76c456756db570c260148aceb6dc1eadbf727103 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 10 Oct 2022 17:44:22 +0200 Subject: [PATCH 05/32] fix --- .../java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 2f12b34423..34b7433eca 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -23,6 +23,7 @@ import java.io.IOException class SentryOkHttpInterceptor( private val hub: IHub = HubAdapter.getInstance(), private val beforeSpan: BeforeSpanCallback? = null, + // TODO: should this be under the options or here? also define the names private val captureFailedRequests: Boolean = false, private val failedRequestStatusCode: List = listOf(StatusCodeRange(500, 599)), private val failedRequestsTargets: List = listOf(".*") @@ -62,6 +63,7 @@ class SentryOkHttpInterceptor( code = response.code span?.status = SpanStatus.fromHttpStatusCode(code) + // OkHttp errors (4xx, 5xx) don't throw, so it's safe to call within this block. captureEvent(request, response) return response From 54cac05b738d878e127c274cc07389387c1946de Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 12 Oct 2022 11:06:17 +0200 Subject: [PATCH 06/32] implement review --- .../android/okhttp/SentryOkHttpInterceptor.kt | 31 +++++------ .../sentry/spring/SentryRequestResolver.java | 9 +--- .../spring/webflux/SentryRequestResolver.java | 8 +-- sentry/src/main/java/io/sentry/Baggage.java | 6 +-- .../src/main/java/io/sentry/OutboxSender.java | 4 +- .../main/java/io/sentry/SentryOptions.java | 8 +-- .../java/io/sentry/util/HttpHeadersUtils.java | 18 +++++++ ...mpleRateUtil.java => SampleRateUtils.java} | 2 +- .../java/io/sentry/util/SampleRateUtilTest.kt | 54 +++++++++---------- 9 files changed, 71 insertions(+), 69 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java rename sentry/src/main/java/io/sentry/util/{SampleRateUtil.java => SampleRateUtils.java} (96%) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 95d63020e6..6c58fa3890 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -14,6 +14,7 @@ import io.sentry.TypeCheckHint.OKHTTP_RESPONSE import io.sentry.exception.ExceptionMechanismException import io.sentry.protocol.Mechanism import io.sentry.protocol.Message +import io.sentry.util.HttpHeadersUtils import okhttp3.Headers import okhttp3.Interceptor import okhttp3.Request @@ -144,24 +145,11 @@ class SentryOkHttpInterceptor( val mechanism = Mechanism().apply { type = "SentryOkHttpInterceptor" - -// // with description, mechanism in the UI is buggy -// description = message - - // TODO: should it be synthetic? likely not, mechanism SentryOkHttpInterceptor should be considered for grouping -// synthetic = true } val exception = SentryHttpClientError("Event was captured because the request status code was ${response.code}") - // TODO: okhttp thread uses thread pool so its always the same stack trace (okhttp frames only), stacktrace is set as snapshot=true, is that ok? val mechanismException = ExceptionMechanismException(mechanism, exception, Thread.currentThread(), true) val event = SentryEvent(mechanismException) - // TODO: Do we need a message if the request and response already have the info? - val sentryMessage = Message().apply { - message = "HTTP url: %s status: %s" - params = listOf(requestUrl, response.code.toString()) - } - val hint = Hint() hint.set("request", request) hint.set("response", response) @@ -175,7 +163,8 @@ class SentryOkHttpInterceptor( val sentryRequest = io.sentry.protocol.Request().apply { url = requestUrl - cookies = request.headers["Cookie"] + // 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) @@ -190,7 +179,8 @@ class SentryOkHttpInterceptor( } val sentryResponse = io.sentry.protocol.Response().apply { - cookies = response.headers["Cookie"] + // 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 @@ -202,7 +192,6 @@ class SentryOkHttpInterceptor( event.tags = tags event.request = sentryRequest event.contexts.setResponse(sentryResponse) - event.message = sentryMessage hub.captureEvent(event, hint) } @@ -217,8 +206,7 @@ class SentryOkHttpInterceptor( } private fun getHeaders(requestHeaders: Headers): MutableMap? { - // TODO: should it be under isSendDefaultPii? - // Server already does data scrubbing + // Headers are only sent if isSendDefaultPii is enabled due to PII if (!hub.options.isSendDefaultPii) { return null } @@ -226,8 +214,13 @@ class SentryOkHttpInterceptor( val headers = mutableMapOf() for (i in 0 until requestHeaders.size) { - // TODO: should we remove sentry-trace and baggage from headers? val name = requestHeaders.name(i) + + // header is only sent if isn't sensitive + if (HttpHeadersUtils.containsSensitiveHeader(name)) { + continue + } + val value = requestHeaders.value(i) headers[name] = value } diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java index ec78868ad4..da9ac214dd 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java @@ -3,13 +3,11 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.IHub; import io.sentry.protocol.Request; +import io.sentry.util.HttpHeadersUtils; 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; @@ -17,9 +15,6 @@ @Open public class SentryRequestResolver { - private static final List SENSITIVE_HEADERS = - Arrays.asList("X-FORWARDED-FOR", "AUTHORIZATION", "COOKIE"); - private final @NotNull IHub hub; public SentryRequestResolver(final @NotNull IHub hub) { @@ -47,7 +42,7 @@ Map resolveHeadersMap(final @NotNull HttpServletRequest request) 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))) { + || !HttpHeadersUtils.containsSensitiveHeader(headerName)) { headersMap.put(headerName, toString(request.getHeaders(headerName))); } } diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java index 52c6ef49d9..1d3f18eb00 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java +++ b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java @@ -3,11 +3,10 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.IHub; import io.sentry.protocol.Request; +import io.sentry.util.HttpHeadersUtils; import io.sentry.util.Objects; -import java.util.Arrays; import java.util.HashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -18,9 +17,6 @@ @Open @ApiStatus.Experimental public class SentryRequestResolver { - private static final List SENSITIVE_HEADERS = - Arrays.asList("X-FORWARDED-FOR", "AUTHORIZATION", "COOKIE"); - private final @NotNull IHub hub; public SentryRequestResolver(final @NotNull IHub hub) { @@ -46,7 +42,7 @@ Map resolveHeadersMap(final HttpHeaders request) { for (Map.Entry> entry : request.entrySet()) { // do not copy personal information identifiable headers if (hub.getOptions().isSendDefaultPii() - || !SENSITIVE_HEADERS.contains(entry.getKey().toUpperCase(Locale.ROOT))) { + || !HttpHeadersUtils.containsSensitiveHeader(entry.getKey())) { headersMap.put(entry.getKey(), toString(entry.getValue())); } } diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index d3bfae0a6f..a0cc0aacf1 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -3,7 +3,7 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.TransactionNameSource; import io.sentry.protocol.User; -import io.sentry.util.SampleRateUtil; +import io.sentry.util.SampleRateUtils; import io.sentry.util.StringUtils; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; @@ -313,7 +313,7 @@ public void setValuesFromTransaction( } private static @Nullable String sampleRateToString(@Nullable Double sampleRateAsDouble) { - if (!SampleRateUtil.isValidTracesSampleRate(sampleRateAsDouble, false)) { + if (!SampleRateUtils.isValidTracesSampleRate(sampleRateAsDouble, false)) { return null; } @@ -333,7 +333,7 @@ private static boolean isHighQualityTransactionName( if (sampleRateString != null) { try { double sampleRate = Double.parseDouble(sampleRateString); - if (SampleRateUtil.isValidTracesSampleRate(sampleRate, false)) { + if (SampleRateUtils.isValidTracesSampleRate(sampleRate, false)) { return sampleRate; } } catch (NumberFormatException e) { diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index 902e76bfb0..7381bdc00a 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -13,7 +13,7 @@ import io.sentry.util.HintUtils; import io.sentry.util.LogUtils; import io.sentry.util.Objects; -import io.sentry.util.SampleRateUtil; +import io.sentry.util.SampleRateUtils; import java.io.BufferedInputStream; import java.io.BufferedReader; import java.io.ByteArrayInputStream; @@ -229,7 +229,7 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @NotN if (sampleRateString != null) { try { final Double sampleRate = Double.parseDouble(sampleRateString); - if (!SampleRateUtil.isValidTracesSampleRate(sampleRate, false)) { + if (!SampleRateUtils.isValidTracesSampleRate(sampleRate, false)) { logger.log( SentryLevel.ERROR, "Invalid sample rate parsed from TraceContext: %s", diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index e7db4c41fa..39dca1ad54 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -10,7 +10,7 @@ import io.sentry.transport.NoOpEnvelopeCache; import io.sentry.transport.NoOpTransportGate; import io.sentry.util.Platform; -import io.sentry.util.SampleRateUtil; +import io.sentry.util.SampleRateUtils; import io.sentry.util.StringUtils; import java.io.File; import java.util.ArrayList; @@ -745,7 +745,7 @@ public void setProxy(@Nullable Proxy proxy) { * @param sampleRate the sample rate */ public void setSampleRate(Double sampleRate) { - if (!SampleRateUtil.isValidSampleRate(sampleRate)) { + if (!SampleRateUtils.isValidSampleRate(sampleRate)) { throw new IllegalArgumentException( "The value " + sampleRate @@ -769,7 +769,7 @@ public void setSampleRate(Double sampleRate) { * @param tracesSampleRate the sample rate */ public void setTracesSampleRate(final @Nullable Double tracesSampleRate) { - if (!SampleRateUtil.isValidTracesSampleRate(tracesSampleRate)) { + if (!SampleRateUtils.isValidTracesSampleRate(tracesSampleRate)) { throw new IllegalArgumentException( "The value " + tracesSampleRate @@ -1568,7 +1568,7 @@ public void setProfilesSampler(final @Nullable ProfilesSamplerCallback profilesS * @param profilesSampleRate the sample rate */ public void setProfilesSampleRate(final @Nullable Double profilesSampleRate) { - if (!SampleRateUtil.isValidProfilesSampleRate(profilesSampleRate)) { + if (!SampleRateUtils.isValidProfilesSampleRate(profilesSampleRate)) { throw new IllegalArgumentException( "The value " + profilesSampleRate diff --git a/sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java b/sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java new file mode 100644 index 0000000000..abb7519f82 --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java @@ -0,0 +1,18 @@ +package io.sentry.util; + +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +import java.util.Arrays; +import java.util.List; +import java.util.Locale; + +@ApiStatus.Internal +public final class HttpHeadersUtils { + private static final List SENSITIVE_HEADERS = + Arrays.asList("X-FORWARDED-FOR", "AUTHORIZATION", "COOKIE"); + + public static boolean containsSensitiveHeader(final @NotNull String header) { + return SENSITIVE_HEADERS.contains(header.toUpperCase(Locale.ROOT)); + } +} diff --git a/sentry/src/main/java/io/sentry/util/SampleRateUtil.java b/sentry/src/main/java/io/sentry/util/SampleRateUtils.java similarity index 96% rename from sentry/src/main/java/io/sentry/util/SampleRateUtil.java rename to sentry/src/main/java/io/sentry/util/SampleRateUtils.java index 6bc5cea9c2..201b81390c 100644 --- a/sentry/src/main/java/io/sentry/util/SampleRateUtil.java +++ b/sentry/src/main/java/io/sentry/util/SampleRateUtils.java @@ -4,7 +4,7 @@ import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -public final class SampleRateUtil { +public final class SampleRateUtils { public static boolean isValidSampleRate(@Nullable Double sampleRate) { return isValidSampleRate(sampleRate, true); diff --git a/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt b/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt index a2693e6141..db8cbf917b 100644 --- a/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt +++ b/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt @@ -8,136 +8,136 @@ class SampleRateUtilTest { @Test fun `accepts 0 dot 01 for sample rate`() { - assertTrue(SampleRateUtil.isValidSampleRate(0.01)) + assertTrue(SampleRateUtils.isValidSampleRate(0.01)) } @Test fun `accepts 1 for sample rate`() { - assertTrue(SampleRateUtil.isValidSampleRate(1.0)) + assertTrue(SampleRateUtils.isValidSampleRate(1.0)) } @Test fun `rejects 0 for sample rate`() { - assertFalse(SampleRateUtil.isValidSampleRate(0.0)) + assertFalse(SampleRateUtils.isValidSampleRate(0.0)) } @Test fun `rejects 1 dot 01 for sample rate`() { - assertFalse(SampleRateUtil.isValidSampleRate(1.01)) + assertFalse(SampleRateUtils.isValidSampleRate(1.01)) } @Test fun `rejects negative sample rate`() { - assertFalse(SampleRateUtil.isValidSampleRate(-0.5)) + assertFalse(SampleRateUtils.isValidSampleRate(-0.5)) } @Test fun `rejects NaN sample rate`() { - assertFalse(SampleRateUtil.isValidSampleRate(Double.NaN)) + assertFalse(SampleRateUtils.isValidSampleRate(Double.NaN)) } @Test fun `rejects positive infinite sample rate`() { - assertFalse(SampleRateUtil.isValidSampleRate(Double.POSITIVE_INFINITY)) + assertFalse(SampleRateUtils.isValidSampleRate(Double.POSITIVE_INFINITY)) } @Test fun `rejects negative infinite sample rate`() { - assertFalse(SampleRateUtil.isValidSampleRate(Double.NEGATIVE_INFINITY)) + assertFalse(SampleRateUtils.isValidSampleRate(Double.NEGATIVE_INFINITY)) } @Test fun `accepts null sample rate if told so`() { - assertTrue(SampleRateUtil.isValidSampleRate(null, true)) + assertTrue(SampleRateUtils.isValidSampleRate(null, true)) } @Test fun `rejects null sample rate if told so`() { - assertFalse(SampleRateUtil.isValidSampleRate(null, false)) + assertFalse(SampleRateUtils.isValidSampleRate(null, false)) } @Test fun `accepts 0 for traces sample rate`() { - assertTrue(SampleRateUtil.isValidTracesSampleRate(0.0)) + assertTrue(SampleRateUtils.isValidTracesSampleRate(0.0)) } @Test fun `accepts 1 for traces sample rate`() { - assertTrue(SampleRateUtil.isValidTracesSampleRate(1.0)) + assertTrue(SampleRateUtils.isValidTracesSampleRate(1.0)) } @Test fun `rejects negative traces sample rate`() { - assertFalse(SampleRateUtil.isValidTracesSampleRate(-0.5)) + assertFalse(SampleRateUtils.isValidTracesSampleRate(-0.5)) } @Test fun `rejects 1 dot 01 for traces sample rate`() { - assertFalse(SampleRateUtil.isValidTracesSampleRate(1.01)) + assertFalse(SampleRateUtils.isValidTracesSampleRate(1.01)) } @Test fun `rejects NaN traces sample rate`() { - assertFalse(SampleRateUtil.isValidTracesSampleRate(Double.NaN)) + assertFalse(SampleRateUtils.isValidTracesSampleRate(Double.NaN)) } @Test fun `rejects positive infinite traces sample rate`() { - assertFalse(SampleRateUtil.isValidTracesSampleRate(Double.POSITIVE_INFINITY)) + assertFalse(SampleRateUtils.isValidTracesSampleRate(Double.POSITIVE_INFINITY)) } @Test fun `rejects negative infinite traces sample rate`() { - assertFalse(SampleRateUtil.isValidTracesSampleRate(Double.NEGATIVE_INFINITY)) + assertFalse(SampleRateUtils.isValidTracesSampleRate(Double.NEGATIVE_INFINITY)) } @Test fun `accepts null traces sample rate if told so`() { - assertTrue(SampleRateUtil.isValidTracesSampleRate(null, true)) + assertTrue(SampleRateUtils.isValidTracesSampleRate(null, true)) } @Test fun `rejects null traces sample rate if told so`() { - assertFalse(SampleRateUtil.isValidTracesSampleRate(null, false)) + assertFalse(SampleRateUtils.isValidTracesSampleRate(null, false)) } @Test fun `accepts 0 for profiles sample rate`() { - assertTrue(SampleRateUtil.isValidProfilesSampleRate(0.0)) + assertTrue(SampleRateUtils.isValidProfilesSampleRate(0.0)) } @Test fun `accepts 1 for profiles sample rate`() { - assertTrue(SampleRateUtil.isValidProfilesSampleRate(1.0)) + assertTrue(SampleRateUtils.isValidProfilesSampleRate(1.0)) } @Test fun `rejects negative profiles sample rate`() { - assertFalse(SampleRateUtil.isValidProfilesSampleRate(-0.5)) + assertFalse(SampleRateUtils.isValidProfilesSampleRate(-0.5)) } @Test fun `rejects 1 dot 01 for profiles sample rate`() { - assertFalse(SampleRateUtil.isValidProfilesSampleRate(1.01)) + assertFalse(SampleRateUtils.isValidProfilesSampleRate(1.01)) } @Test fun `rejects NaN profiles sample rate`() { - assertFalse(SampleRateUtil.isValidProfilesSampleRate(Double.NaN)) + assertFalse(SampleRateUtils.isValidProfilesSampleRate(Double.NaN)) } @Test fun `rejects positive infinite profiles sample rate`() { - assertFalse(SampleRateUtil.isValidProfilesSampleRate(Double.POSITIVE_INFINITY)) + assertFalse(SampleRateUtils.isValidProfilesSampleRate(Double.POSITIVE_INFINITY)) } @Test fun `rejects negative infinite profiles sample rate`() { - assertFalse(SampleRateUtil.isValidProfilesSampleRate(Double.NEGATIVE_INFINITY)) + assertFalse(SampleRateUtils.isValidProfilesSampleRate(Double.NEGATIVE_INFINITY)) } @Test fun `accepts null profiles sample rate`() { - assertTrue(SampleRateUtil.isValidProfilesSampleRate(null)) + assertTrue(SampleRateUtils.isValidProfilesSampleRate(null)) } } From 3804a5c5c4a256e4b9a68c1bdce97fdb139c54d6 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Wed, 12 Oct 2022 09:09:49 +0000 Subject: [PATCH 07/32] Format code --- .../android/okhttp/SentryOkHttpInterceptor.kt | 1 - .../java/io/sentry/util/HttpHeadersUtils.java | 15 +++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 6c58fa3890..a6154d640e 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -13,7 +13,6 @@ import io.sentry.TypeCheckHint.OKHTTP_REQUEST import io.sentry.TypeCheckHint.OKHTTP_RESPONSE import io.sentry.exception.ExceptionMechanismException import io.sentry.protocol.Mechanism -import io.sentry.protocol.Message import io.sentry.util.HttpHeadersUtils import okhttp3.Headers import okhttp3.Interceptor diff --git a/sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java b/sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java index abb7519f82..fd0bd375d7 100644 --- a/sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java +++ b/sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java @@ -1,18 +1,17 @@ package io.sentry.util; -import org.jetbrains.annotations.ApiStatus; -import org.jetbrains.annotations.NotNull; - import java.util.Arrays; import java.util.List; import java.util.Locale; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; @ApiStatus.Internal public final class HttpHeadersUtils { - private static final List SENSITIVE_HEADERS = - Arrays.asList("X-FORWARDED-FOR", "AUTHORIZATION", "COOKIE"); + private static final List SENSITIVE_HEADERS = + Arrays.asList("X-FORWARDED-FOR", "AUTHORIZATION", "COOKIE"); - public static boolean containsSensitiveHeader(final @NotNull String header) { - return SENSITIVE_HEADERS.contains(header.toUpperCase(Locale.ROOT)); - } + public static boolean containsSensitiveHeader(final @NotNull String header) { + return SENSITIVE_HEADERS.contains(header.toUpperCase(Locale.ROOT)); + } } From d970ff9a9fdc45f9346340b00c50ce349120719c Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 12 Oct 2022 11:19:13 +0200 Subject: [PATCH 08/32] move the propagation targets to utils --- .../android/okhttp/SentryOkHttpInterceptor.kt | 7 +++---- .../apollo3/SentryApollo3HttpInterceptor.kt | 4 ++-- .../io/sentry/openfeign/SentryFeignClient.java | 4 ++-- .../SentrySpanClientHttpRequestInterceptor.java | 4 ++-- .../tracing/SentrySpanClientWebRequestFilter.java | 4 ++-- .../PropagationTargetsUtils.java} | 12 ++++++------ .../java/io/sentry/TracePropagationTargetsTest.kt | 15 ++++++++------- 7 files changed, 25 insertions(+), 25 deletions(-) rename sentry/src/main/java/io/sentry/{TracePropagationTargets.java => util/PropagationTargetsUtils.java} (70%) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 6c58fa3890..be5c1d7213 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -8,12 +8,11 @@ import io.sentry.IHub import io.sentry.ISpan import io.sentry.SentryEvent import io.sentry.SpanStatus -import io.sentry.TracePropagationTargets +import io.sentry.util.PropagationTargetsUtils import io.sentry.TypeCheckHint.OKHTTP_REQUEST import io.sentry.TypeCheckHint.OKHTTP_RESPONSE import io.sentry.exception.ExceptionMechanismException import io.sentry.protocol.Mechanism -import io.sentry.protocol.Message import io.sentry.util.HttpHeadersUtils import okhttp3.Headers import okhttp3.Interceptor @@ -48,7 +47,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) @@ -139,7 +138,7 @@ class SentryOkHttpInterceptor( requestUrl = requestUrl.replace("#$urlFragment", "") } - if (!captureFailedRequests || !TracePropagationTargets.contain(failedRequestsTargets, requestUrl) || !containsStatusCode(response.code)) { + if (!captureFailedRequests || !PropagationTargetsUtils.contain(failedRequestsTargets, requestUrl) || !containsStatusCode(response.code)) { return } diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index 98f34e0c0b..d63989f043 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -15,7 +15,7 @@ import io.sentry.IHub import io.sentry.ISpan import io.sentry.SentryLevel import io.sentry.SpanStatus -import io.sentry.TracePropagationTargets +import io.sentry.util.PropagationTargetsUtils import io.sentry.TypeCheckHint class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IHub = HubAdapter.getInstance(), private val beforeSpan: BeforeSpanCallback? = null) : @@ -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)) diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index 9a418c9c43..ffeec03570 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -13,7 +13,7 @@ import io.sentry.ISpan; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; -import io.sentry.TracePropagationTargets; +import io.sentry.util.PropagationTargetsUtils; import io.sentry.util.Objects; import java.io.IOException; import java.util.ArrayList; @@ -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 requestBaggageHeader = request.headers().get(BaggageHeader.BAGGAGE_HEADER); diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index 0186670eec..650c8b46e7 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -12,7 +12,7 @@ import io.sentry.ISpan; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; -import io.sentry.TracePropagationTargets; +import io.sentry.util.PropagationTargetsUtils; import io.sentry.util.Objects; import java.io.IOException; import org.jetbrains.annotations.NotNull; @@ -49,7 +49,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - if (TracePropagationTargets.contain( + if (PropagationTargetsUtils.contain( hub.getOptions().getTracePropagationTargets(), request.getURI())) { request.getHeaders().add(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); @Nullable diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java index d570c16bb7..959b73aa3b 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java @@ -11,7 +11,7 @@ import io.sentry.ISpan; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; -import io.sentry.TracePropagationTargets; +import io.sentry.util.PropagationTargetsUtils; import io.sentry.util.Objects; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -45,7 +45,7 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { final ClientRequest.Builder requestBuilder = ClientRequest.from(request); - if (TracePropagationTargets.contain( + if (PropagationTargetsUtils.contain( hub.getOptions().getTracePropagationTargets(), request.url())) { requestBuilder.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); diff --git a/sentry/src/main/java/io/sentry/TracePropagationTargets.java b/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java similarity index 70% rename from sentry/src/main/java/io/sentry/TracePropagationTargets.java rename to sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java index 0047d2bd2c..067fc6e3ac 100644 --- a/sentry/src/main/java/io/sentry/TracePropagationTargets.java +++ b/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java @@ -1,16 +1,16 @@ -package io.sentry; +package io.sentry.util; import java.net.URI; import java.util.List; -import org.jetbrains.annotations.NotNull; -// TODO: rename class to something more generic +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; /** - * Checks if an URL matches the list of origins to which `sentry-trace` header should be sent in - * HTTP integrations. + * Checks if an URL matches the list of origins. */ -public final class TracePropagationTargets { +@ApiStatus.Internal +public final class PropagationTargetsUtils { public static boolean contain(final @NotNull List origins, final @NotNull String url) { if (origins.isEmpty()) { diff --git a/sentry/src/test/java/io/sentry/TracePropagationTargetsTest.kt b/sentry/src/test/java/io/sentry/TracePropagationTargetsTest.kt index 60eb178723..96f9d13ce7 100644 --- a/sentry/src/test/java/io/sentry/TracePropagationTargetsTest.kt +++ b/sentry/src/test/java/io/sentry/TracePropagationTargetsTest.kt @@ -1,5 +1,6 @@ package io.sentry +import io.sentry.util.PropagationTargetsUtils import kotlin.test.Test import kotlin.test.assertFalse import kotlin.test.assertTrue @@ -9,21 +10,21 @@ class TracePropagationTargetsTest { @Test fun `origins contain the url when it contains one of the defined origins`() { val origins = listOf("localhost", "^(http|https)://api\\..*$") - assertTrue(TracePropagationTargets.contain(origins, "http://localhost:8080/foo")) - assertTrue(TracePropagationTargets.contain(origins, "http://xxx.localhost:8080/foo")) + assertTrue(PropagationTargetsUtils.contain(origins, "http://localhost:8080/foo")) + assertTrue(PropagationTargetsUtils.contain(origins, "http://xxx.localhost:8080/foo")) } @Test fun `origins contain the url when it matches regex`() { val origins = listOf("localhost", "^(http|https)://api\\..*$") - assertTrue(TracePropagationTargets.contain(origins, "http://api.foo.bar:8080/foo")) - assertTrue(TracePropagationTargets.contain(origins, "https://api.foo.bar:8080/foo")) - assertFalse(TracePropagationTargets.contain(origins, "ftp://api.foo.bar:8080/foo")) - assertTrue(TracePropagationTargets.contain(origins, "http://api.localhost:8080/foo")) + assertTrue(PropagationTargetsUtils.contain(origins, "http://api.foo.bar:8080/foo")) + assertTrue(PropagationTargetsUtils.contain(origins, "https://api.foo.bar:8080/foo")) + assertFalse(PropagationTargetsUtils.contain(origins, "ftp://api.foo.bar:8080/foo")) + assertTrue(PropagationTargetsUtils.contain(origins, "http://api.localhost:8080/foo")) } @Test fun `when no origins are defined, returns false for every url`() { - assertFalse(TracePropagationTargets.contain(emptyList(), "http://some.api.com/")) + assertFalse(PropagationTargetsUtils.contain(emptyList(), "http://some.api.com/")) } } From bee45105b9bb3cbc223af2095d8f9cc0c1aa6871 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Wed, 12 Oct 2022 09:22:21 +0000 Subject: [PATCH 09/32] Format code --- .../java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt | 2 +- .../java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt | 2 +- .../src/main/java/io/sentry/openfeign/SentryFeignClient.java | 2 +- .../tracing/SentrySpanClientHttpRequestInterceptor.java | 2 +- .../spring/tracing/SentrySpanClientWebRequestFilter.java | 2 +- .../main/java/io/sentry/util/PropagationTargetsUtils.java | 5 +---- 6 files changed, 6 insertions(+), 9 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index be5c1d7213..59cecd8172 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -8,12 +8,12 @@ import io.sentry.IHub import io.sentry.ISpan import io.sentry.SentryEvent import io.sentry.SpanStatus -import io.sentry.util.PropagationTargetsUtils import io.sentry.TypeCheckHint.OKHTTP_REQUEST import io.sentry.TypeCheckHint.OKHTTP_RESPONSE import io.sentry.exception.ExceptionMechanismException import io.sentry.protocol.Mechanism import io.sentry.util.HttpHeadersUtils +import io.sentry.util.PropagationTargetsUtils import okhttp3.Headers import okhttp3.Interceptor import okhttp3.Request diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index d63989f043..e04d1a4071 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/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.util.PropagationTargetsUtils 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 { diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index ffeec03570..01c43c739b 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/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.util.PropagationTargetsUtils; import io.sentry.util.Objects; +import io.sentry.util.PropagationTargetsUtils; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index 650c8b46e7..0c5562c760 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -12,8 +12,8 @@ import io.sentry.ISpan; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; -import io.sentry.util.PropagationTargetsUtils; import io.sentry.util.Objects; +import io.sentry.util.PropagationTargetsUtils; import java.io.IOException; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java index 959b73aa3b..dfa9810d7d 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java @@ -11,8 +11,8 @@ import io.sentry.ISpan; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; -import io.sentry.util.PropagationTargetsUtils; import io.sentry.util.Objects; +import io.sentry.util.PropagationTargetsUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.springframework.web.reactive.function.client.ClientRequest; diff --git a/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java b/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java index 067fc6e3ac..4388d68d6a 100644 --- a/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java +++ b/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java @@ -2,13 +2,10 @@ import java.net.URI; import java.util.List; - import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; -/** - * Checks if an URL matches the list of origins. - */ +/** Checks if an URL matches the list of origins. */ @ApiStatus.Internal public final class PropagationTargetsUtils { From 1541330b39f668af0440ed1f8092b4120be8c696 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 12 Oct 2022 11:37:31 +0200 Subject: [PATCH 10/32] ref --- .../android/okhttp/SentryHttpClientError.kt | 4 --- .../android/okhttp/SentryOkHttpInterceptor.kt | 12 +++++---- .../sentry/android/okhttp/StatusCodeRange.kt | 12 --------- .../apollo3/SentryApollo3HttpInterceptor.kt | 2 +- .../sentry/openfeign/SentryFeignClient.java | 2 +- .../sentry/spring/SentryRequestResolver.java | 5 ++-- ...entrySpanClientHttpRequestInterceptor.java | 2 +- .../SentrySpanClientWebRequestFilter.java | 2 +- .../spring/webflux/SentryRequestResolver.java | 4 +-- .../java/io/sentry/HttpStatusCodeRange.java | 25 +++++++++++++++++++ .../exception/SentryEnvelopeException.java | 6 +++++ .../exception/SentryHttpClientException.java | 14 +++++++++++ .../{HttpHeadersUtils.java => HttpUtils.java} | 2 +- .../sentry/util/PropagationTargetsUtils.java | 5 +--- 14 files changed, 62 insertions(+), 35 deletions(-) delete mode 100644 sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt delete mode 100644 sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt create mode 100644 sentry/src/main/java/io/sentry/HttpStatusCodeRange.java create mode 100644 sentry/src/main/java/io/sentry/exception/SentryHttpClientException.java rename sentry/src/main/java/io/sentry/util/{HttpHeadersUtils.java => HttpUtils.java} (92%) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt deleted file mode 100644 index a34e03230a..0000000000 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryHttpClientError.kt +++ /dev/null @@ -1,4 +0,0 @@ -package io.sentry.android.okhttp - -// @ApiStatus.Internal -class SentryHttpClientError(override val message: String) : RuntimeException(message) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index be5c1d7213..02d5850e83 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -3,17 +3,19 @@ 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.util.PropagationTargetsUtils 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.HttpHeadersUtils +import io.sentry.util.HttpUtils +import io.sentry.util.PropagationTargetsUtils import okhttp3.Headers import okhttp3.Interceptor import okhttp3.Request @@ -25,7 +27,7 @@ class SentryOkHttpInterceptor( private val beforeSpan: BeforeSpanCallback? = null, // TODO: should this be under the options or here? also define the names private val captureFailedRequests: Boolean = false, - private val failedRequestStatusCode: List = listOf(StatusCodeRange(500, 599)), + private val failedRequestStatusCode: List = listOf(HttpStatusCodeRange(500, 599)), private val failedRequestsTargets: List = listOf(".*") ) : Interceptor { @@ -145,7 +147,7 @@ class SentryOkHttpInterceptor( val mechanism = Mechanism().apply { type = "SentryOkHttpInterceptor" } - val exception = SentryHttpClientError("Event was captured because the request status code was ${response.code}") + 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) @@ -216,7 +218,7 @@ class SentryOkHttpInterceptor( val name = requestHeaders.name(i) // header is only sent if isn't sensitive - if (HttpHeadersUtils.containsSensitiveHeader(name)) { + if (HttpUtils.containsSensitiveHeader(name)) { continue } diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt deleted file mode 100644 index 5a2aa46452..0000000000 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/StatusCodeRange.kt +++ /dev/null @@ -1,12 +0,0 @@ -package io.sentry.android.okhttp - -// import org.jetbrains.annotations.ApiStatus - -// @ApiStatus.Internal -class StatusCodeRange(private val min: Int, private val max: Int) { - constructor(statusCode: Int) : this(statusCode, statusCode) - - fun isInRange(statusCode: Int): Boolean { - return statusCode in min..max - } -} diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index d63989f043..e04d1a4071 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/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.util.PropagationTargetsUtils 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 { diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index ffeec03570..01c43c739b 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/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.util.PropagationTargetsUtils; import io.sentry.util.Objects; +import io.sentry.util.PropagationTargetsUtils; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java index da9ac214dd..18aa9bcded 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestResolver.java @@ -3,7 +3,7 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.IHub; import io.sentry.protocol.Request; -import io.sentry.util.HttpHeadersUtils; +import io.sentry.util.HttpUtils; import io.sentry.util.Objects; import java.util.Collections; import java.util.Enumeration; @@ -41,8 +41,7 @@ Map resolveHeadersMap(final @NotNull HttpServletRequest request) final Map headersMap = new HashMap<>(); for (String headerName : Collections.list(request.getHeaderNames())) { // do not copy personal information identifiable headers - if (hub.getOptions().isSendDefaultPii() - || !HttpHeadersUtils.containsSensitiveHeader(headerName)) { + if (hub.getOptions().isSendDefaultPii() || !HttpUtils.containsSensitiveHeader(headerName)) { headersMap.put(headerName, toString(request.getHeaders(headerName))); } } diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index 650c8b46e7..0c5562c760 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -12,8 +12,8 @@ import io.sentry.ISpan; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; -import io.sentry.util.PropagationTargetsUtils; import io.sentry.util.Objects; +import io.sentry.util.PropagationTargetsUtils; import java.io.IOException; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java index 959b73aa3b..dfa9810d7d 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java @@ -11,8 +11,8 @@ import io.sentry.ISpan; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; -import io.sentry.util.PropagationTargetsUtils; import io.sentry.util.Objects; +import io.sentry.util.PropagationTargetsUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.springframework.web.reactive.function.client.ClientRequest; diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java index 1d3f18eb00..5e18c5cea3 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java +++ b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryRequestResolver.java @@ -3,7 +3,7 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.IHub; import io.sentry.protocol.Request; -import io.sentry.util.HttpHeadersUtils; +import io.sentry.util.HttpUtils; import io.sentry.util.Objects; import java.util.HashMap; import java.util.List; @@ -42,7 +42,7 @@ Map resolveHeadersMap(final HttpHeaders request) { for (Map.Entry> entry : request.entrySet()) { // do not copy personal information identifiable headers if (hub.getOptions().isSendDefaultPii() - || !HttpHeadersUtils.containsSensitiveHeader(entry.getKey())) { + || !HttpUtils.containsSensitiveHeader(entry.getKey())) { headersMap.put(entry.getKey(), toString(entry.getValue())); } } diff --git a/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java new file mode 100644 index 0000000000..7505643a1f --- /dev/null +++ b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java @@ -0,0 +1,25 @@ +package io.sentry; + +/** + * The Http status code range. Example for a range: 400 to 499 500 to 599 400 to 599 + * + *

Example for a single status code 400 500 + */ +public final class HttpStatusCodeRange { + private final int min; + private final int max; + + public HttpStatusCodeRange(final int min, final int max) { + this.min = min; + this.max = max; + } + + public HttpStatusCodeRange(final int statusCode) { + this.min = statusCode; + this.max = statusCode; + } + + public boolean isInRange(final int statusCode) { + return statusCode >= min && statusCode <= max; + } +} diff --git a/sentry/src/main/java/io/sentry/exception/SentryEnvelopeException.java b/sentry/src/main/java/io/sentry/exception/SentryEnvelopeException.java index 60f0902bb9..8c4fc70050 100644 --- a/sentry/src/main/java/io/sentry/exception/SentryEnvelopeException.java +++ b/sentry/src/main/java/io/sentry/exception/SentryEnvelopeException.java @@ -1,7 +1,13 @@ package io.sentry.exception; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Nullable; +/** + * Thrown when there was an issue reading/creating the envelope. Examples: Failed to read the file. + * The file path does not exist. The file exceed the limit in size. + */ +@ApiStatus.Internal public final class SentryEnvelopeException extends Exception { private static final long serialVersionUID = -8307801916948173232L; diff --git a/sentry/src/main/java/io/sentry/exception/SentryHttpClientException.java b/sentry/src/main/java/io/sentry/exception/SentryHttpClientException.java new file mode 100644 index 0000000000..f3435fc3b9 --- /dev/null +++ b/sentry/src/main/java/io/sentry/exception/SentryHttpClientException.java @@ -0,0 +1,14 @@ +package io.sentry.exception; + +import org.jetbrains.annotations.Nullable; + +/** + * Used for holding a HTTP client error, for example. An integration that does not throw when API + * returns 5xx. + */ +public class SentryHttpClientException extends Exception { + + public SentryHttpClientException(final @Nullable String message) { + super(message); + } +} diff --git a/sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java b/sentry/src/main/java/io/sentry/util/HttpUtils.java similarity index 92% rename from sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java rename to sentry/src/main/java/io/sentry/util/HttpUtils.java index fd0bd375d7..b937800ae4 100644 --- a/sentry/src/main/java/io/sentry/util/HttpHeadersUtils.java +++ b/sentry/src/main/java/io/sentry/util/HttpUtils.java @@ -7,7 +7,7 @@ import org.jetbrains.annotations.NotNull; @ApiStatus.Internal -public final class HttpHeadersUtils { +public final class HttpUtils { private static final List SENSITIVE_HEADERS = Arrays.asList("X-FORWARDED-FOR", "AUTHORIZATION", "COOKIE"); diff --git a/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java b/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java index 067fc6e3ac..4388d68d6a 100644 --- a/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java +++ b/sentry/src/main/java/io/sentry/util/PropagationTargetsUtils.java @@ -2,13 +2,10 @@ import java.net.URI; import java.util.List; - import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; -/** - * Checks if an URL matches the list of origins. - */ +/** Checks if an URL matches the list of origins. */ @ApiStatus.Internal public final class PropagationTargetsUtils { From 412bc0ed8fb8aa97715940b47e610ea421072cfd Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 12 Oct 2022 11:58:38 +0200 Subject: [PATCH 11/32] remove tags --- sentry-android-okhttp/build.gradle.kts | 1 - .../io/sentry/android/okhttp/SentryOkHttpInterceptor.kt | 8 ++++---- .../src/main/java/io/sentry/samples/android/GithubAPI.kt | 4 ++-- .../io/sentry/exception/SentryHttpClientException.java | 3 ++- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sentry-android-okhttp/build.gradle.kts b/sentry-android-okhttp/build.gradle.kts index d4d44ec366..3b74a72bb1 100644 --- a/sentry-android-okhttp/build.gradle.kts +++ b/sentry-android-okhttp/build.gradle.kts @@ -71,7 +71,6 @@ dependencies { compileOnly(Config.Libs.okhttpBom) compileOnly(Config.Libs.okhttp) -// compileOnly(Config.CompileOnly.jetbrainsAnnotations) implementation(kotlin(Config.kotlinStdLib, KotlinCompilerVersion.VERSION)) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 02d5850e83..dc410666be 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -156,9 +156,9 @@ class SentryOkHttpInterceptor( hint.set("response", response) // TODO: remove after fields indexed - val tags = mutableMapOf() - tags["status_code"] = response.code.toString() - tags["url"] = requestUrl +// val tags = mutableMapOf() +// tags["status_code"] = response.code.toString() +// tags["url"] = requestUrl val unknownRequestFields = mutableMapOf() @@ -190,7 +190,7 @@ class SentryOkHttpInterceptor( } } - event.tags = tags +// event.tags = tags event.request = sentryRequest event.contexts.setResponse(sentryResponse) diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt index 39e8539b77..84a5f1ecf8 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt @@ -1,7 +1,7 @@ package io.sentry.samples.android +import io.sentry.HttpStatusCodeRange import io.sentry.android.okhttp.SentryOkHttpInterceptor -import io.sentry.android.okhttp.StatusCodeRange import okhttp3.OkHttpClient import retrofit2.Retrofit import retrofit2.converter.gson.GsonConverterFactory @@ -13,7 +13,7 @@ object GithubAPI { captureFailedRequests = true, // TODO: 200 just for testing failedRequestStatusCode = listOf( - StatusCodeRange(200, 599) + HttpStatusCodeRange(200, 599) ) ) ).build() diff --git a/sentry/src/main/java/io/sentry/exception/SentryHttpClientException.java b/sentry/src/main/java/io/sentry/exception/SentryHttpClientException.java index f3435fc3b9..c734539d8d 100644 --- a/sentry/src/main/java/io/sentry/exception/SentryHttpClientException.java +++ b/sentry/src/main/java/io/sentry/exception/SentryHttpClientException.java @@ -6,7 +6,8 @@ * Used for holding a HTTP client error, for example. An integration that does not throw when API * returns 5xx. */ -public class SentryHttpClientException extends Exception { +public final class SentryHttpClientException extends Exception { + private static final long serialVersionUID = 1L; public SentryHttpClientException(final @Nullable String message) { super(message); From 9136b37c69c98facc999cb05288e87ca4260343e Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 12 Oct 2022 15:40:46 +0200 Subject: [PATCH 12/32] fix --- .../io/sentry/android/okhttp/SentryOkHttpInterceptor.kt | 8 +++++--- .../main/java/io/sentry/samples/android/GitHubService.kt | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index dc410666be..78bddc8d7b 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -31,6 +31,7 @@ class SentryOkHttpInterceptor( private val failedRequestsTargets: List = listOf(".*") ) : Interceptor { + constructor() : this(HubAdapter.getInstance()) constructor(hub: IHub) : this(hub, null) constructor(beforeSpan: BeforeSpanCallback) : this(HubAdapter.getInstance(), beforeSpan) @@ -66,6 +67,8 @@ class SentryOkHttpInterceptor( 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 @@ -119,8 +122,6 @@ class SentryOkHttpInterceptor( } } - // TODO: ignore exceptions of the type UnknownHostException - 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. @@ -172,7 +173,8 @@ class SentryOkHttpInterceptor( fragment = urlFragment request.body?.contentLength().ifHasValidLength { - // TODO: should be mapped in relay and added to the protocol + // TODO: should be mapped in relay and added to the protocol, right now + // relay isn't retaining unmapped fields unknownRequestFields["body_size"] = it } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt index 635f1c86da..e24592dfe8 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt @@ -17,4 +17,6 @@ interface GitHubService { class Repo { val full_name: String = "" +// TODO: throws json serialization error, because it should be a boolean +// val private: Int = 1 } From 2284f8841b48dc8a6958e37b8ae1a4993abc3578 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 12 Oct 2022 15:43:56 +0200 Subject: [PATCH 13/32] fix api --- .../api/sentry-android-okhttp.api | 4 +- sentry/api/sentry.api | 65 +++++++++++++++++-- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/sentry-android-okhttp/api/sentry-android-okhttp.api b/sentry-android-okhttp/api/sentry-android-okhttp.api index 1dc2bbfcab..7a472922c1 100644 --- a/sentry-android-okhttp/api/sentry-android-okhttp.api +++ b/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 ()V public fun (Lio/sentry/IHub;)V - public fun (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;)V - public synthetic fun (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;ZLjava/util/List;Ljava/util/List;)V + public synthetic fun (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;ZLjava/util/List;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun (Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;)V public fun intercept (Lokhttp3/Interceptor$Chain;)Lokhttp3/Response; } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 70d4d0c11f..e811a1ed7c 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -260,6 +260,12 @@ public final class io/sentry/Hint { public static fun withAttachments (Ljava/util/List;)Lio/sentry/Hint; } +public final class io/sentry/HttpStatusCodeRange { + public fun (I)V + public fun (II)V + public fun isInRange (I)Z +} + public final class io/sentry/Hub : io/sentry/IHub { public fun (Lio/sentry/SentryOptions;)V public fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V @@ -1814,12 +1820,6 @@ public final class io/sentry/TraceContext$JsonKeys { public fun ()V } -public final class io/sentry/TracePropagationTargets { - public fun ()V - public static fun contain (Ljava/util/List;Ljava/lang/String;)Z - public static fun contain (Ljava/util/List;Ljava/net/URI;)Z -} - public final class io/sentry/TracesSamplingDecision { public fun (Ljava/lang/Boolean;)V public fun (Ljava/lang/Boolean;Ljava/lang/Double;)V @@ -2082,6 +2082,10 @@ public final class io/sentry/exception/SentryEnvelopeException : java/lang/Excep public fun (Ljava/lang/String;)V } +public final class io/sentry/exception/SentryHttpClientException : java/lang/Exception { + public fun (Ljava/lang/String;)V +} + public abstract interface class io/sentry/hints/ApplyScopeData { } @@ -2251,6 +2255,7 @@ public final class io/sentry/protocol/Contexts : java/util/concurrent/Concurrent public fun getDevice ()Lio/sentry/protocol/Device; public fun getGpu ()Lio/sentry/protocol/Gpu; public fun getOperatingSystem ()Lio/sentry/protocol/OperatingSystem; + public fun getResponse ()Lio/sentry/protocol/Response; public fun getRuntime ()Lio/sentry/protocol/SentryRuntime; public fun getTrace ()Lio/sentry/SpanContext; public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V @@ -2259,6 +2264,7 @@ public final class io/sentry/protocol/Contexts : java/util/concurrent/Concurrent public fun setDevice (Lio/sentry/protocol/Device;)V public fun setGpu (Lio/sentry/protocol/Gpu;)V public fun setOperatingSystem (Lio/sentry/protocol/OperatingSystem;)V + public fun setResponse (Lio/sentry/protocol/Response;)V public fun setRuntime (Lio/sentry/protocol/SentryRuntime;)V public fun setTrace (Lio/sentry/SpanContext;)V } @@ -2637,6 +2643,7 @@ public final class io/sentry/protocol/Request : io/sentry/JsonSerializable, io/s public fun getCookies ()Ljava/lang/String; public fun getData ()Ljava/lang/Object; public fun getEnvs ()Ljava/util/Map; + public fun getFragment ()Ljava/lang/String; public fun getHeaders ()Ljava/util/Map; public fun getMethod ()Ljava/lang/String; public fun getOthers ()Ljava/util/Map; @@ -2647,6 +2654,7 @@ public final class io/sentry/protocol/Request : io/sentry/JsonSerializable, io/s public fun setCookies (Ljava/lang/String;)V public fun setData (Ljava/lang/Object;)V public fun setEnvs (Ljava/util/Map;)V + public fun setFragment (Ljava/lang/String;)V public fun setHeaders (Ljava/util/Map;)V public fun setMethod (Ljava/lang/String;)V public fun setOthers (Ljava/util/Map;)V @@ -2665,6 +2673,7 @@ public final class io/sentry/protocol/Request$JsonKeys { public static final field COOKIES Ljava/lang/String; public static final field DATA Ljava/lang/String; public static final field ENV Ljava/lang/String; + public static final field FRAGMENT Ljava/lang/String; public static final field HEADERS Ljava/lang/String; public static final field METHOD Ljava/lang/String; public static final field OTHER Ljava/lang/String; @@ -2673,6 +2682,37 @@ public final class io/sentry/protocol/Request$JsonKeys { public fun ()V } +public final class io/sentry/protocol/Response : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public static final field TYPE Ljava/lang/String; + public fun ()V + public fun (Lio/sentry/protocol/Response;)V + public fun getBodySize ()Ljava/lang/Long; + public fun getCookies ()Ljava/lang/String; + public fun getHeaders ()Ljava/util/Map; + public fun getStatusCode ()Ljava/lang/Integer; + public fun getUnknown ()Ljava/util/Map; + public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V + public fun setBodySize (Ljava/lang/Long;)V + public fun setCookies (Ljava/lang/String;)V + public fun setHeaders (Ljava/util/Map;)V + public fun setStatusCode (Ljava/lang/Integer;)V + public fun setUnknown (Ljava/util/Map;)V +} + +public final class io/sentry/protocol/Response$Deserializer : io/sentry/JsonDeserializer { + public fun ()V + public fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Lio/sentry/protocol/Response; + public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; +} + +public final class io/sentry/protocol/Response$JsonKeys { + public static final field BODY_SIZE Ljava/lang/String; + public static final field COOKIES Ljava/lang/String; + public static final field HEADERS Ljava/lang/String; + public static final field STATUS_CODE Ljava/lang/String; + public fun ()V +} + public final class io/sentry/protocol/SdkInfo : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public fun ()V public fun getSdkName ()Ljava/lang/String; @@ -3247,6 +3287,11 @@ public abstract interface class io/sentry/util/HintUtils$SentryNullableConsumer public abstract fun accept (Ljava/lang/Object;)V } +public final class io/sentry/util/HttpUtils { + public fun ()V + public static fun containsSensitiveHeader (Ljava/lang/String;)Z +} + public final class io/sentry/util/LogUtils { public fun ()V public static fun logNotInstanceOf (Ljava/lang/Class;Ljava/lang/Object;Lio/sentry/ILogger;)V @@ -3268,7 +3313,13 @@ public final class io/sentry/util/Platform { public static fun isJvm ()Z } -public final class io/sentry/util/SampleRateUtil { +public final class io/sentry/util/PropagationTargetsUtils { + public fun ()V + public static fun contain (Ljava/util/List;Ljava/lang/String;)Z + public static fun contain (Ljava/util/List;Ljava/net/URI;)Z +} + +public final class io/sentry/util/SampleRateUtils { public fun ()V public static fun isValidProfilesSampleRate (Ljava/lang/Double;)Z public static fun isValidSampleRate (Ljava/lang/Double;)Z From 6268a191caf2ef9fdf5cb5f45d2c5d58919e2476 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 12 Oct 2022 16:57:12 +0200 Subject: [PATCH 14/32] fix build --- .../android/okhttp/SentryOkHttpInterceptor.kt | 16 ++++++++++------ sentry/api/sentry.api | 2 ++ .../main/java/io/sentry/HttpStatusCodeRange.java | 3 +++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 78bddc8d7b..989c8f189b 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -25,9 +25,10 @@ import java.io.IOException class SentryOkHttpInterceptor( private val hub: IHub = HubAdapter.getInstance(), private val beforeSpan: BeforeSpanCallback? = null, - // TODO: should this be under the options or here? also define the names + // should this be under the options or here? also define the names private val captureFailedRequests: Boolean = false, - private val failedRequestStatusCode: List = listOf(HttpStatusCodeRange(500, 599)), + private val failedRequestStatusCode: List = listOf( + HttpStatusCodeRange(HttpStatusCodeRange.DEFAULT_MIN, HttpStatusCodeRange.DEFAULT_MAX)), private val failedRequestsTargets: List = listOf(".*") ) : Interceptor { @@ -141,14 +142,17 @@ class SentryOkHttpInterceptor( requestUrl = requestUrl.replace("#$urlFragment", "") } - if (!captureFailedRequests || !PropagationTargetsUtils.contain(failedRequestsTargets, requestUrl) || !containsStatusCode(response.code)) { + if (!captureFailedRequests || + !PropagationTargetsUtils.contain(failedRequestsTargets, requestUrl) || + !containsStatusCode(response.code)) { return } val mechanism = Mechanism().apply { type = "SentryOkHttpInterceptor" } - val exception = SentryHttpClientException("Event was captured because the request status code was ${response.code}") + 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) @@ -156,7 +160,7 @@ class SentryOkHttpInterceptor( hint.set("request", request) hint.set("response", response) - // TODO: remove after fields indexed + // remove after fields indexed // val tags = mutableMapOf() // tags["status_code"] = response.code.toString() // tags["url"] = requestUrl @@ -173,7 +177,7 @@ class SentryOkHttpInterceptor( fragment = urlFragment request.body?.contentLength().ifHasValidLength { - // TODO: should be mapped in relay and added to the protocol, right now + // should be mapped in relay and added to the protocol, right now // relay isn't retaining unmapped fields unknownRequestFields["body_size"] = it } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index e811a1ed7c..964138e0ae 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -261,6 +261,8 @@ public final class io/sentry/Hint { } public final class io/sentry/HttpStatusCodeRange { + public static final field DEFAULT_MAX I + public static final field DEFAULT_MIN I public fun (I)V public fun (II)V public fun isInRange (I)Z diff --git a/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java index 7505643a1f..56602fcc79 100644 --- a/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java +++ b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java @@ -6,6 +6,9 @@ *

Example for a single status code 400 500 */ public final class HttpStatusCodeRange { + public static final int DEFAULT_MIN = 500; + public static final int DEFAULT_MAX = 599; + private final int min; private final int max; From 566d70e8064fffb617a1ac1d0f6d7af35291eca0 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Wed, 12 Oct 2022 15:00:24 +0000 Subject: [PATCH 15/32] Format code --- .../io/sentry/android/okhttp/SentryOkHttpInterceptor.kt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 989c8f189b..dc9464124c 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -28,7 +28,8 @@ class SentryOkHttpInterceptor( // should this be under the options or here? also define the names private val captureFailedRequests: Boolean = false, private val failedRequestStatusCode: List = listOf( - HttpStatusCodeRange(HttpStatusCodeRange.DEFAULT_MIN, HttpStatusCodeRange.DEFAULT_MAX)), + HttpStatusCodeRange(HttpStatusCodeRange.DEFAULT_MIN, HttpStatusCodeRange.DEFAULT_MAX) + ), private val failedRequestsTargets: List = listOf(".*") ) : Interceptor { @@ -144,7 +145,8 @@ class SentryOkHttpInterceptor( if (!captureFailedRequests || !PropagationTargetsUtils.contain(failedRequestsTargets, requestUrl) || - !containsStatusCode(response.code)) { + !containsStatusCode(response.code) + ) { return } @@ -152,7 +154,8 @@ class SentryOkHttpInterceptor( type = "SentryOkHttpInterceptor" } val exception = SentryHttpClientException( - "Event was captured because the request status code was ${response.code}") + "Event was captured because the request status code was ${response.code}" + ) val mechanismException = ExceptionMechanismException(mechanism, exception, Thread.currentThread(), true) val event = SentryEvent(mechanismException) From 176e7efc06144efea1fba87d3a484c6385ab40cb Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 14 Oct 2022 08:43:18 +0200 Subject: [PATCH 16/32] fix --- .../io/sentry/android/okhttp/SentryOkHttpInterceptor.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 989c8f189b..d129c7a31a 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -27,9 +27,9 @@ class SentryOkHttpInterceptor( private val beforeSpan: BeforeSpanCallback? = null, // should this be under the options or here? also define the names private val captureFailedRequests: Boolean = false, - private val failedRequestStatusCode: List = listOf( + private val failedRequestStatusCodes: List = listOf( HttpStatusCodeRange(HttpStatusCodeRange.DEFAULT_MIN, HttpStatusCodeRange.DEFAULT_MAX)), - private val failedRequestsTargets: List = listOf(".*") + private val failedRequestTargets: List = listOf(".*") ) : Interceptor { constructor() : this(HubAdapter.getInstance()) @@ -143,7 +143,7 @@ class SentryOkHttpInterceptor( } if (!captureFailedRequests || - !PropagationTargetsUtils.contain(failedRequestsTargets, requestUrl) || + !PropagationTargetsUtils.contain(failedRequestTargets, requestUrl) || !containsStatusCode(response.code)) { return } @@ -204,7 +204,7 @@ class SentryOkHttpInterceptor( } private fun containsStatusCode(statusCode: Int): Boolean { - for (item in failedRequestStatusCode) { + for (item in failedRequestStatusCodes) { if (item.isInRange(statusCode)) { return true } From 67fc1c1173358571d8da7f3b0d94031aeb1400dd Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 14 Oct 2022 09:09:22 +0200 Subject: [PATCH 17/32] fix conflict --- .../android/okhttp/SentryOkHttpInterceptor.kt | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 666114598e..f117ba95b7 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -27,16 +27,10 @@ class SentryOkHttpInterceptor( private val beforeSpan: BeforeSpanCallback? = null, // should this be under the options or here? also define the names private val captureFailedRequests: Boolean = false, -<<<<<<< HEAD private val failedRequestStatusCodes: List = listOf( - HttpStatusCodeRange(HttpStatusCodeRange.DEFAULT_MIN, HttpStatusCodeRange.DEFAULT_MAX)), - private val failedRequestTargets: List = listOf(".*") -======= - private val failedRequestStatusCode: List = listOf( HttpStatusCodeRange(HttpStatusCodeRange.DEFAULT_MIN, HttpStatusCodeRange.DEFAULT_MAX) ), - private val failedRequestsTargets: List = listOf(".*") ->>>>>>> 9b5e6bb971e6b14f2e79f973be95f809430bb790 + private val failedRequestTargets: List = listOf(".*") ) : Interceptor { constructor() : this(HubAdapter.getInstance()) @@ -150,14 +144,9 @@ class SentryOkHttpInterceptor( } if (!captureFailedRequests || -<<<<<<< HEAD !PropagationTargetsUtils.contain(failedRequestTargets, requestUrl) || - !containsStatusCode(response.code)) { -======= - !PropagationTargetsUtils.contain(failedRequestsTargets, requestUrl) || !containsStatusCode(response.code) ) { ->>>>>>> 9b5e6bb971e6b14f2e79f973be95f809430bb790 return } From 0e27884b6ddbbdd0dbaf4d55f9d602a11255ddc0 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 14 Oct 2022 09:29:09 +0200 Subject: [PATCH 18/32] add body size to request field --- .../android/okhttp/SentryOkHttpInterceptor.kt | 6 +----- .../main/java/io/sentry/protocol/Request.java | 19 +++++++++++++++++++ .../java/io/sentry/protocol/Response.java | 16 ++++++++-------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index f117ba95b7..74d6a80729 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -168,8 +168,6 @@ class SentryOkHttpInterceptor( // tags["status_code"] = response.code.toString() // tags["url"] = requestUrl - val unknownRequestFields = mutableMapOf() - val sentryRequest = io.sentry.protocol.Request().apply { url = requestUrl // Cookie is only sent if isSendDefaultPii is enabled @@ -182,10 +180,8 @@ class SentryOkHttpInterceptor( request.body?.contentLength().ifHasValidLength { // should be mapped in relay and added to the protocol, right now // relay isn't retaining unmapped fields - unknownRequestFields["body_size"] = it + bodySize = it } - - unknown = unknownRequestFields.ifEmpty { null } } val sentryResponse = io.sentry.protocol.Response().apply { diff --git a/sentry/src/main/java/io/sentry/protocol/Request.java b/sentry/src/main/java/io/sentry/protocol/Request.java index 5a69a0a60d..b25ad49f9f 100644 --- a/sentry/src/main/java/io/sentry/protocol/Request.java +++ b/sentry/src/main/java/io/sentry/protocol/Request.java @@ -100,6 +100,9 @@ public final class Request implements JsonUnknown, JsonSerializable { */ private @Nullable Map env; + /** The body size in bytes */ + private @Nullable Long bodySize; + private @Nullable Map other; /** The fragment (anchor) of the request URL. */ @@ -121,6 +124,7 @@ public Request(final @NotNull Request request) { this.unknown = CollectionUtils.newConcurrentHashMap(request.unknown); this.data = request.data; this.fragment = request.fragment; + this.bodySize = request.bodySize; } public @Nullable String getUrl() { @@ -208,6 +212,14 @@ public void setFragment(final @Nullable String fragment) { this.fragment = fragment; } + public @Nullable Long getBodySize() { + return bodySize; + } + + public void setBodySize(final @Nullable Long bodySize) { + this.bodySize = bodySize; + } + public static final class JsonKeys { public static final String URL = "url"; public static final String METHOD = "method"; @@ -218,6 +230,7 @@ public static final class JsonKeys { public static final String ENV = "env"; public static final String OTHER = "other"; public static final String FRAGMENT = "fragment"; + public static final String BODY_SIZE = "body_size"; } @Override @@ -251,6 +264,9 @@ public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger) if (fragment != null) { writer.name(JsonKeys.FRAGMENT).value(logger, fragment); } + if (bodySize != null) { + writer.name(Response.JsonKeys.BODY_SIZE).value(logger, bodySize); + } if (unknown != null) { for (String key : unknown.keySet()) { Object value = unknown.get(key); @@ -309,6 +325,9 @@ public static final class Deserializer implements JsonDeserializer { case JsonKeys.FRAGMENT: request.fragment = reader.nextStringOrNull(); break; + case Response.JsonKeys.BODY_SIZE: + request.bodySize = reader.nextLongOrNull(); + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); diff --git a/sentry/src/main/java/io/sentry/protocol/Response.java b/sentry/src/main/java/io/sentry/protocol/Response.java index e8820c77ed..a0e09a75d7 100644 --- a/sentry/src/main/java/io/sentry/protocol/Response.java +++ b/sentry/src/main/java/io/sentry/protocol/Response.java @@ -46,8 +46,8 @@ public Response(final @NotNull Response response) { this.cookies = response.cookies; this.headers = CollectionUtils.newConcurrentHashMap(response.headers); this.unknown = CollectionUtils.newConcurrentHashMap(response.unknown); - this.setStatusCode(response.getStatusCode()); - this.setBodySize(response.getBodySize()); + this.statusCode = response.statusCode; + this.bodySize = response.bodySize; } public @Nullable String getCookies() { @@ -113,11 +113,11 @@ public void serialize(final @NotNull JsonObjectWriter writer, final @NotNull ILo if (headers != null) { writer.name(JsonKeys.HEADERS).value(logger, headers); } - if (getStatusCode() != null) { - writer.name(JsonKeys.STATUS_CODE).value(logger, getStatusCode()); + if (statusCode != null) { + writer.name(JsonKeys.STATUS_CODE).value(logger, statusCode); } - if (getBodySize() != null) { - writer.name(JsonKeys.BODY_SIZE).value(logger, getBodySize()); + if (bodySize != null) { + writer.name(JsonKeys.BODY_SIZE).value(logger, bodySize); } if (unknown != null) { @@ -152,10 +152,10 @@ public static final class Deserializer implements JsonDeserializer { } break; case JsonKeys.STATUS_CODE: - response.setStatusCode(reader.nextIntegerOrNull()); + response.statusCode = reader.nextIntegerOrNull(); break; case JsonKeys.BODY_SIZE: - response.setBodySize(reader.nextLongOrNull()); + response.bodySize = reader.nextLongOrNull(); break; default: if (unknown == null) { From a6211b8c99653c1ff5c08ae606f74b8b710f0254 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 14 Oct 2022 09:45:46 +0200 Subject: [PATCH 19/32] api --- sentry/api/sentry.api | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 385f4e86d0..c3f0e3ace8 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -2644,6 +2644,7 @@ public final class io/sentry/protocol/OperatingSystem$JsonKeys { public final class io/sentry/protocol/Request : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public fun ()V public fun (Lio/sentry/protocol/Request;)V + public fun getBodySize ()Ljava/lang/Long; public fun getCookies ()Ljava/lang/String; public fun getData ()Ljava/lang/Object; public fun getEnvs ()Ljava/util/Map; @@ -2655,6 +2656,7 @@ public final class io/sentry/protocol/Request : io/sentry/JsonSerializable, io/s public fun getUnknown ()Ljava/util/Map; public fun getUrl ()Ljava/lang/String; public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V + public fun setBodySize (Ljava/lang/Long;)V public fun setCookies (Ljava/lang/String;)V public fun setData (Ljava/lang/Object;)V public fun setEnvs (Ljava/util/Map;)V @@ -2674,6 +2676,7 @@ public final class io/sentry/protocol/Request$Deserializer : io/sentry/JsonDeser } public final class io/sentry/protocol/Request$JsonKeys { + public static final field BODY_SIZE Ljava/lang/String; public static final field COOKIES Ljava/lang/String; public static final field DATA Ljava/lang/String; public static final field ENV Ljava/lang/String; From 2f10f79a077f5c1281d9ffd957505a44b1cb9e97 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 14 Oct 2022 11:30:15 +0200 Subject: [PATCH 20/32] fix naming --- .../src/main/java/io/sentry/samples/android/GithubAPI.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt index 84a5f1ecf8..7bd50050b5 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt @@ -12,7 +12,7 @@ object GithubAPI { SentryOkHttpInterceptor( captureFailedRequests = true, // TODO: 200 just for testing - failedRequestStatusCode = listOf( + failedRequestStatusCodes = listOf( HttpStatusCodeRange(200, 599) ) ) From a26cde4781e8b4d6bc5e9e287458abb6f7ece589 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 14 Oct 2022 13:03:20 +0200 Subject: [PATCH 21/32] add tests --- .../core/ManifestMetadataReaderTest.kt | 25 ++++++++++ .../java/io/sentry/HttpStatusCodeRangeTest.kt | 42 +++++++++++++++++ .../protocol/ContextsSerializationTest.kt | 47 ++++++------------- .../java/io/sentry/protocol/ContextsTest.kt | 2 + .../sentry/protocol/GpuSerializationTest.kt | 44 ++++------------- .../protocol/RequestSerializationTest.kt | 42 +++++------------ .../java/io/sentry/protocol/RequestTest.kt | 10 +++- .../protocol/ResponseSerializationTest.kt | 38 +++++++++++++++ .../SentryBaseEventSerializationTest.kt | 38 ++++----------- .../io/sentry/protocol/SerializationUtils.kt | 38 +++++++++++++++ sentry/src/test/resources/json/contexts.json | 10 ++++ sentry/src/test/resources/json/request.json | 4 +- sentry/src/test/resources/json/response.json | 9 ++++ .../resources/json/sentry_base_event.json | 14 +++++- .../src/test/resources/json/sentry_event.json | 14 +++++- .../resources/json/sentry_transaction.json | 14 +++++- ...sentry_transaction_legacy_date_format.json | 14 +++++- 17 files changed, 273 insertions(+), 132 deletions(-) create mode 100644 sentry/src/test/java/io/sentry/HttpStatusCodeRangeTest.kt create mode 100644 sentry/src/test/java/io/sentry/protocol/ResponseSerializationTest.kt create mode 100644 sentry/src/test/java/io/sentry/protocol/SerializationUtils.kt create mode 100644 sentry/src/test/resources/json/response.json diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index 3e4f0b32f4..109edaa121 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/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) + } } diff --git a/sentry/src/test/java/io/sentry/HttpStatusCodeRangeTest.kt b/sentry/src/test/java/io/sentry/HttpStatusCodeRangeTest.kt new file mode 100644 index 0000000000..29dc0054de --- /dev/null +++ b/sentry/src/test/java/io/sentry/HttpStatusCodeRangeTest.kt @@ -0,0 +1,42 @@ +package io.sentry + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class HttpStatusCodeRangeTest { + @Test + fun `when single range is given and it is a match`() { + val range = HttpStatusCodeRange(500) + + assertTrue(range.isInRange(500)) + } + + @Test + fun `when single range is given and it is not a match`() { + val range = HttpStatusCodeRange(500) + + assertFalse(range.isInRange(400)) + } + + @Test + fun `when range is given and it is a match`() { + val range = HttpStatusCodeRange(500, 599) + + assertTrue(range.isInRange(501)) + } + + @Test + fun `when range is given and it is lower than min`() { + val range = HttpStatusCodeRange(500, 599) + + assertFalse(range.isInRange(499)) + } + + @Test + fun `when range is given and it is higher than max`() { + val range = HttpStatusCodeRange(500, 599) + + assertFalse(range.isInRange(600)) + } +} diff --git a/sentry/src/test/java/io/sentry/protocol/ContextsSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/ContextsSerializationTest.kt index c274324031..7b02b11a88 100644 --- a/sentry/src/test/java/io/sentry/protocol/ContextsSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/ContextsSerializationTest.kt @@ -4,14 +4,9 @@ import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever -import io.sentry.FileFromResources import io.sentry.ILogger -import io.sentry.JsonObjectReader import io.sentry.JsonObjectWriter -import io.sentry.JsonSerializable import org.junit.Test -import java.io.StringReader -import java.io.StringWriter import kotlin.test.assertEquals class ContextsSerializationTest { @@ -26,6 +21,7 @@ class ContextsSerializationTest { setOperatingSystem(OperatingSystemSerializationTest.Fixture().getSut()) setRuntime(SentryRuntimeSerializationTest.Fixture().getSut()) setGpu(GpuSerializationTest.Fixture().getSut()) + setResponse(ResponseSerializationTest.Fixture().getSut()) trace = SpanContextSerializationTest.Fixture().getSut() } } @@ -33,8 +29,9 @@ class ContextsSerializationTest { @Test fun serialize() { - val expected = sanitizedFile("json/contexts.json") - val actual = serialize(fixture.getSut()) + val expected = SerializationUtils.sanitizedFile("json/contexts.json") + val actual = SerializationUtils.serializeToString(fixture.getSut(), fixture.logger) + assertEquals(expected, actual) } @@ -54,9 +51,12 @@ class ContextsSerializationTest { @Test fun deserialize() { - val expectedJson = sanitizedFile("json/contexts.json") - val actual = deserialize(expectedJson) - val actualJson = serialize(actual) + val expectedJson = SerializationUtils.sanitizedFile("json/contexts.json") + val actual = SerializationUtils.deserializeJson( + expectedJson, Contexts.Deserializer(), fixture.logger + ) + val actualJson = SerializationUtils.serializeToString(actual, fixture.logger) + assertEquals(expectedJson, actualJson) } @@ -64,28 +64,11 @@ class ContextsSerializationTest { fun deserializeUnknownEntry() { val sut = fixture.getSut() sut["fixture-key"] = "fixture-value" - val serialized = serialize(sut) - val deserialized = deserialize(serialized) - assertEquals("fixture-value", deserialized["fixture-key"]) - } - - // Helper + val serialized = SerializationUtils.serializeToString(sut, fixture.logger) + val deserialized = SerializationUtils.deserializeJson( + serialized, Contexts.Deserializer(), fixture.logger + ) - private fun sanitizedFile(path: String): String { - return FileFromResources.invoke(path) - .replace(Regex("[\n\r]"), "") - .replace(" ", "") - } - - private fun serialize(jsonSerializable: JsonSerializable): String { - val wrt = StringWriter() - val jsonWrt = JsonObjectWriter(wrt, 100) - jsonSerializable.serialize(jsonWrt, fixture.logger) - return wrt.toString() - } - - private fun deserialize(json: String): Contexts { - val reader = JsonObjectReader(StringReader(json)) - return Contexts.Deserializer().deserialize(reader, fixture.logger) + assertEquals("fixture-value", deserialized["fixture-key"]) } } diff --git a/sentry/src/test/java/io/sentry/protocol/ContextsTest.kt b/sentry/src/test/java/io/sentry/protocol/ContextsTest.kt index e8e418fbd3..c1fb47b1c7 100644 --- a/sentry/src/test/java/io/sentry/protocol/ContextsTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/ContextsTest.kt @@ -17,6 +17,7 @@ class ContextsTest { contexts.setOperatingSystem(OperatingSystem()) contexts.setRuntime(SentryRuntime()) contexts.setGpu(Gpu()) + contexts.setResponse(Response()) contexts.trace = SpanContext("op") val clone = Contexts(contexts) @@ -30,6 +31,7 @@ class ContextsTest { assertNotSame(contexts.runtime, clone.runtime) assertNotSame(contexts.gpu, clone.gpu) assertNotSame(contexts.trace, clone.trace) + assertNotSame(contexts.response, clone.response) } @Test diff --git a/sentry/src/test/java/io/sentry/protocol/GpuSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/GpuSerializationTest.kt index e44181cd31..45383f9141 100644 --- a/sentry/src/test/java/io/sentry/protocol/GpuSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/GpuSerializationTest.kt @@ -1,14 +1,8 @@ package io.sentry.protocol import com.nhaarman.mockitokotlin2.mock -import io.sentry.FileFromResources import io.sentry.ILogger -import io.sentry.JsonObjectReader -import io.sentry.JsonObjectWriter -import io.sentry.JsonSerializable import org.junit.Test -import java.io.StringReader -import java.io.StringWriter import kotlin.test.assertEquals class GpuSerializationTest { @@ -32,40 +26,20 @@ class GpuSerializationTest { @Test fun serialize() { - val expected = sanitizedFile("json/gpu.json") - val actual = serializeToString(fixture.getSut()) + val expected = SerializationUtils.sanitizedFile("json/gpu.json") + val actual = SerializationUtils.serializeToString(fixture.getSut(), fixture.logger) + assertEquals(expected, actual) } @Test fun deserialize() { - val expectedJson = sanitizedFile("json/gpu.json") - val actual = deserializeBrowser(expectedJson) - val actualJson = serializeToString(actual) - assertEquals(expectedJson, actualJson) - } - - // Helper - - private fun sanitizedFile(path: String): String { - return FileFromResources.invoke(path) - .replace(Regex("[\n\r]"), "") - .replace(" ", "") - } + val expectedJson = SerializationUtils.sanitizedFile("json/gpu.json") + val actual = SerializationUtils.deserializeJson( + expectedJson, Gpu.Deserializer(), fixture.logger + ) + val actualJson = SerializationUtils.serializeToString(actual, fixture.logger) - private fun serializeToString(jsonSerializable: JsonSerializable): String { - return this.serializeToString { wrt -> jsonSerializable.serialize(wrt, fixture.logger) } - } - - private fun serializeToString(serialize: (JsonObjectWriter) -> Unit): String { - val wrt = StringWriter() - val jsonWrt = JsonObjectWriter(wrt, 100) - serialize(jsonWrt) - return wrt.toString() - } - - private fun deserializeBrowser(json: String): Gpu { - val reader = JsonObjectReader(StringReader(json)) - return Gpu.Deserializer().deserialize(reader, fixture.logger) + assertEquals(expectedJson, actualJson) } } diff --git a/sentry/src/test/java/io/sentry/protocol/RequestSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/RequestSerializationTest.kt index 6bdc4766ad..6f19c725ff 100644 --- a/sentry/src/test/java/io/sentry/protocol/RequestSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/RequestSerializationTest.kt @@ -1,14 +1,8 @@ package io.sentry.protocol import com.nhaarman.mockitokotlin2.mock -import io.sentry.FileFromResources import io.sentry.ILogger -import io.sentry.JsonObjectReader -import io.sentry.JsonObjectWriter -import io.sentry.JsonSerializable import org.junit.Test -import java.io.StringReader -import java.io.StringWriter import kotlin.test.assertEquals class RequestSerializationTest { @@ -33,42 +27,28 @@ class RequestSerializationTest { others = mapOf( "669ff1c1-517b-46dc-a889-131555364a56" to "89043294-f6e1-4e2e-b152-1fdf9b1102fc" ) + bodySize = 1000 + fragment = "fragment" } } private val fixture = Fixture() @Test fun serialize() { - val expected = sanitizedFile("json/request.json") - val actual = serialize(fixture.getSut()) + val expected = SerializationUtils.sanitizedFile("json/request.json") + val actual = SerializationUtils.serializeToString(fixture.getSut(), fixture.logger) + assertEquals(expected, actual) } @Test fun deserialize() { - val expectedJson = sanitizedFile("json/request.json") - val actual = deserialize(expectedJson) - val actualJson = serialize(actual) - assertEquals(expectedJson, actualJson) - } + val expectedJson = SerializationUtils.sanitizedFile("json/request.json") + val actual = SerializationUtils.deserializeJson( + expectedJson, Request.Deserializer(), fixture.logger + ) + val actualJson = SerializationUtils.serializeToString(actual, fixture.logger) - // Helper - - private fun sanitizedFile(path: String): String { - return FileFromResources.invoke(path) - .replace(Regex("[\n\r]"), "") - .replace(" ", "") - } - - private fun serialize(jsonSerializable: JsonSerializable): String { - val wrt = StringWriter() - val jsonWrt = JsonObjectWriter(wrt, 100) - jsonSerializable.serialize(jsonWrt, fixture.logger) - return wrt.toString() - } - - private fun deserialize(json: String): Request { - val reader = JsonObjectReader(StringReader(json)) - return Request.Deserializer().deserialize(reader, fixture.logger) + assertEquals(expectedJson, actualJson) } } diff --git a/sentry/src/test/java/io/sentry/protocol/RequestTest.kt b/sentry/src/test/java/io/sentry/protocol/RequestTest.kt index d7fa61d1c8..2acb3316d6 100644 --- a/sentry/src/test/java/io/sentry/protocol/RequestTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/RequestTest.kt @@ -33,6 +33,8 @@ class RequestTest { assertEquals("envs", clone.envs!!["envs"]) assertEquals("others", clone.others!!["others"]) assertEquals("unknown", clone.unknown!!["unknown"]) + assertEquals(1000, clone.bodySize) + assertEquals("fragment", clone.fragment) } @Test @@ -47,7 +49,9 @@ class RequestTest { request.others!!["others"] = "newOthers" request.others!!["anotherOne"] = "anotherOne" val newUnknown = mapOf(Pair("unknown", "newUnknown"), Pair("otherUnknown", "otherUnknown")) - request.setUnknown(newUnknown) + request.unknown = newUnknown + request.bodySize = 1001 + request.fragment = "fragment2" assertEquals("get", clone.method) assertEquals("http://localhost:8080", clone.url) @@ -58,6 +62,8 @@ class RequestTest { assertEquals(1, clone.others!!.size) assertEquals("unknown", clone.unknown!!["unknown"]) assertEquals(1, clone.unknown!!.size) + assertEquals(1000, clone.bodySize) + assertEquals("fragment", clone.fragment) } @Test @@ -111,6 +117,8 @@ class RequestTest { setOthers(others) val unknown = mapOf(Pair("unknown", "unknown")) setUnknown(unknown) + bodySize = 1000 + fragment = "fragment" } } } diff --git a/sentry/src/test/java/io/sentry/protocol/ResponseSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/ResponseSerializationTest.kt new file mode 100644 index 0000000000..ae09606b9e --- /dev/null +++ b/sentry/src/test/java/io/sentry/protocol/ResponseSerializationTest.kt @@ -0,0 +1,38 @@ +package io.sentry.protocol + +import com.nhaarman.mockitokotlin2.mock +import io.sentry.ILogger +import org.junit.Test +import kotlin.test.assertEquals + +class ResponseSerializationTest { + + class Fixture { + val logger = mock() + fun getSut() = Response().apply { + cookies = "PHPSESSID=298zf09hf012fh2;csrftoken=u32t4o3tb3gg43;_gat=1;" + headers = mapOf("content-type" to "text/html") + statusCode = 500 + bodySize = 1000 + unknown = mapOf("arbitrary_field" to "arbitrary") + } + } + private val fixture = Fixture() + + @Test + fun serialize() { + val expected = SerializationUtils.sanitizedFile("json/response.json") + val actual = SerializationUtils.serializeToString(fixture.getSut(), fixture.logger) + assertEquals(expected, actual) + } + + @Test + fun deserialize() { + val expectedJson = SerializationUtils.sanitizedFile("json/response.json") + val actual = SerializationUtils.deserializeJson( + expectedJson, Response.Deserializer(), fixture.logger + ) + val actualJson = SerializationUtils.serializeToString(actual, fixture.logger) + assertEquals(expectedJson, actualJson) + } +} diff --git a/sentry/src/test/java/io/sentry/protocol/SentryBaseEventSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/SentryBaseEventSerializationTest.kt index 9b270ca8dc..6b5c988f81 100644 --- a/sentry/src/test/java/io/sentry/protocol/SentryBaseEventSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/SentryBaseEventSerializationTest.kt @@ -1,7 +1,6 @@ package io.sentry.protocol import com.nhaarman.mockitokotlin2.mock -import io.sentry.FileFromResources import io.sentry.ILogger import io.sentry.JsonDeserializer import io.sentry.JsonObjectReader @@ -10,8 +9,6 @@ import io.sentry.JsonSerializable import io.sentry.SentryBaseEvent import io.sentry.vendor.gson.stream.JsonToken import org.junit.Test -import java.io.StringReader -import java.io.StringWriter import kotlin.test.assertEquals class SentryBaseEventSerializationTest { @@ -55,6 +52,7 @@ class SentryBaseEventSerializationTest { setGpu(GpuSerializationTest.Fixture().getSut()) setOperatingSystem(OperatingSystemSerializationTest.Fixture().getSut()) setRuntime(SentryRuntimeSerializationTest.Fixture().getSut()) + setResponse(ResponseSerializationTest.Fixture().getSut()) trace = SpanContextSerializationTest.Fixture().getSut() } sdk = SdkVersionSerializationTest.Fixture().getSut() @@ -79,37 +77,21 @@ class SentryBaseEventSerializationTest { @Test fun serialize() { - val expected = sanitizedFile("json/sentry_base_event.json") + val expected = SerializationUtils.sanitizedFile("json/sentry_base_event.json") val sut = Sut().apply { fixture.update(this) } - val actual = serialize(sut) + val actual = SerializationUtils.serializeToString(sut, fixture.logger) + assertEquals(expected, actual) } @Test fun deserialize() { - val expectedJson = sanitizedFile("json/sentry_base_event.json") - val actual = deserialize(expectedJson) - val actualJson = serialize(actual) - assertEquals(expectedJson, actualJson) - } + val expectedJson = SerializationUtils.sanitizedFile("json/sentry_base_event.json") + val actual = SerializationUtils.deserializeJson( + expectedJson, Sut.Deserializer(), fixture.logger + ) + val actualJson = SerializationUtils.serializeToString(actual, fixture.logger) - // Helper - - private fun sanitizedFile(path: String): String { - return FileFromResources.invoke(path) - .replace(Regex("[\n\r]"), "") - .replace(" ", "") - } - - private fun serialize(jsonSerializable: JsonSerializable): String { - val wrt = StringWriter() - val jsonWrt = JsonObjectWriter(wrt, 100) - jsonSerializable.serialize(jsonWrt, fixture.logger) - return wrt.toString() - } - - private fun deserialize(json: String): Sut { - val reader = JsonObjectReader(StringReader(json)) - return Sut.Deserializer().deserialize(reader, fixture.logger) + assertEquals(expectedJson, actualJson) } } diff --git a/sentry/src/test/java/io/sentry/protocol/SerializationUtils.kt b/sentry/src/test/java/io/sentry/protocol/SerializationUtils.kt new file mode 100644 index 0000000000..8e98199197 --- /dev/null +++ b/sentry/src/test/java/io/sentry/protocol/SerializationUtils.kt @@ -0,0 +1,38 @@ +package io.sentry.protocol + +import io.sentry.FileFromResources +import io.sentry.ILogger +import io.sentry.JsonDeserializer +import io.sentry.JsonObjectReader +import io.sentry.JsonObjectWriter +import io.sentry.JsonSerializable +import java.io.StringReader +import java.io.StringWriter + +class SerializationUtils { + // TODO: refactor every ser and deser tests to reuse this utils, a lot of boilerplate + + companion object { + fun deserializeJson(json: String, deserializer: JsonDeserializer, logger: ILogger): T { + val reader = JsonObjectReader(StringReader(json)) + return deserializer.deserialize(reader, logger) + } + + fun sanitizedFile(path: String): String { + return FileFromResources.invoke(path) + .replace(Regex("[\n\r]"), "") + .replace(" ", "") + } + + fun serializeToString(jsonSerializable: JsonSerializable, logger: ILogger): String { + return this.serializeToString { wrt -> jsonSerializable.serialize(wrt, logger) } + } + + private fun serializeToString(serialize: (JsonObjectWriter) -> Unit): String { + val wrt = StringWriter() + val jsonWrt = JsonObjectWriter(wrt, 100) + serialize(jsonWrt) + return wrt.toString() + } + } +} diff --git a/sentry/src/test/resources/json/contexts.json b/sentry/src/test/resources/json/contexts.json index bfc738aac7..24c46e4669 100644 --- a/sentry/src/test/resources/json/contexts.json +++ b/sentry/src/test/resources/json/contexts.json @@ -81,6 +81,16 @@ "kernel_version": "1df24aec-3a6f-49a9-8b50-69ae5f9dde08", "rooted": true }, + "response": + { + "cookies": "PHPSESSID=298zf09hf012fh2; csrftoken=u32t4o3tb3gg43; _gat=1;", + "headers": { + "content-type": "text/html" + }, + "status_code": 500, + "body_size": 1000, + "arbitrary_field": "arbitrary" + }, "runtime": { "name": "4ed019c4-9af9-43e0-830e-bfde9fe4461c", diff --git a/sentry/src/test/resources/json/request.json b/sentry/src/test/resources/json/request.json index d272463182..d0eb6d9735 100644 --- a/sentry/src/test/resources/json/request.json +++ b/sentry/src/test/resources/json/request.json @@ -18,5 +18,7 @@ "other": { "669ff1c1-517b-46dc-a889-131555364a56": "89043294-f6e1-4e2e-b152-1fdf9b1102fc" - } + }, + "fragment": "fragment", + "body_size": 1000 } diff --git a/sentry/src/test/resources/json/response.json b/sentry/src/test/resources/json/response.json new file mode 100644 index 0000000000..9faf2b4183 --- /dev/null +++ b/sentry/src/test/resources/json/response.json @@ -0,0 +1,9 @@ +{ + "cookies": "PHPSESSID=298zf09hf012fh2; csrftoken=u32t4o3tb3gg43; _gat=1;", + "headers": { + "content-type": "text/html" + }, + "status_code": 500, + "body_size": 1000, + "arbitrary_field": "arbitrary" +} diff --git a/sentry/src/test/resources/json/sentry_base_event.json b/sentry/src/test/resources/json/sentry_base_event.json index 6db965ebf9..a97c2d22f5 100644 --- a/sentry/src/test/resources/json/sentry_base_event.json +++ b/sentry/src/test/resources/json/sentry_base_event.json @@ -84,6 +84,16 @@ "kernel_version": "1df24aec-3a6f-49a9-8b50-69ae5f9dde08", "rooted": true }, + "response": + { + "cookies": "PHPSESSID=298zf09hf012fh2; csrftoken=u32t4o3tb3gg43; _gat=1;", + "headers": { + "content-type": "text/html" + }, + "status_code": 500, + "body_size": 1000, + "arbitrary_field": "arbitrary" + }, "runtime": { "name": "4ed019c4-9af9-43e0-830e-bfde9fe4461c", @@ -144,7 +154,9 @@ "other": { "669ff1c1-517b-46dc-a889-131555364a56": "89043294-f6e1-4e2e-b152-1fdf9b1102fc" - } + }, + "fragment": "fragment", + "body_size": 1000 }, "tags": { diff --git a/sentry/src/test/resources/json/sentry_event.json b/sentry/src/test/resources/json/sentry_event.json index cfebd9cfaa..becbbcdcd7 100644 --- a/sentry/src/test/resources/json/sentry_event.json +++ b/sentry/src/test/resources/json/sentry_event.json @@ -231,6 +231,16 @@ "kernel_version": "1df24aec-3a6f-49a9-8b50-69ae5f9dde08", "rooted": true }, + "response": + { + "cookies": "PHPSESSID=298zf09hf012fh2; csrftoken=u32t4o3tb3gg43; _gat=1;", + "headers": { + "content-type": "text/html" + }, + "status_code": 500, + "body_size": 1000, + "arbitrary_field": "arbitrary" + }, "runtime": { "name": "4ed019c4-9af9-43e0-830e-bfde9fe4461c", @@ -291,7 +301,9 @@ "other": { "669ff1c1-517b-46dc-a889-131555364a56": "89043294-f6e1-4e2e-b152-1fdf9b1102fc" - } + }, + "fragment": "fragment", + "body_size": 1000 }, "tags": { diff --git a/sentry/src/test/resources/json/sentry_transaction.json b/sentry/src/test/resources/json/sentry_transaction.json index 3c77b85504..4efa4fdd44 100644 --- a/sentry/src/test/resources/json/sentry_transaction.json +++ b/sentry/src/test/resources/json/sentry_transaction.json @@ -127,6 +127,16 @@ "kernel_version": "1df24aec-3a6f-49a9-8b50-69ae5f9dde08", "rooted": true }, + "response": + { + "cookies": "PHPSESSID=298zf09hf012fh2; csrftoken=u32t4o3tb3gg43; _gat=1;", + "headers": { + "content-type": "text/html" + }, + "status_code": 500, + "body_size": 1000, + "arbitrary_field": "arbitrary" + }, "runtime": { "name": "4ed019c4-9af9-43e0-830e-bfde9fe4461c", @@ -187,7 +197,9 @@ "other": { "669ff1c1-517b-46dc-a889-131555364a56": "89043294-f6e1-4e2e-b152-1fdf9b1102fc" - } + }, + "fragment": "fragment", + "body_size": 1000 }, "tags": { diff --git a/sentry/src/test/resources/json/sentry_transaction_legacy_date_format.json b/sentry/src/test/resources/json/sentry_transaction_legacy_date_format.json index 19efc05e72..1cc93370c6 100644 --- a/sentry/src/test/resources/json/sentry_transaction_legacy_date_format.json +++ b/sentry/src/test/resources/json/sentry_transaction_legacy_date_format.json @@ -127,6 +127,16 @@ "kernel_version": "1df24aec-3a6f-49a9-8b50-69ae5f9dde08", "rooted": true }, + "response": + { + "cookies": "PHPSESSID=298zf09hf012fh2; csrftoken=u32t4o3tb3gg43; _gat=1;", + "headers": { + "content-type": "text/html" + }, + "status_code": 500, + "body_size": 1000, + "arbitrary_field": "arbitrary" + }, "runtime": { "name": "4ed019c4-9af9-43e0-830e-bfde9fe4461c", @@ -187,7 +197,9 @@ "other": { "669ff1c1-517b-46dc-a889-131555364a56": "89043294-f6e1-4e2e-b152-1fdf9b1102fc" - } + }, + "fragment": "fragment", + "body_size": 1000 }, "tags": { From d452d9d6d44df9d1a47aede1250e32466a44cfe9 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 14 Oct 2022 14:59:43 +0200 Subject: [PATCH 22/32] add changelog --- CHANGELOG.md | 4 ++++ .../src/main/java/io/sentry/samples/android/GitHubService.kt | 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67a1a2cca8..29d43b1069 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Remove verbose FrameMetricsAggregator failure logging ([#2293](https://github.com/getsentry/sentry-java/pull/2293)) +### Features + +- HTTP Client errors for OkHttp ([#2287](https://github.com/getsentry/sentry-java/pull/2287)) + ## 6.5.0 ### Fixes diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt index e24592dfe8..5a73fcc3ec 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt @@ -10,13 +10,10 @@ interface GitHubService { @GET("users/{user}/repos") fun listRepos(@Path("user") user: String): Call> - // TODO: @GET("users/{user}/repos/#test") throws 404 @GET("users/{user}/repos/") suspend fun listReposAsync(@Path("user") user: String, @Query("per_page") perPage: Int): List } class Repo { val full_name: String = "" -// TODO: throws json serialization error, because it should be a boolean -// val private: Int = 1 } From d27d2c2e5185e85e5b13a81a99eecf1aa29a130f Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 17 Oct 2022 13:38:50 +0200 Subject: [PATCH 23/32] review --- .../android/okhttp/SentryOkHttpInterceptor.kt | 8 ++++---- .../main/java/io/sentry/HttpStatusCodeRange.java | 5 +++-- .../test/java/io/sentry/HttpStatusCodeRangeTest.kt | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 74d6a80729..3d6f5f879a 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -144,8 +144,8 @@ class SentryOkHttpInterceptor( } if (!captureFailedRequests || - !PropagationTargetsUtils.contain(failedRequestTargets, requestUrl) || - !containsStatusCode(response.code) + !containsStatusCode(response.code) || + !PropagationTargetsUtils.contain(failedRequestTargets, requestUrl) ) { return } @@ -160,8 +160,8 @@ class SentryOkHttpInterceptor( val event = SentryEvent(mechanismException) val hint = Hint() - hint.set("request", request) - hint.set("response", response) + hint.set(OKHTTP_REQUEST, request) + hint.set(OKHTTP_RESPONSE, response) // remove after fields indexed // val tags = mutableMapOf() diff --git a/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java index 56602fcc79..4193f7316d 100644 --- a/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java +++ b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java @@ -1,9 +1,10 @@ package io.sentry; /** - * The Http status code range. Example for a range: 400 to 499 500 to 599 400 to 599 + * The Http status code range. Example for a range: 400 to 499, 500 to 599, 400 to 599 + * The range is inclusive so the min and max is considered part of the range. * - *

Example for a single status code 400 500 + *

Example for a single status code 400, 500 */ public final class HttpStatusCodeRange { public static final int DEFAULT_MIN = 500; diff --git a/sentry/src/test/java/io/sentry/HttpStatusCodeRangeTest.kt b/sentry/src/test/java/io/sentry/HttpStatusCodeRangeTest.kt index 29dc0054de..e688aeff99 100644 --- a/sentry/src/test/java/io/sentry/HttpStatusCodeRangeTest.kt +++ b/sentry/src/test/java/io/sentry/HttpStatusCodeRangeTest.kt @@ -26,6 +26,20 @@ class HttpStatusCodeRangeTest { assertTrue(range.isInRange(501)) } + @Test + fun `when range is given and is a match with the lower bound`() { + val range = HttpStatusCodeRange(500, 599) + + assertTrue(range.isInRange(500)) + } + + @Test + fun `when range is given and is a match with the upper bound`() { + val range = HttpStatusCodeRange(500, 599) + + assertTrue(range.isInRange(599)) + } + @Test fun `when range is given and it is lower than min`() { val range = HttpStatusCodeRange(500, 599) From c06174f6b2887c1efba643be4779a63632d58f50 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Mon, 17 Oct 2022 11:44:12 +0000 Subject: [PATCH 24/32] Format code --- sentry/src/main/java/io/sentry/HttpStatusCodeRange.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java index 4193f7316d..690ab115c5 100644 --- a/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java +++ b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java @@ -1,8 +1,8 @@ package io.sentry; /** - * The Http status code range. Example for a range: 400 to 499, 500 to 599, 400 to 599 - * The range is inclusive so the min and max is considered part of the range. + * The Http status code range. Example for a range: 400 to 499, 500 to 599, 400 to 599 The range is + * inclusive so the min and max is considered part of the range. * *

Example for a single status code 400, 500 */ From 76168e0f9e2c64ba9f21670ff6ba2b404048439e Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 17 Oct 2022 13:52:24 +0200 Subject: [PATCH 25/32] fix --- sentry/src/main/java/io/sentry/HttpStatusCodeRange.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java index 4193f7316d..690ab115c5 100644 --- a/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java +++ b/sentry/src/main/java/io/sentry/HttpStatusCodeRange.java @@ -1,8 +1,8 @@ package io.sentry; /** - * The Http status code range. Example for a range: 400 to 499, 500 to 599, 400 to 599 - * The range is inclusive so the min and max is considered part of the range. + * The Http status code range. Example for a range: 400 to 499, 500 to 599, 400 to 599 The range is + * inclusive so the min and max is considered part of the range. * *

Example for a single status code 400, 500 */ From 7572c8af2dbc16bab99171ae44d06780acd38ded Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 18 Oct 2022 12:53:31 +0200 Subject: [PATCH 26/32] tests --- .../android/okhttp/SentryOkHttpInterceptor.kt | 9 - .../okhttp/SentryOkHttpInterceptorTest.kt | 249 ++++++++++++++++-- 2 files changed, 232 insertions(+), 26 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 3d6f5f879a..11b60cb0b5 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -25,7 +25,6 @@ import java.io.IOException class SentryOkHttpInterceptor( private val hub: IHub = HubAdapter.getInstance(), 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 = listOf( HttpStatusCodeRange(HttpStatusCodeRange.DEFAULT_MIN, HttpStatusCodeRange.DEFAULT_MAX) @@ -163,11 +162,6 @@ class SentryOkHttpInterceptor( hint.set(OKHTTP_REQUEST, request) hint.set(OKHTTP_RESPONSE, response) - // remove after fields indexed -// val tags = mutableMapOf() -// 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 @@ -178,8 +172,6 @@ class SentryOkHttpInterceptor( 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 } } @@ -195,7 +187,6 @@ class SentryOkHttpInterceptor( } } -// event.tags = tags event.request = sentryRequest event.contexts.setResponse(sentryResponse) diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index 850d88e590..879569693b 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -1,24 +1,29 @@ -@file:Suppress("MaxLineLength") package io.sentry.android.okhttp import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.check import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import io.sentry.BaggageHeader import io.sentry.Breadcrumb +import io.sentry.Hint +import io.sentry.HttpStatusCodeRange import io.sentry.IHub import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext +import io.sentry.TypeCheckHint +import io.sentry.exception.SentryHttpClientException import okhttp3.Interceptor import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient import okhttp3.Request +import okhttp3.RequestBody import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -36,7 +41,6 @@ class SentryOkHttpInterceptorTest { class Fixture { val hub = mock() - var interceptor = SentryOkHttpInterceptor(hub) val server = MockWebServer() lateinit var sentryTracer: SentryTracer lateinit var options: SentryOptions @@ -50,15 +54,23 @@ class SentryOkHttpInterceptorTest { beforeSpan: SentryOkHttpInterceptor.BeforeSpanCallback? = null, includeMockServerInTracePropagationTargets: Boolean = true, keepDefaultTracePropagationTargets: Boolean = false, + captureFailedRequests: Boolean = false, + failedRequestTargets: List = listOf(".*"), + failedRequestStatusCodes: List = listOf( + HttpStatusCodeRange( + HttpStatusCodeRange.DEFAULT_MIN, HttpStatusCodeRange.DEFAULT_MAX + ) + ), + sendDefaultPii: Boolean = false ): OkHttpClient { options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" - isTraceSampling = true if (includeMockServerInTracePropagationTargets) { setTracePropagationTargets(listOf(server.hostName)) } else if (!keepDefaultTracePropagationTargets) { setTracePropagationTargets(listOf("other-api")) } + isSendDefaultPii = sendDefaultPii } whenever(hub.options).thenReturn(options) @@ -70,31 +82,58 @@ class SentryOkHttpInterceptorTest { server.enqueue( MockResponse() .setBody(responseBody) + .addHeader("myResponseHeader", "myValue") .setSocketPolicy(socketPolicy) .setResponseCode(httpStatusCode) ) - if (beforeSpan != null) { - interceptor = SentryOkHttpInterceptor(hub, beforeSpan) - } + val interceptor = SentryOkHttpInterceptor( + hub, + beforeSpan, + captureFailedRequests = captureFailedRequests, + failedRequestTargets = failedRequestTargets, + failedRequestStatusCodes = failedRequestStatusCodes + ) return OkHttpClient.Builder().addInterceptor(interceptor).build() } } private val fixture = Fixture() - private val getRequest = { Request.Builder().get().url(fixture.server.url("/hello")).build() } - private val getRequestWithBaggagHeader = { Request.Builder().addHeader("baggage", "thirdPartyBaggage=someValue").addHeader("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue").get().url(fixture.server.url("/hello")).build() } - private val postRequest = { - Request.Builder().post( - "request-body" - .toRequestBody( - "text/plain" - .toMediaType() - ) + private fun getRequest(url: String = "/hello"): Request { + return Request.Builder() + .addHeader("myHeader", "myValue") + .get() + .url(fixture.server.url(url)) + .build() + } + + private val getRequestWithBaggagHeader = { + Request.Builder() + .addHeader("baggage", "thirdPartyBaggage=someValue") + .addHeader( + "baggage", + "secondThirdPartyBaggage=secondValue; " + + "property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue" + ) + .get() + .url(fixture.server.url("/hello")) + .build() + } + + private fun postRequest( + body: RequestBody = "request-body" + .toRequestBody( + "text/plain" + .toMediaType() + ) + ): Request { + return Request.Builder().post( + body ).url(fixture.server.url("/hello")).build() } + @SuppressWarnings("MaxLineLength") @Test fun `when there is an active span and server is listed in tracing origins, adds sentry trace headers to the request`() { val sut = fixture.getSut() @@ -104,6 +143,7 @@ class SentryOkHttpInterceptorTest { assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } + @SuppressWarnings("MaxLineLength") @Test fun `when there is an active span and tracing origins contains default regex, adds sentry trace headers to the request`() { val sut = fixture.getSut(keepDefaultTracePropagationTargets = true) @@ -114,6 +154,7 @@ class SentryOkHttpInterceptorTest { assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } + @SuppressWarnings("MaxLineLength") @Test fun `when there is an active span and server is not listed in tracing origins, does not add sentry trace headers to the request`() { val sut = fixture.getSut(includeMockServerInTracePropagationTargets = false) @@ -123,6 +164,7 @@ class SentryOkHttpInterceptorTest { assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } + @SuppressWarnings("MaxLineLength") @Test fun `when there is an active span and server tracing origins is empty, does not add sentry trace headers to the request`() { val sut = fixture.getSut() @@ -152,7 +194,14 @@ class SentryOkHttpInterceptorTest { val baggageHeaderValues = recorderRequest.headers.values(BaggageHeader.BAGGAGE_HEADER) assertEquals(baggageHeaderValues.size, 1) - assertTrue(baggageHeaderValues[0].startsWith("thirdPartyBaggage=someValue,secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue")) + assertTrue( + baggageHeaderValues[0].startsWith( + "thirdPartyBaggage=someValue," + + "secondThirdPartyBaggage=secondValue; " + + "property;propertyKey=propertyValue," + + "anotherThirdPartyBaggage=anotherValue" + ) + ) assertTrue(baggageHeaderValues[0].contains("sentry-public_key=key")) assertTrue(baggageHeaderValues[0].contains("sentry-transaction=name")) assertTrue(baggageHeaderValues[0].contains("sentry-trace_id")) @@ -211,12 +260,13 @@ class SentryOkHttpInterceptorTest { @SuppressWarnings("SwallowedException") @Test fun `adds breadcrumb when http calls results in exception`() { + val interceptor = SentryOkHttpInterceptor(fixture.hub) val chain = mock() whenever(chain.proceed(any())).thenThrow(IOException()) whenever(chain.request()).thenReturn(getRequest()) try { - fixture.interceptor.intercept(chain) + interceptor.intercept(chain) fail() } catch (e: IOException) { // ignore me @@ -287,4 +337,169 @@ class SentryOkHttpInterceptorTest { assertFalse(it) } } + + @Test + fun `captures an event if captureFailedRequests is enabled and within the range`() { + val sut = fixture.getSut( + captureFailedRequests = true, + httpStatusCode = 500 + ) + sut.newCall(getRequest()).execute() + + verify(fixture.hub).captureEvent(any(), any()) + } + + @Test + fun `does not capture an event if captureFailedRequests is disabled`() { + val sut = fixture.getSut( + httpStatusCode = 500 + ) + sut.newCall(getRequest()).execute() + + verify(fixture.hub, never()).captureEvent(any(), any()) + } + + @Test + fun `does not capture an event if captureFailedRequests is enabled and not within the range`() { + // default status code 201 + val sut = fixture.getSut( + captureFailedRequests = true + ) + sut.newCall(getRequest()).execute() + + verify(fixture.hub, never()).captureEvent(any(), any()) + } + + @Test + fun `does not capture an event if captureFailedRequests is enabled and not within the targets`() { + val sut = fixture.getSut( + captureFailedRequests = true, + httpStatusCode = 500, + failedRequestTargets = listOf("myapi.com") + ) + sut.newCall(getRequest()).execute() + + verify(fixture.hub, never()).captureEvent(any(), any()) + } + + @Test + fun `captures an error event with hints`() { + val sut = fixture.getSut( + captureFailedRequests = true, + httpStatusCode = 500 + ) + sut.newCall(getRequest()).execute() + + verify(fixture.hub).captureEvent( + any(), + check { + assertNotNull(it.get(TypeCheckHint.OKHTTP_REQUEST)) + assertNotNull(it.get(TypeCheckHint.OKHTTP_RESPONSE)) + } + ) + } + + @Test + fun `captures an error event with request and response fields set`() { + val statusCode = 500 + val sut = fixture.getSut( + captureFailedRequests = true, + httpStatusCode = statusCode, + responseBody = "fail" + ) + + val request = getRequest(url = "/hello?myQuery=myValue#myFragment") + val response = sut.newCall(request).execute() + + verify(fixture.hub).captureEvent( + check { + val sentryRequest = it.request!! + assertEquals("http://localhost:${fixture.server.port}/hello", sentryRequest.url) + assertEquals("myQuery=myValue", sentryRequest.queryString) + assertEquals("myFragment", sentryRequest.fragment) + assertEquals("GET", sentryRequest.method) + + // because of isSendDefaultPii + assertNull(sentryRequest.headers) + assertNull(sentryRequest.cookies) + + val sentryResponse = it.contexts.response!! + assertEquals(statusCode, sentryResponse.statusCode) + assertEquals(response.body!!.contentLength(), sentryResponse.bodySize) + + // because of isSendDefaultPii + assertNull(sentryRequest.headers) + assertNull(sentryRequest.cookies) + + assertTrue(it.throwable is SentryHttpClientException) + }, + any() + ) + } + + @Test + fun `captures an error event with request body size`() { + val sut = fixture.getSut( + captureFailedRequests = true, + httpStatusCode = 500, + ) + + val body = "fail" + .toRequestBody( + "text/plain" + .toMediaType() + ) + + sut.newCall(postRequest(body = body)).execute() + + verify(fixture.hub).captureEvent( + check { + val sentryRequest = it.request!! + assertEquals(body.contentLength(), sentryRequest.bodySize) + }, + any() + ) + } + + @Test + fun `captures an error event with headers`() { + val sut = fixture.getSut( + captureFailedRequests = true, + httpStatusCode = 500, + sendDefaultPii = true + ) + + sut.newCall(getRequest()).execute() + + verify(fixture.hub).captureEvent( + check { + val sentryRequest = it.request!! + assertEquals("myValue", sentryRequest.headers!!["myHeader"]) + + val sentryResponse = it.contexts.response!! + assertEquals("myValue", sentryResponse.headers!!["myResponseHeader"]) + }, + any() + ) + } + + @SuppressWarnings("SwallowedException") + @Test + fun `does not capture an error even if it throws`() { + val interceptor = SentryOkHttpInterceptor( + fixture.hub, + captureFailedRequests = true + ) + val chain = mock() + whenever(chain.proceed(any())).thenThrow(IOException()) + whenever(chain.request()).thenReturn(getRequest()) + + try { + interceptor.intercept(chain) + fail() + } catch (e: IOException) { + // ignore me + } + verify(fixture.hub, never()).captureEvent(any(), any()) + } } From 68408b5771c5835c406aec40a52771a49fdc3330 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 18 Oct 2022 13:09:41 +0200 Subject: [PATCH 27/32] fix test --- sentry/src/test/java/io/sentry/TracePropagationTargetsTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/test/java/io/sentry/TracePropagationTargetsTest.kt b/sentry/src/test/java/io/sentry/TracePropagationTargetsTest.kt index 8aaba10efe..6c857a67e9 100644 --- a/sentry/src/test/java/io/sentry/TracePropagationTargetsTest.kt +++ b/sentry/src/test/java/io/sentry/TracePropagationTargetsTest.kt @@ -30,6 +30,6 @@ class TracePropagationTargetsTest { @Test fun `ignores broken regex`() { - assertFalse(TracePropagationTargets.contain(listOf("AABB???"), "http://some.api.com/")) + assertFalse(PropagationTargetsUtils.contain(listOf("AABB???"), "http://some.api.com/")) } } From f0a629476cad966be6c065aecf8f392d75f64721 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 18 Oct 2022 13:17:13 +0200 Subject: [PATCH 28/32] fix todos --- .../io/sentry/samples/android/GitHubService.kt | 2 +- .../java/io/sentry/samples/android/GithubAPI.kt | 3 +-- .../samples/android/compose/ComposeActivity.kt | 14 ++------------ 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt index 5a73fcc3ec..36b6a5a495 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GitHubService.kt +++ b/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> - @GET("users/{user}/repos/") + @GET("users/{user}/repos") suspend fun listReposAsync(@Path("user") user: String, @Query("per_page") perPage: Int): List } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt index 7bd50050b5..aef2b64a0e 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt @@ -11,9 +11,8 @@ object GithubAPI { private val client = OkHttpClient.Builder().addInterceptor( SentryOkHttpInterceptor( captureFailedRequests = true, - // TODO: 200 just for testing failedRequestStatusCodes = listOf( - HttpStatusCodeRange(200, 599) + HttpStatusCodeRange(400, 599) ) ) ).build() diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt index 01b765bec7..74f7cdccfc 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/compose/ComposeActivity.kt @@ -90,12 +90,7 @@ fun Github( val scope = rememberCoroutineScope() LaunchedEffect(perPage) { - 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) - } + result = GithubAPI.service.listReposAsync(user.text, perPage).random().full_name } Column( @@ -113,12 +108,7 @@ fun Github( Button( onClick = { scope.launch { - try { - result = - GithubAPI.service.listReposAsync(user.text, perPage).random().full_name - } catch (e: Throwable) { - Sentry.captureException(e) - } + result = GithubAPI.service.listReposAsync(user.text, perPage).random().full_name } }, modifier = Modifier.padding(top = 32.dp) From a3ca145f879893164dfd23d30808ba5e282e81ba Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 19 Oct 2022 08:59:29 +0200 Subject: [PATCH 29/32] fixm essage --- .../java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 11b60cb0b5..58ed12e32b 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -153,7 +153,7 @@ class SentryOkHttpInterceptor( type = "SentryOkHttpInterceptor" } val exception = SentryHttpClientException( - "Event was captured because the request status code was ${response.code}" + "HTTP Client Error with status code: ${response.code}" ) val mechanismException = ExceptionMechanismException(mechanism, exception, Thread.currentThread(), true) val event = SentryEvent(mechanismException) From 6a4013b3b5444e03407e9103f0c6884cd5fd3c90 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 20 Oct 2022 10:14:43 +0200 Subject: [PATCH 30/32] review --- .../android/okhttp/SentryOkHttpInterceptor.kt | 14 +++++++ .../okhttp/SentryOkHttpInterceptorTest.kt | 4 +- .../java/io/sentry/protocol/Response.java | 4 +- .../io/sentry/protocol/SerializationUtils.kt | 38 +++++++++---------- 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 58ed12e32b..341f00a2ad 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -22,6 +22,20 @@ 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, diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index 879569693b..36e24c3ebf 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -108,7 +108,7 @@ class SentryOkHttpInterceptorTest { .build() } - private val getRequestWithBaggagHeader = { + private val getRequestWithBaggageHeader = { Request.Builder() .addHeader("baggage", "thirdPartyBaggage=someValue") .addHeader( @@ -187,7 +187,7 @@ class SentryOkHttpInterceptorTest { @Test fun `when there is an active span, existing baggage headers are merged with sentry baggage into single header`() { val sut = fixture.getSut() - sut.newCall(getRequestWithBaggagHeader()).execute() + sut.newCall(getRequestWithBaggageHeader()).execute() val recorderRequest = fixture.server.takeRequest() assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) diff --git a/sentry/src/main/java/io/sentry/protocol/Response.java b/sentry/src/main/java/io/sentry/protocol/Response.java index a0e09a75d7..d34b605d2d 100644 --- a/sentry/src/main/java/io/sentry/protocol/Response.java +++ b/sentry/src/main/java/io/sentry/protocol/Response.java @@ -66,8 +66,6 @@ public void setHeaders(final @Nullable Map headers) { this.headers = CollectionUtils.newConcurrentHashMap(headers); } - // region json - @Nullable @Override public Map getUnknown() { @@ -95,6 +93,8 @@ public void setBodySize(final @Nullable Long bodySize) { this.bodySize = bodySize; } + // region json + public static final class JsonKeys { public static final String COOKIES = "cookies"; public static final String HEADERS = "headers"; diff --git a/sentry/src/test/java/io/sentry/protocol/SerializationUtils.kt b/sentry/src/test/java/io/sentry/protocol/SerializationUtils.kt index 8e98199197..3545e7135f 100644 --- a/sentry/src/test/java/io/sentry/protocol/SerializationUtils.kt +++ b/sentry/src/test/java/io/sentry/protocol/SerializationUtils.kt @@ -9,30 +9,28 @@ import io.sentry.JsonSerializable import java.io.StringReader import java.io.StringWriter -class SerializationUtils { +object SerializationUtils { // TODO: refactor every ser and deser tests to reuse this utils, a lot of boilerplate - companion object { - fun deserializeJson(json: String, deserializer: JsonDeserializer, logger: ILogger): T { - val reader = JsonObjectReader(StringReader(json)) - return deserializer.deserialize(reader, logger) - } + fun deserializeJson(json: String, deserializer: JsonDeserializer, logger: ILogger): T { + val reader = JsonObjectReader(StringReader(json)) + return deserializer.deserialize(reader, logger) + } - fun sanitizedFile(path: String): String { - return FileFromResources.invoke(path) - .replace(Regex("[\n\r]"), "") - .replace(" ", "") - } + fun sanitizedFile(path: String): String { + return FileFromResources.invoke(path) + .replace(Regex("[\n\r]"), "") + .replace(" ", "") + } - fun serializeToString(jsonSerializable: JsonSerializable, logger: ILogger): String { - return this.serializeToString { wrt -> jsonSerializable.serialize(wrt, logger) } - } + fun serializeToString(jsonSerializable: JsonSerializable, logger: ILogger): String { + return this.serializeToString { wrt -> jsonSerializable.serialize(wrt, logger) } + } - private fun serializeToString(serialize: (JsonObjectWriter) -> Unit): String { - val wrt = StringWriter() - val jsonWrt = JsonObjectWriter(wrt, 100) - serialize(jsonWrt) - return wrt.toString() - } + private fun serializeToString(serialize: (JsonObjectWriter) -> Unit): String { + val wrt = StringWriter() + val jsonWrt = JsonObjectWriter(wrt, 100) + serialize(jsonWrt) + return wrt.toString() } } From 57d37767711ac49a731c11ab60772c953b502f45 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 20 Oct 2022 10:33:17 +0200 Subject: [PATCH 31/32] fix --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d21f76b4f..919afe8e34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,9 +14,6 @@ - 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)) - 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 From b79a165be139ac8f7dc2b56c598831b9d3d8601c Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 20 Oct 2022 11:43:16 +0200 Subject: [PATCH 32/32] review --- .../sentry/android/okhttp/SentryOkHttpInterceptor.kt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 341f00a2ad..d7f2d6d56e 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -138,6 +138,11 @@ 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 @@ -156,10 +161,8 @@ class SentryOkHttpInterceptor( requestUrl = requestUrl.replace("#$urlFragment", "") } - if (!captureFailedRequests || - !containsStatusCode(response.code) || - !PropagationTargetsUtils.contain(failedRequestTargets, requestUrl) - ) { + // return if its not a target match + if (!PropagationTargetsUtils.contain(failedRequestTargets, requestUrl)) { return }