Skip to content

Commit

Permalink
KTOR-5466 Connect timeout is not respected when using the HttpRequest…
Browse files Browse the repository at this point in the history
…Retry plugin (#3418)
  • Loading branch information
rsinukov authored Feb 24, 2023
1 parent c9327e7 commit 32c1888
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ package io.ktor.client.plugins

import io.ktor.client.*
import io.ktor.client.call.*
import io.ktor.client.network.sockets.*
import io.ktor.client.request.*
import io.ktor.client.statement.*
import io.ktor.client.utils.*
import io.ktor.events.*
import io.ktor.http.*
import io.ktor.util.*
Expand Down Expand Up @@ -167,14 +169,16 @@ public class HttpRequestRetry internal constructor(configuration: Configuration)
/**
* Enables retrying a request if an exception is thrown during the [HttpSend] phase
* and specifies the number of retries.
* By default, [HttpRequestTimeoutException] is not retried. Set [retryOnTimeout] to `true` to retry on timeout.
* By default, [HttpRequestTimeoutException], [ConnectTimeoutException] and [SocketTimeoutException]
* are not retried.
* Set [retryOnTimeout] to `true` to retry on timeout.
* Note, that in this case, [HttpTimeout] plugin should be installed after [HttpRequestRetry].
*/
public fun retryOnException(maxRetries: Int = -1, retryOnTimeout: Boolean = false) {
retryOnExceptionIf(maxRetries) { _, cause ->
when {
cause.isTimeoutException() -> retryOnTimeout
cause is CancellationException -> false
cause is HttpRequestTimeoutException && retryOnTimeout -> true
else -> true
}
}
Expand Down Expand Up @@ -392,3 +396,10 @@ private val RetryDelayPerRequestAttributeKey =
AttributeKey<HttpRequestRetry.DelayContext.(Int) -> Long>(
"RetryDelayPerRequestAttributeKey"
)

private fun Throwable.isTimeoutException(): Boolean {
val exception = unwrapCancellationException()
return exception is HttpRequestTimeoutException ||
exception is ConnectTimeoutException ||
exception is SocketTimeoutException
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,15 @@ import io.ktor.utils.io.*
/**
* If the exception contains cause that differs from [CancellationException] returns it otherwise returns itself.
*/
public actual fun Throwable.unwrapCancellationException(): Throwable = this
public actual fun Throwable.unwrapCancellationException(): Throwable {
var exception: Throwable? = this
while (exception is CancellationException) {
// If there is a cycle, we return the initial exception.
if (exception == exception.cause) {
return this
}
exception = exception.cause
}

return exception ?: this
}
Original file line number Diff line number Diff line change
Expand Up @@ -408,9 +408,8 @@ class HttpRequestRetryTest {
}

test { client ->
assertFailsWith<HttpRequestTimeoutException> {
client.get {}
}
val response = client.get {}
assertEquals(HttpStatusCode.OK, response.status)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ public class RoutingResolveContext(
trait: ArrayList<RoutingResolveResult.Success>
) {
val current = failedEvaluation ?: return
if ((current.quality < new.quality || failedEvaluationDepth < trait.size)
&& trait.all {
if ((current.quality < new.quality || failedEvaluationDepth < trait.size) &&
trait.all {
it.quality == RouteSelectorEvaluation.qualityTransparent ||
it.quality == RouteSelectorEvaluation.qualityConstant
}
Expand Down

0 comments on commit 32c1888

Please sign in to comment.