Skip to content

Commit

Permalink
feat: limit re-trying commit to 2 retry attemps (#2068)
Browse files Browse the repository at this point in the history
* feat: limit re-trying commit to 2 retry attemps

* refactor: rename retryCount to remainingAttempts for better readability
  • Loading branch information
typfel committed Oct 6, 2023
1 parent 9951637 commit cc275ae
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
)
Expand All @@ -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
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,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()
Expand Down Expand Up @@ -566,6 +587,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()
Expand Down Expand Up @@ -651,6 +690,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()
Expand Down Expand Up @@ -888,6 +948,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
Expand Down

0 comments on commit cc275ae

Please sign in to comment.