Merge pull request #3142 from vector-im/feature/bma/global_retry

Generic hanlding of rate limit eror by the SDK
This commit is contained in:
Benoit Marty 2021-04-08 18:23:52 +02:00 committed by GitHub
commit 65b4b2915f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 18 additions and 9 deletions

View file

@ -18,6 +18,7 @@ Improvements 🙌:
- Room list improvements (paging) - Room list improvements (paging)
- Fix quick click action (#3127) - Fix quick click action (#3127)
- Get Event after a Push for a faster notification display in some conditions - Get Event after a Push for a faster notification display in some conditions
- Always try to retry Http requests in case of 429 (#1300)
Bugfix 🐛: Bugfix 🐛:
- Fix bad theme change for the MainActivity - Fix bad theme change for the MainActivity

View file

@ -19,6 +19,7 @@ package org.matrix.android.sdk.internal.network
import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
import org.matrix.android.sdk.api.failure.Failure import org.matrix.android.sdk.api.failure.Failure
import org.matrix.android.sdk.api.failure.MatrixError
import org.matrix.android.sdk.api.failure.getRetryDelay import org.matrix.android.sdk.api.failure.getRetryDelay
import org.matrix.android.sdk.api.failure.shouldBeRetried import org.matrix.android.sdk.api.failure.shouldBeRetried
import org.matrix.android.sdk.internal.network.ssl.CertUtil import org.matrix.android.sdk.internal.network.ssl.CertUtil
@ -28,22 +29,21 @@ import java.io.IOException
/** /**
* Execute a request from the requestBlock and handle some of the Exception it could generate * Execute a request from the requestBlock and handle some of the Exception it could generate
* Ref: https://github.com/matrix-org/matrix-js-sdk/blob/develop/src/scheduler.js#L138-L175
* *
* @param globalErrorReceiver will be use to notify error such as invalid token error. See [GlobalError] * @param globalErrorReceiver will be use to notify error such as invalid token error. See [GlobalError]
* @param canRetry if set to true, the request will be executed again in case of error, after a delay * @param canRetry if set to true, the request will be executed again in case of error, after a delay
* @param initialDelayBeforeRetry the first delay to wait before a request is retried. Will be doubled after each retry
* @param maxDelayBeforeRetry the max delay to wait before a retry * @param maxDelayBeforeRetry the max delay to wait before a retry
* @param maxRetriesCount the max number of retries * @param maxRetriesCount the max number of retries
* @param requestBlock a suspend lambda to perform the network request * @param requestBlock a suspend lambda to perform the network request
*/ */
internal suspend inline fun <DATA> executeRequest(globalErrorReceiver: GlobalErrorReceiver?, internal suspend inline fun <DATA> executeRequest(globalErrorReceiver: GlobalErrorReceiver?,
canRetry: Boolean = false, canRetry: Boolean = false,
initialDelayBeforeRetry: Long = 100L, maxDelayBeforeRetry: Long = 32_000L,
maxDelayBeforeRetry: Long = 10_000L, maxRetriesCount: Int = 4,
maxRetriesCount: Int = Int.MAX_VALUE,
noinline requestBlock: suspend () -> DATA): DATA { noinline requestBlock: suspend () -> DATA): DATA {
var currentRetryCount = 0 var currentRetryCount = 0
var currentDelay = initialDelayBeforeRetry var currentDelay = 1_000L
while (true) { while (true) {
try { try {
@ -72,9 +72,16 @@ internal suspend inline fun <DATA> executeRequest(globalErrorReceiver: GlobalErr
// } // }
?.also { unrecognizedCertificateException -> throw unrecognizedCertificateException } ?.also { unrecognizedCertificateException -> throw unrecognizedCertificateException }
if (canRetry && currentRetryCount++ < maxRetriesCount && exception.shouldBeRetried()) { currentRetryCount++
// In case of 429, ensure we wait enough
delay(currentDelay.coerceAtLeast(exception.getRetryDelay(0))) if (exception is Failure.ServerError
&& exception.httpCode == 429
&& exception.error.code == MatrixError.M_LIMIT_EXCEEDED
&& currentRetryCount < maxRetriesCount) {
// 429, we can retry
delay(exception.getRetryDelay(1_000))
} else if (canRetry && currentRetryCount < maxRetriesCount && exception.shouldBeRetried()) {
delay(currentDelay)
currentDelay = currentDelay.times(2L).coerceAtMost(maxDelayBeforeRetry) currentDelay = currentDelay.times(2L).coerceAtMost(maxDelayBeforeRetry)
// Try again (loop) // Try again (loop)
} else { } else {

View file

@ -91,7 +91,7 @@ internal class SendEventWorker(context: Context,
if (/*currentAttemptCount >= MAX_NUMBER_OF_RETRY_BEFORE_FAILING ||**/ !exception.shouldBeRetried()) { if (/*currentAttemptCount >= MAX_NUMBER_OF_RETRY_BEFORE_FAILING ||**/ !exception.shouldBeRetried()) {
Timber.e("## SendEvent: [${System.currentTimeMillis()}] Send event Failed cannot retry ${params.eventId} > ${exception.localizedMessage}") Timber.e("## SendEvent: [${System.currentTimeMillis()}] Send event Failed cannot retry ${params.eventId} > ${exception.localizedMessage}")
localEchoRepository.updateSendState(event.eventId, event.roomId, SendState.UNDELIVERED) localEchoRepository.updateSendState(event.eventId, event.roomId, SendState.UNDELIVERED)
return Result.success() Result.success()
} else { } else {
Timber.e("## SendEvent: [${System.currentTimeMillis()}] Send event Failed schedule retry ${params.eventId} > ${exception.localizedMessage}") Timber.e("## SendEvent: [${System.currentTimeMillis()}] Send event Failed schedule retry ${params.eventId} > ${exception.localizedMessage}")
Result.retry() Result.retry()

View file

@ -78,6 +78,7 @@ class UiAllScreensSanityTest {
// Last passing: // Last passing:
// 2020-11-09 // 2020-11-09
// 2020-12-16 After ViewBinding huge change // 2020-12-16 After ViewBinding huge change
// 2021-04-08 Testing 429 change
@Test @Test
fun allScreensTest() { fun allScreensTest() {
// Create an account // Create an account