feat: limit re-trying commit to 2 retry attemps (#2068)

* feat: limit re-trying commit to 2 retry attemps * refactor: rename retryCount to remainingAttempts for better readability
wireapp · Sep 26, 2023 · 7481d37 · 7481d37
1 parent 3cdaa0c
commit 7481d37
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 3 deletions.
diff --git a/...rc/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt b/...rc/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt
@@ -117,10 +117,15 @@ private enum class CommitStrategy {
 }
 
 private fun CoreFailure.getStrategy(
+    remainingAttempts: Int,
     retryOnClientMismatch: Boolean = true,
     retryOnStaleMessage: Boolean = true
 ): CommitStrategy {
-    return if (this is NetworkFailure.ServerMiscommunication && this.kaliumException is KaliumException.InvalidRequestError) {
+    return if (
+        remainingAttempts > 0 &&
+        this is NetworkFailure.ServerMiscommunication &&
+        kaliumException is KaliumException.InvalidRequestError
+    ) {
         if (this.kaliumException.isMlsClientMismatch() && retryOnClientMismatch) {
             CommitStrategy.DISCARD_AND_RETRY
         } else if (
@@ -520,18 +525,27 @@ internal class MLSConversationDataSource(
     ) =
         operation()
             .flatMapLeft {
-                handleCommitFailure(it, groupID, retryOnClientMismatch, retryOnStaleMessage, operation)
+                handleCommitFailure(
+                    failure = it,
+                    groupID = groupID,
+                    remainingAttempts = 2,
+                    retryOnClientMismatch = retryOnClientMismatch,
+                    retryOnStaleMessage = retryOnStaleMessage,
+                    retryOperation = operation
+                )
             }
 
     private suspend fun handleCommitFailure(
         failure: CoreFailure,
         groupID: GroupID,
+        remainingAttempts: Int,
         retryOnClientMismatch: Boolean,
         retryOnStaleMessage: Boolean,
         retryOperation: suspend () -> Either<CoreFailure, Unit>
     ): Either<CoreFailure, Unit> {
         return when (
             failure.getStrategy(
+                remainingAttempts = remainingAttempts,
                 retryOnClientMismatch = retryOnClientMismatch,
                 retryOnStaleMessage = retryOnStaleMessage
             )
@@ -540,7 +554,14 @@ internal class MLSConversationDataSource(
             CommitStrategy.DISCARD_AND_RETRY -> discardCommitAndRetry(groupID, retryOperation)
             CommitStrategy.ABORT -> return discardCommit(groupID).flatMap { Either.Left(failure) }
         }.flatMapLeft {
-            handleCommitFailure(it, groupID, retryOnClientMismatch, retryOnStaleMessage, retryOperation)
+            handleCommitFailure(
+                failure = it,
+                groupID = groupID,
+                remainingAttempts = remainingAttempts - 1,
+                retryOnClientMismatch = retryOnClientMismatch,
+                retryOnStaleMessage = retryOnStaleMessage,
+                retryOperation = retryOperation
+            )
         }
     }
 

diff --git a/...ommonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt b/...ommonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt
@@ -423,6 +423,27 @@ class MLSConversationRepositoryTest {
             .wasInvoked(once)
     }
 
+    @Test
+    fun givenRetryLimitIsReached_whenCallingAddMemberToMLSGroup_thenClearCommitAndFail() = runTest {
+        val (arrangement, mlsConversationRepository) = Arrangement()
+            .withClaimKeyPackagesSuccessful()
+            .withGetMLSClientSuccessful()
+            .withAddMLSMemberSuccessful()
+            .withSendCommitBundleFailing(Arrangement.MLS_STALE_MESSAGE_ERROR, times = Int.MAX_VALUE)
+            .withCommitPendingProposalsSuccessful()
+            .withClearProposalTimerSuccessful()
+            .withWaitUntilLiveSuccessful()
+            .arrange()
+
+        val result = mlsConversationRepository.addMemberToMLSGroup(Arrangement.GROUP_ID, listOf(TestConversation.USER_ID1))
+        result.shouldFail()
+
+        verify(arrangement.mlsClient)
+            .function(arrangement.mlsClient::clearPendingCommit)
+            .with(eq(Arrangement.RAW_GROUP_ID))
+            .wasInvoked(once)
+    }
+
     @Test
     fun givenSuccessfulResponses_whenCallingRequestToJoinGroup_ThenGroupStateIsUpdated() = runTest {
         val (arrangement, mlsConversationRepository) = Arrangement()
@@ -570,6 +591,24 @@ class MLSConversationRepositoryTest {
             .wasInvoked(once)
     }
 
+    @Test
+    fun givenRetryLimitIsReached_whenCallingCommitPendingProposals_thenClearCommitAndFail() = runTest {
+        val (arrangement, mlsConversationRepository) = Arrangement()
+            .withGetMLSClientSuccessful()
+            .withCommitPendingProposalsSuccessful()
+            .withSendCommitBundleFailing(Arrangement.MLS_STALE_MESSAGE_ERROR, times = Int.MAX_VALUE)
+            .withWaitUntilLiveSuccessful()
+            .arrange()
+
+        val result = mlsConversationRepository.commitPendingProposals(Arrangement.GROUP_ID)
+        result.shouldFail()
+
+        verify(arrangement.mlsClient)
+            .function(arrangement.mlsClient::clearPendingCommit)
+            .with(eq(Arrangement.RAW_GROUP_ID))
+            .wasInvoked(once)
+    }
+
     @Test
     fun givenSuccessfulResponses_whenCallingRemoveMemberFromGroup_thenCommitBundleIsSentAndAccepted() = runTest {
         val (arrangement, mlsConversationRepository) = Arrangement()
@@ -655,6 +694,27 @@ class MLSConversationRepositoryTest {
             .wasInvoked(once)
     }
 
+    @Test
+    fun givenRetryLimitIsReached_whenCallingRemoveMemberFromGroup_thenClearCommitAndFail() = runTest {
+        val (arrangement, mlsConversationRepository) = Arrangement()
+            .withCommitPendingProposalsSuccessful()
+            .withGetMLSClientSuccessful()
+            .withFetchClientsOfUsersSuccessful()
+            .withRemoveMemberSuccessful()
+            .withSendCommitBundleFailing(Arrangement.MLS_STALE_MESSAGE_ERROR, times = Int.MAX_VALUE)
+            .withWaitUntilLiveSuccessful()
+            .arrange()
+
+        val users = listOf(TestUser.USER_ID)
+        val result = mlsConversationRepository.removeMembersFromMLSGroup(Arrangement.GROUP_ID, users)
+        result.shouldFail()
+
+        verify(arrangement.mlsClient)
+            .function(arrangement.mlsClient::clearPendingCommit)
+            .with(eq(Arrangement.RAW_GROUP_ID))
+            .wasInvoked(once)
+    }
+
     @Test
     fun givenClientMismatchError_whenCallingRemoveMemberFromGroup_thenClearCommitAndRetry() = runTest {
         val (arrangement, mlsConversationRepository) = Arrangement()
@@ -892,6 +952,25 @@ class MLSConversationRepositoryTest {
             .wasInvoked(once)
     }
 
+    @Test
+    fun givenRetryLimitIsReached_whenCallingUpdateKeyMaterial_clearCommitAndFail() = runTest {
+        val (arrangement, mlsConversationRepository) = Arrangement()
+            .withGetMLSClientSuccessful()
+            .withUpdateKeyingMaterialSuccessful()
+            .withCommitPendingProposalsSuccessful()
+            .withSendCommitBundleFailing(Arrangement.MLS_STALE_MESSAGE_ERROR, times = Int.MAX_VALUE)
+            .withWaitUntilLiveSuccessful()
+            .arrange()
+
+        val result = mlsConversationRepository.updateKeyingMaterial(Arrangement.GROUP_ID)
+        result.shouldFail()
+
+        verify(arrangement.mlsClient)
+            .function(arrangement.mlsClient::clearPendingCommit)
+            .with(eq(Arrangement.RAW_GROUP_ID))
+            .wasInvoked(once)
+    }
+
     @Test
     fun givenConversationWithOutdatedEpoch_whenCallingIsGroupOutOfSync_returnsTrue() = runTest {
         val returnEpoch = 10UL