ktorio / ktor

Framework for quickly creating connected applications in Kotlin with minimal effort
https://ktor.io
Apache License 2.0
12.71k stars 1.04k forks source link

Retry on HttpCode #1285

Closed imanushin closed 1 year ago

imanushin commented 5 years ago

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:

e5l commented 5 years ago

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.

augustorsouza commented 4 years ago

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

e5l commented 4 years ago

Sure :)

imanushin commented 4 years ago

Hi @augustorsouza , I did not look on it.

augustorsouza commented 4 years ago

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 commented 4 years ago

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

ColinHebert commented 4 years ago

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

marekpietrasz commented 4 years ago

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.

dtanner commented 4 years ago

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.

oleg-larshin commented 4 years ago

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

ionutale commented 3 years ago

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

dtretyakov commented 2 years ago

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