diff --git a/CHANGELOG.md b/CHANGELOG.md index da49e486cb1..ef9cc8e2795 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,17 @@ - Quartz `SentryJobListener` can send a monitor config derived from the trigger with check-ins, so Sentry creates or updates the monitor from code. Set the `sentry-upsert-monitor-config` job or trigger data key to `"true"` to send the schedule ([#6216](https://github.com/getsentry/sentry-java/pull/6216)) - Send a monitor config derived from Spring `@Scheduled` with `@SentryCheckIn` check-ins, so Sentry creates or updates the monitor from code. Set `@SentryCheckIn(upsertMonitorConfig = true)` to send the schedule ([#6215](https://github.com/getsentry/sentry-java/pull/6215)) +### Fixes + +- Stop marking an OkHttp call span as failed when a connection attempt to one of the host's addresses fails but OkHttp connects on another ([#6472](https://github.com/getsentry/sentry-java/pull/6472)) + +### Dependencies + +- Bump OkHttp from 4.9.2 to 5.5.1 ([#6472](https://github.com/getsentry/sentry-java/pull/6472)) + - `sentry-okhttp` and the Spring Boot integrations keep OkHttp as a `compileOnly` dependency, so your app keeps the OkHttp version it declares. OkHttp 4 is still supported. + - [changelog](https://github.com/square/okhttp/blob/master/CHANGELOG.md) + - [diff](https://github.com/square/okhttp/compare/parent-4.9.2...parent-5.5.1) + ## 8.63.0 ### Features diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c4a98a0f115..25681bf9137 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -27,7 +27,7 @@ log4j2 = "2.26.1" nopen = "1.0.1" # see https://www.jetbrains.com/help/kotlin-multiplatform-dev/compose-compatibility-and-versioning.html#kotlin-compatibility # see https://developer.android.com/jetpack/androidx/releases/compose-kotlin -okhttp = "4.9.2" +okhttp = "5.5.1" openfeature = "1.18.2" otel = "1.63.0" otelAlpha = "1.63.0-alpha" @@ -295,9 +295,11 @@ mockito-kotlin = { module = "org.mockito.kotlin:mockito-kotlin", version = "6.4. mockito-core = { module = "org.mockito:mockito-core", version = "5.24.0" } msgpack = { module = "org.msgpack:msgpack-core", version = "0.9.8" } okhttp-mockwebserver = { module = "com.squareup.okhttp3:mockwebserver", version.ref = "okhttp" } +# For tests that can't run on okhttp 5: apollo-runtime 2.x passes a null body to +# Response.Builder.body(), and the Android UI tests are pinned to leakcanary's okio 2, while okhttp +# 5 needs okio 3. +okhttp-mockwebserver4 = { module = "com.squareup.okhttp3:mockwebserver", version = "4.9.2" } okhttp-tls = { module = "com.squareup.okhttp3:okhttp-tls", version.ref = "okhttp" } -# apollo-runtime 5.x pulls in okhttp 5.x; mockwebserver 4.x is incompatible with that classpath -okhttp-mockwebserver5 = { module = "com.squareup.okhttp3:mockwebserver", version = "5.3.0" } okio = { module = "com.squareup.okio:okio", version = "1.13.0" } roboelectric = { module = "org.robolectric:robolectric", version = "4.15" } diff --git a/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts b/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts index d67f29648a4..7bbbe39e04f 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts +++ b/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts @@ -124,7 +124,7 @@ dependencies { androidTestImplementation(libs.awaitility3.kotlin) androidTestImplementation(libs.kotlin.test.junit) androidTestImplementation(libs.leakcanary.instrumentation) - androidTestImplementation(libs.okhttp.mockwebserver) + androidTestImplementation(libs.okhttp.mockwebserver4) } tasks.withType().configureEach { diff --git a/sentry-apollo-5/build.gradle.kts b/sentry-apollo-5/build.gradle.kts index 3078d833294..47db9da4984 100644 --- a/sentry-apollo-5/build.gradle.kts +++ b/sentry-apollo-5/build.gradle.kts @@ -42,7 +42,7 @@ dependencies { testImplementation(libs.kotlinx.coroutines.test) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.core) - testImplementation(libs.okhttp.mockwebserver5) + testImplementation(libs.okhttp.mockwebserver) } tasks.withType().configureEach { diff --git a/sentry-apollo/build.gradle.kts b/sentry-apollo/build.gradle.kts index 809b7f3a75c..e11b91a4c29 100644 --- a/sentry-apollo/build.gradle.kts +++ b/sentry-apollo/build.gradle.kts @@ -43,7 +43,7 @@ dependencies { testImplementation(libs.kotlinx.coroutines) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.core) - testImplementation(libs.okhttp.mockwebserver) + testImplementation(libs.okhttp.mockwebserver4) } tasks.withType().configureEach { diff --git a/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt b/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt index b43fcfdfb66..12836a732b0 100644 --- a/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt +++ b/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpEventListener.kt @@ -245,11 +245,9 @@ public open class SentryOkHttpEventListener( } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setProtocol(protocol?.name) - okHttpEvent.setError(ioe.message) - okHttpEvent.onEventFinish(CONNECT_EVENT) { - it.throwable = ioe - it.status = SpanStatus.INTERNAL_ERROR - } + // A failed route doesn't fail the call: OkHttp moves on to the next route, and OkHttp 5 races + // routes for Happy Eyeballs. If no route connects, callFailed() reports the error. + okHttpEvent.onEventFinish(CONNECT_EVENT) } override fun connectionAcquired(call: Call, connection: Connection) { diff --git a/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpEventListenerTest.kt index e4772145879..7f9915f13b1 100644 --- a/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpEventListenerTest.kt +++ b/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpEventListenerTest.kt @@ -1,5 +1,6 @@ package io.sentry.okhttp +import com.google.common.truth.Truth.assertThat import io.sentry.BaggageHeader import io.sentry.IScopes import io.sentry.SentryOptions @@ -9,6 +10,7 @@ import io.sentry.SpanDataConvention import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.mockServerRequestTimeoutMillis +import java.io.IOException import java.util.concurrent.TimeUnit import kotlin.test.Test import kotlin.test.assertEquals @@ -26,7 +28,6 @@ import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy import org.mockito.kotlin.any -import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.spy @@ -281,7 +282,7 @@ class SentryOkHttpEventListenerTest { fun `call span status is not overridden if not null`() { val mockListener = mock() lateinit var call: Call - whenever(mockListener.connectStart(any(), anyOrNull(), anyOrNull())).then { + whenever(mockListener.requestHeadersStart(any())).then { val okHttpEvent = SentryOkHttpEventListener.eventMap[call] val callSpan = okHttpEvent?.callSpan assertNotNull(callSpan) @@ -300,6 +301,48 @@ class SentryOkHttpEventListenerTest { assertEquals(SpanStatus.UNKNOWN, callSpan.status) } + @Test + fun `a failed route does not fail the call when the next route connects`() { + val sut = fixture.getSut() + val listener = fixture.sentryOkHttpEventListener + val call = sut.newCall(getRequest()) + val response = mock() + whenever(response.code).thenReturn(200) + whenever(response.protocol).thenReturn(Protocol.HTTP_1_1) + + listener.callStart(call) + listener.connectStart(call, mock(), mock()) + listener.connectFailed(call, mock(), mock(), null, IOException("Connection refused")) + listener.connectStart(call, mock(), mock()) + listener.connectEnd(call, mock(), mock(), Protocol.HTTP_1_1) + listener.responseHeadersStart(call) + listener.responseHeadersEnd(call, response) + val callSpan = SentryOkHttpEventListener.eventMap[call]!!.callSpan!! + listener.callEnd(call) + + assertThat(callSpan.status).isEqualTo(SpanStatus.OK) + assertThat(callSpan.throwable).isNull() + assertThat(callSpan.getData("error_message")).isNull() + } + + @Test + fun `a call that fails to connect on every route is marked as failed`() { + val sut = fixture.getSut() + val listener = fixture.sentryOkHttpEventListener + val call = sut.newCall(getRequest()) + val error = IOException("Connection refused") + + listener.callStart(call) + listener.connectStart(call, mock(), mock()) + listener.connectFailed(call, mock(), mock(), null, error) + val callSpan = SentryOkHttpEventListener.eventMap[call]!!.callSpan!! + listener.callFailed(call, error) + + assertThat(callSpan.status).isEqualTo(SpanStatus.INTERNAL_ERROR) + assertThat(callSpan.throwable).isSameInstanceAs(error) + assertThat(callSpan.getData("error_message")).isEqualTo("Connection refused") + } + private fun verifyDelegation( listener: SentryOkHttpEventListener, originalListener: EventListener,