-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
handle Retry-After header #151
base: master
Are you sure you want to change the base?
Changes from 3 commits
e82234d
29905f2
c29e53c
ab0ed64
16a757b
642a488
41714ab
c4c3397
ac1042f
173270d
d77de84
74e2e84
8312649
c3a0dc8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package io.github.rybalkinsd.kohttp.interceptors | |
import okhttp3.Interceptor | ||
import okhttp3.Response | ||
import java.net.SocketTimeoutException | ||
import kotlin.math.max | ||
|
||
/** | ||
* Retry Interceptor | ||
|
@@ -18,10 +19,10 @@ import java.net.SocketTimeoutException | |
* @author UDarya | ||
* */ | ||
class RetryInterceptor( | ||
private val failureThreshold: Int = 3, | ||
private val invocationTimeout: Long = 0, | ||
private val ratio: Int = 1, | ||
private val errorStatuses: List<Int> = listOf(503, 504) | ||
private val failureThreshold: Int = 3, | ||
private val invocationTimeout: Long = 0, | ||
private val ratio: Int = 1, | ||
private val errorStatuses: List<Int> = listOf(429, 503, 504) | ||
) : Interceptor { | ||
|
||
override fun intercept(chain: Interceptor.Chain): Response { | ||
|
@@ -34,7 +35,13 @@ class RetryInterceptor( | |
delay = performAndReturnDelay(delay) | ||
} | ||
val response = chain.proceed(request) | ||
if (!isRetry(response, attemptsCount)) return response | ||
|
||
val nextTime = calculateNextRetry(response, attemptsCount, delay, chain.readTimeoutMillis()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a room for improvement here.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest to have two stages here:
Probably, we need to modify |
||
if (nextTime != null) { | ||
delay = nextTime | ||
} else { | ||
return response | ||
} | ||
} catch (e: SocketTimeoutException) { | ||
if (attemptsCount >= failureThreshold) throw e | ||
} | ||
|
@@ -52,6 +59,14 @@ class RetryInterceptor( | |
|
||
private fun shouldDelay(attemptsCount: Int) = invocationTimeout > 0 && attemptsCount > 0 | ||
|
||
internal fun isRetry(response: Response, attemptsCount: Int): Boolean = | ||
attemptsCount < failureThreshold && response.code() in errorStatuses | ||
internal fun calculateNextRetry(response: Response, attemptsCount: Int, delay: Long, maxDelayTime: Int): Long? { | ||
val retryAfter = response.header("Retry-After")?.toLongOrNull() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably need an ext function to get header by name. BTW, did you check what is the metric of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
OK!
The unit of Retry-After value is seconds. But in my code it is millisecond. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no worries at all, that's why we're doing reviews :) |
||
if (retryAfter != null && retryAfter > maxDelayTime) return null | ||
|
||
return if (attemptsCount < failureThreshold && response.code() in errorStatuses) { | ||
max(retryAfter ?: 0, delay) | ||
} else { | ||
null | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid formatting change