ktor icon indicating copy to clipboard operation
ktor copied to clipboard

Retry on HttpCode

Open imanushin opened this issue 4 years ago • 12 comments

Subsystem Client

Is your feature request related to a problem? Please describe. Sometimes external service returns 500/502 errors, however next call can be successful. In this case do connection retry after a small delay.

Describe the solution you'd like I'd like to have separate Ktor Client feature to automate these retries. For example, it can get handler like needRetry: Response -> Bool on input and configurable retry pauses and retry count.

Motivation to include to ktor Of course retries can be implemented above the call. However client can do them better, because:

  • It can preserve low-level resources (like data buffers, resolved headers, etc.)
  • It already does retry logic on 401 responses

imanushin avatar Aug 15 '19 09:08 imanushin

Hi @imanushin. Thanks for the report, it's good idea. The retry mechanism is implemented in the HttpSend feature, it looks easy to introduce the feature to use it.

e5l avatar Aug 15 '19 11:08 e5l

Hello guys, can I try to implement this one or are you guys looking at it already?

augustorsouza avatar Aug 28 '19 01:08 augustorsouza

Sure :)

e5l avatar Aug 28 '19 08:08 e5l

Hi @augustorsouza , I did not look on it.

imanushin avatar Aug 28 '19 10:08 imanushin

Hello guys, I think I have an alpha version and I would love feedbacks from you :)


package io.ktor.client.features

import io.ktor.client.HttpClient
import io.ktor.client.HttpClientConfig
import io.ktor.client.request.HttpRequestBuilder
import io.ktor.client.request.takeFrom
import io.ktor.client.response.HttpReceivePipeline
import io.ktor.client.response.HttpResponse
import io.ktor.util.AttributeKey

typealias NeedRetryHandler = suspend (response: HttpResponse) -> Boolean

class NeedRetry(
    private val retryHandlers: List<NeedRetryHandler>
) {
    class Config {
        internal val retryHandlers: MutableList<NeedRetryHandler> = mutableListOf()

        fun needRetryHandler(block: NeedRetryHandler) {
            retryHandlers += block
        }
    }

    companion object : HttpClientFeature<Config, NeedRetry> {
        override val key: AttributeKey<NeedRetry> = AttributeKey("NeedRetry")

        override fun prepare(block: Config.() -> Unit): NeedRetry {
            val config = Config().apply(block)

            config.retryHandlers.reversed()

            return NeedRetry(config.retryHandlers)
        }

        override fun install(feature: NeedRetry, scope: HttpClient) {
            scope.receivePipeline.intercept(HttpReceivePipeline.After) {
                try {
                    val isRetryNeeded = feature.retryHandlers.map { it(context.response) }.contains(true)

                    if (isRetryNeeded) {
                        context.client.execute(HttpRequestBuilder().takeFrom(context.request))
                    }

                    proceedWith(it)
                } catch (cause: Throwable) {
                    throw cause
                }
            }
        }
    }
}

fun HttpClientConfig<*>.NeedRetryHandler(block: NeedRetry.Config.() -> Unit) {
    install(NeedRetry, block)
}

Do you guys think I am going in the right direction?

I order to check this running this is a simple application which uses it:

package com.example

import io.ktor.application.Application
import io.ktor.client.HttpClient
import io.ktor.client.call.call
import io.ktor.client.engine.apache.Apache
import io.ktor.client.features.NeedRetryHandler
import io.ktor.client.features.logging.LogLevel
import io.ktor.client.features.logging.Logging
import kotlinx.coroutines.runBlocking

fun main(args: Array<String>): Unit = io.ktor.server.cio.EngineMain.main(args)

@Suppress("unused") // Referenced in application.conf
@kotlin.jvm.JvmOverloads
fun Application.module(testing: Boolean = false) {
    var i = 0
    runBlocking {
        val client = HttpClient(Apache) {
            followRedirects = true

            install(Logging) {
                level = LogLevel.INFO
            }

            NeedRetryHandler {
                needRetryHandler {
                    i += 1
                    i <= 4
                }
            }
        }

        client.call("https://httpbin.org/status/500")
    }

}


augustorsouza avatar Sep 01 '19 02:09 augustorsouza

The PR is waiting for a review :) after that we can solve this issue

augustorsouza avatar Oct 29 '19 15:10 augustorsouza

I wonder, will this support retry operations when requestTimeout get triggered?

ColinHebert avatar Oct 30 '19 10:10 ColinHebert

I wonder, will this support retry operations when requestTimeout get triggered?

I have tested it and looks like timeout could not be handled this way (tested with jetty). Do you know guys if there is a way to add a nice exception handling mechanism using features for timeouts or HostNotFound problems other than try/catch. I am struggling to use existing abstraction for that.

marekpietrasz avatar Nov 11 '19 16:11 marekpietrasz

Any thoughts on how you'd add a delay to the retries? The functions are not currently suspended, so wondering if there's a clean way to add that, since backoff in retries is a pretty common need.

dtanner avatar Nov 19 '19 21:11 dtanner

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

oleg-larshin avatar Aug 10 '20 15:08 oleg-larshin

has this feature been implemented? aswer here :https://github.com/ktorio/ktor/pull/1310#issuecomment-717754026

Regarding https://youtrack.jetbrains.com/issue/KTOR-1142, we're not going to introduce new features in the core. Please consider creating a separate repo for that feature

ionutale avatar Jul 27 '21 18:07 ionutale

It looks like it's implemented in dedicated plugin for Ktor 2.0: https://api.ktor.io/ktor-client/ktor-client-core/io.ktor.client.plugins/-http-request-retry/index.html

dtretyakov avatar Feb 10 '22 16:02 dtretyakov