Merge pull request #4995 from vector-im/feature/fre/fix_retry_delay_on_429

Do not auto-retry on 429 in case of too long retry delay

changelog.d/4995.removal

@@ -0,0 +1 @@
+429 are not automatically retried anymore in case of too long retry delay

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/failure/Extensions.kt

@@ -35,10 +35,15 @@ fun Throwable.isTokenError() =
                         error.code == MatrixError.M_MISSING_TOKEN ||
                         error.code == MatrixError.ORG_MATRIX_EXPIRED_ACCOUNT)
 
+fun Throwable.isLimitExceededError() =
+        this is Failure.ServerError &&
+                httpCode == 429 &&
+                error.code == MatrixError.M_LIMIT_EXCEEDED
+
 fun Throwable.shouldBeRetried(): Boolean {
     return this is Failure.NetworkConnection ||
             this is IOException ||
-            (this is Failure.ServerError && error.code == MatrixError.M_LIMIT_EXCEEDED)
+            this.isLimitExceededError()
 }
 
 /**

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/Request.kt

@@ -19,8 +19,9 @@ 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.GlobalError
 import org.matrix.android.sdk.api.failure.getRetryDelay
+import org.matrix.android.sdk.api.failure.isLimitExceededError
 import org.matrix.android.sdk.api.failure.shouldBeRetried
 import org.matrix.android.sdk.internal.network.ssl.CertUtil
 import retrofit2.HttpException
@@ -33,7 +34,8 @@ import java.io.IOException
  *
  * @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 maxDelayBeforeRetry the max delay to wait before a retry
+ * @param maxDelayBeforeRetry the max delay to wait before a retry. Note that in the case of a 429, if the provided delay exceeds this value, the error will
+ * be propagated as it does not make sense to retry it with a shorter delay.
  * @param maxRetriesCount the max number of retries
  * @param requestBlock a suspend lambda to perform the network request
  */
@@ -74,12 +76,15 @@ internal suspend inline fun <DATA> executeRequest(globalErrorReceiver: GlobalErr
 
             currentRetryCount++
 
-            if (exception is Failure.ServerError &&
+            if (exception.isLimitExceededError() && currentRetryCount < maxRetriesCount) {
-                    exception.httpCode == 429 &&
-                    exception.error.code == MatrixError.M_LIMIT_EXCEEDED &&
-                    currentRetryCount < maxRetriesCount) {
                 // 429, we can retry
-                delay(exception.getRetryDelay(1_000))
+                val retryDelay = exception.getRetryDelay(1_000)
+                if (retryDelay <= maxDelayBeforeRetry) {
+                    delay(retryDelay)
+                } else {
+                    // delay is too high to be retried, propagate the exception
+                    throw exception
+                }
             } else if (canRetry && currentRetryCount < maxRetriesCount && exception.shouldBeRetried()) {
                 delay(currentDelay)
                 currentDelay = currentDelay.times(2L).coerceAtMost(maxDelayBeforeRetry)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessorCoroutine.kt

@@ -23,8 +23,8 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import org.matrix.android.sdk.api.auth.data.SessionParams
 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.isLimitExceededError
 import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.crypto.CryptoService
 import org.matrix.android.sdk.api.session.events.model.Event
@@ -149,7 +149,7 @@ internal class EventSenderProcessorCoroutine @Inject constructor(
                     canReachServer.set(false)
                     task.markAsFailedOrRetry(exception, 0)
                 }
-                (exception is Failure.ServerError && exception.error.code == MatrixError.M_LIMIT_EXCEEDED) -> {
+                (exception.isLimitExceededError())                                 -> {
                     task.markAsFailedOrRetry(exception, exception.getRetryDelay(3_000))
                 }
                 exception is CancellationException                                 -> {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/queue/EventSenderProcessorThread.kt

@@ -23,7 +23,7 @@ import org.matrix.android.sdk.api.auth.data.SessionParams
 import org.matrix.android.sdk.api.auth.data.sessionId
 import org.matrix.android.sdk.api.extensions.tryOrNull
 import org.matrix.android.sdk.api.failure.Failure
-import org.matrix.android.sdk.api.failure.MatrixError
+import org.matrix.android.sdk.api.failure.isLimitExceededError
 import org.matrix.android.sdk.api.failure.isTokenError
 import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.crypto.CryptoService
@@ -180,7 +180,7 @@ internal class EventSenderProcessorThread @Inject constructor(
                                         waitForNetwork()
                                     }
                                 }
-                                (exception is Failure.ServerError && exception.error.code == MatrixError.M_LIMIT_EXCEEDED) -> {
+                                (exception.isLimitExceededError())                                 -> {
                                     if (task.retryCount.getAndIncrement() >= 3) task.onTaskFailed()
                                     Timber.v("## SendThread retryLoop retryable error for $task reason: ${exception.localizedMessage}")
                                     // wait a bit

vector/src/main/java/im/vector/app/core/error/ErrorFormatter.kt

@@ -24,6 +24,7 @@ import org.matrix.android.sdk.api.failure.Failure
 import org.matrix.android.sdk.api.failure.MatrixError
 import org.matrix.android.sdk.api.failure.MatrixIdFailure
 import org.matrix.android.sdk.api.failure.isInvalidPassword
+import org.matrix.android.sdk.api.failure.isLimitExceededError
 import org.matrix.android.sdk.api.session.identity.IdentityServiceError
 import java.net.HttpURLConnection
 import java.net.SocketTimeoutException
@@ -77,7 +78,7 @@ class DefaultErrorFormatter @Inject constructor(
                     throwable.error.code == MatrixError.M_THREEPID_DENIED              -> {
                         stringProvider.getString(R.string.login_error_threepid_denied)
                     }
-                    throwable.error.code == MatrixError.M_LIMIT_EXCEEDED          -> {
+                    throwable.isLimitExceededError()                                   -> {
                         limitExceededError(throwable.error)
                     }
                     throwable.error.code == MatrixError.M_TOO_LARGE                    -> {