diff --git a/CHANGELOG.md b/CHANGELOG.md index c0d6b1dbe1d..c4dfd709c81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,17 @@ - Promote the metrics capture API from experimental to stable ([#6425](https://github.com/getsentry/sentry-java/pull/6425)) +### 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.62.0 ### Improvements diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7cbe8e626c8..5009ad3e8e7 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -25,7 +25,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" @@ -292,8 +292,10 @@ 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" } -# 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" } +# 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" } 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 8f479470cac..2011a6e8b41 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts +++ b/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts @@ -122,7 +122,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 2d78dff8fa1..a7fb7a41a32 100644 --- a/sentry-apollo-5/build.gradle.kts +++ b/sentry-apollo-5/build.gradle.kts @@ -38,7 +38,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 72a183bb7b3..aa3baebc29c 100644 --- a/sentry-apollo/build.gradle.kts +++ b/sentry-apollo/build.gradle.kts @@ -39,7 +39,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,