Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KTOR-5466 Connect timeout is not respected when using the HttpRequestRetry plugin #3418

Merged
merged 1 commit into from
Feb 24, 2023
Merged
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 @@ -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