Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ internal class CallConnectionUser(
connection.connectionListener.noNewExchanges(connection)
}

override fun doExtensiveHealthChecks(): Boolean = chain.request.method != "GET"
override fun doExtensiveHealthChecks(): Boolean = chain.request.body?.isOneShot() ?: false

override fun isCanceled(): Boolean = call.isCanceled()

Expand Down
40 changes: 33 additions & 7 deletions okhttp/src/jvmTest/kotlin/okhttp3/CallKotlinTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,22 @@
package okhttp3

import assertk.assertThat
import assertk.assertions.contains
import assertk.assertions.isEqualTo
import assertk.assertions.isInstanceOf
import assertk.assertions.isNotSameAs
import java.io.IOException
import java.net.Proxy
import java.security.cert.X509Certificate
import java.time.Duration
import kotlin.test.assertFails
import kotlin.test.assertFailsWith
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import mockwebserver3.SocketEffect.CloseSocket
import mockwebserver3.junit5.StartStop
import okhttp3.Headers.Companion.headersOf
import okhttp3.MediaType.Companion.toMediaType
import okhttp3.RequestBody.Companion.toRequestBody
import okhttp3.TestUtil.assertSuppressed
import okhttp3.internal.DoubleInetAddressDns
import okhttp3.internal.connection.RealConnection
Expand All @@ -43,6 +44,8 @@ import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.Timeout
import org.junit.jupiter.api.extension.RegisterExtension
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource
import org.junitpioneer.jupiter.RetryingTest

@Timeout(30)
Expand Down Expand Up @@ -183,13 +186,16 @@ class CallKotlinTest {
}
}

@Test
fun staleConnectionNotReusedForNonIdempotentRequest() {
/** OneShot requests cannot be retried, so it is important to healthcheck the connection first */
@ParameterizedTest
@ValueSource(booleans = [true, false])
fun staleConnectionNotReusedForOneShotRequests(isOneShot: Boolean) {
// Capture the connection so that we can later make it stale.
var connection: RealConnection? = null
client =
client
.newBuilder()
.retryOnConnectionFailure(false)
.addNetworkInterceptor(
Interceptor { chain ->
connection = chain.connection() as RealConnection
Expand Down Expand Up @@ -223,11 +229,31 @@ class CallKotlinTest {
val requestB =
Request(
url = server.url("/"),
body = "b".toRequestBody("text/plain".toMediaType()),
body =
object : RequestBody() {
override fun contentType(): MediaType? = "text/plain".toMediaType()

override fun writeTo(sink: BufferedSink) {
sink.write("b".toByteArray())
}

override fun isOneShot(): Boolean = isOneShot
},
)
val responseB = client.newCall(requestB).execute()
assertThat(responseB.body.string()).isEqualTo("b")
assertThat(server.takeRequest().exchangeIndex).isEqualTo(0)
if (isOneShot) {
// extensiveHealthcheck on the stale connection will create a new connection
val responseB = client.newCall(requestB).execute()
assertThat(responseB.body.string()).isEqualTo("b")
assertThat(server.takeRequest().exchangeIndex).isEqualTo(0)
} else {
// extensiveHealthcheck is skipped because this request could be retried
// However the exception is thrown because retryOnConnectionFailure=false
val throwable =
assertFails {
client.newCall(requestB).execute()
}
assertThat(throwable.message!!).contains("unexpected end of stream")
}
}

/** Confirm suppressed exceptions that occur while connecting are returned. */
Expand Down
Loading