From 9b5bc60fa9674523c2de1eadd31ab5d17f8b57ab Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 8 Apr 2021 09:54:51 +0200 Subject: [PATCH 1/4] Remove unused parameter and use same value than the JS SDK --- .../org/matrix/android/sdk/internal/network/Request.kt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt index e39bce6c67..170ce09149 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt @@ -28,22 +28,21 @@ import java.io.IOException /** * 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 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 maxRetriesCount the max number of retries * @param requestBlock a suspend lambda to perform the network request */ internal suspend inline fun executeRequest(globalErrorReceiver: GlobalErrorReceiver?, canRetry: Boolean = false, - initialDelayBeforeRetry: Long = 100L, - maxDelayBeforeRetry: Long = 10_000L, - maxRetriesCount: Int = Int.MAX_VALUE, + maxDelayBeforeRetry: Long = 32_000L, + maxRetriesCount: Int = 4, noinline requestBlock: suspend () -> DATA): DATA { var currentRetryCount = 0 - var currentDelay = initialDelayBeforeRetry + var currentDelay = 1_000L while (true) { try { From 8dead986a5c5d6d7febb5729e68623f4a9dbc764 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 8 Apr 2021 10:59:51 +0200 Subject: [PATCH 2/4] Always try to retry Http requests in case of 429 (#1300) --- CHANGES.md | 1 + .../matrix/android/sdk/internal/network/Request.kt | 14 +++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 86ba91c9df..1a684ee56f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,7 @@ Improvements 🙌: - Room list improvements (paging) - Fix quick click action (#3127) - 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 🐛: - Fix bad theme change for the MainActivity diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt index 170ce09149..0246bae024 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt @@ -19,6 +19,7 @@ package org.matrix.android.sdk.internal.network import kotlinx.coroutines.CancellationException import kotlinx.coroutines.delay 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.shouldBeRetried import org.matrix.android.sdk.internal.network.ssl.CertUtil @@ -71,9 +72,16 @@ internal suspend inline fun executeRequest(globalErrorReceiver: GlobalErr // } ?.also { unrecognizedCertificateException -> throw unrecognizedCertificateException } - if (canRetry && currentRetryCount++ < maxRetriesCount && exception.shouldBeRetried()) { - // In case of 429, ensure we wait enough - delay(currentDelay.coerceAtLeast(exception.getRetryDelay(0))) + currentRetryCount++ + + 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) // Try again (loop) } else { From c6bd37810457c93a6605a2d65d86579a227dd4bf Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 8 Apr 2021 12:44:28 +0200 Subject: [PATCH 3/4] Test is passing --- .../androidTest/java/im/vector/app/ui/UiAllScreensSanityTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/vector/src/androidTest/java/im/vector/app/ui/UiAllScreensSanityTest.kt b/vector/src/androidTest/java/im/vector/app/ui/UiAllScreensSanityTest.kt index 6f8056de13..53e1645f09 100644 --- a/vector/src/androidTest/java/im/vector/app/ui/UiAllScreensSanityTest.kt +++ b/vector/src/androidTest/java/im/vector/app/ui/UiAllScreensSanityTest.kt @@ -78,6 +78,7 @@ class UiAllScreensSanityTest { // Last passing: // 2020-11-09 // 2020-12-16 After ViewBinding huge change + // 2021-04-08 Testing 429 change @Test fun allScreensTest() { // Create an account From 7b1d313e8ee2e56406f8285aaf8ec4a766ff7788 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 8 Apr 2021 12:46:55 +0200 Subject: [PATCH 4/4] Small cleanup --- .../android/sdk/internal/session/room/send/SendEventWorker.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt index c1fc2fd9fe..d55dce57af 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/SendEventWorker.kt @@ -91,7 +91,7 @@ internal class SendEventWorker(context: Context, if (/*currentAttemptCount >= MAX_NUMBER_OF_RETRY_BEFORE_FAILING ||**/ !exception.shouldBeRetried()) { Timber.e("## SendEvent: [${System.currentTimeMillis()}] Send event Failed cannot retry ${params.eventId} > ${exception.localizedMessage}") localEchoRepository.updateSendState(event.eventId, event.roomId, SendState.UNDELIVERED) - return Result.success() + Result.success() } else { Timber.e("## SendEvent: [${System.currentTimeMillis()}] Send event Failed schedule retry ${params.eventId} > ${exception.localizedMessage}") Result.retry()