Skip to content

Commit 9e0501a

Browse files
authored
fix: skip stale groups in MLS conversation recovery instead of aborting batch [WPB-23438] (#3852)
* fix: skip stale groups in MLS conversation recovery instead of aborting batch When isLocalGroupEpochStale fails with ConversationNotFound (stale epoch group ID not in local CoreCrypto state), the recovery manager would propagate the error and abort the entire batch via foldToEitherWhileRight. This left needsToRecoverMLSGroups=true, causing the recovery to retry on every sync state change — producing repeated ConversationNotFound log noise. Now ConversationNotFound is treated as non-fatal: the group is skipped and recovery continues with the remaining conversations. * style: fix detekt argument wrapping in MLS recovery
1 parent 017a1b5 commit 9e0501a

File tree

2 files changed

+94
-1
lines changed

2 files changed

+94
-1
lines changed

logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/conversation/RecoverMLSConversationsUseCase.kt

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package com.wire.kalium.logic.feature.conversation
2020

2121
import com.wire.kalium.common.error.CoreFailure
22+
import com.wire.kalium.common.error.MLSFailure
2223
import com.wire.kalium.logic.data.client.ClientRepository
2324
import com.wire.kalium.logic.data.conversation.Conversation
2425
import com.wire.kalium.logic.data.conversation.Conversation.ProtocolInfo.MLSCapable.GroupState
@@ -86,7 +87,19 @@ internal class RecoverMLSConversationsUseCaseImpl(
8687
mlsConversationRepository.isLocalGroupEpochStale(mlsContext, protocol.groupId, protocol.epoch)
8788
}
8889
.fold({ checkEpochFailure ->
89-
Either.Left(checkEpochFailure)
90+
if (checkEpochFailure is MLSFailure.ConversationNotFound) {
91+
kaliumLogger.w(
92+
"Marking ${protocol.groupId.toLogString()} as PENDING_AFTER_RESET: " +
93+
"group not found in local MLS state, will retry join on next sync"
94+
)
95+
conversationRepository.updateConversationGroupState(
96+
protocol.groupId,
97+
GroupState.PENDING_AFTER_RESET
98+
)
99+
Either.Right(Unit)
100+
} else {
101+
Either.Left(checkEpochFailure)
102+
}
90103
}, { isGroupOutOfSync ->
91104
if (isGroupOutOfSync) {
92105
joinExistingMLSConversationUseCase(transactionContext, conversation.id).onFailure { joinFailure ->

logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/conversation/RecoverMLSConversationsUseCaseTests.kt

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818

1919
package com.wire.kalium.logic.feature.conversation
2020

21+
import com.wire.kalium.common.error.MLSFailure
2122
import com.wire.kalium.common.error.StorageFailure
23+
import com.wire.kalium.logic.data.conversation.Conversation.ProtocolInfo.MLSCapable.GroupState
2224
import com.wire.kalium.logic.data.client.ClientRepository
2325
import com.wire.kalium.logic.data.conversation.Conversation
2426
import com.wire.kalium.logic.data.conversation.ConversationRepository
@@ -143,6 +145,72 @@ class RecoverMLSConversationsUseCaseTests {
143145
assertIs<RecoverMLSConversationsResult.Success>(actual)
144146
}
145147

148+
@Test
149+
fun givenEpochCheckFailsWithConversationNotFound_ThenGroupIsMarkedPendingAndOtherGroupsProcessed() = runTest {
150+
val conversations = listOf(Arrangement.MLS_CONVERSATION1, Arrangement.MLS_CONVERSATION2)
151+
val (arrangement, recoverMLSConversationsUseCase) = Arrangement()
152+
.withConversationsByGroupStateReturns(Either.Right(conversations))
153+
.withJoinExistingMLSConversationUseCaseSuccessful()
154+
.withIsMLSSupported(true)
155+
.withHasRegisteredMLSClient(true)
156+
.withUpdateConversationGroupStateSuccessful()
157+
.withEpochCheckFailsWithConversationNotFoundFor(Arrangement.GROUP_ID1)
158+
.withConversationIsOutOfSyncReturnsTrueFor(listOf(Arrangement.GROUP_ID2))
159+
.arrange()
160+
161+
val actual = recoverMLSConversationsUseCase(arrangement.transactionContext)
162+
163+
// Both groups should have epoch check attempted
164+
coVerify {
165+
arrangement.mlsConversationRepository.isLocalGroupEpochStale(any(), any(), any())
166+
}.wasInvoked(conversations.size)
167+
168+
// First group should be marked as PENDING_AFTER_RESET so it can be re-joined on next sync
169+
coVerify {
170+
arrangement.conversationRepository.updateConversationGroupState(
171+
eq(Arrangement.GROUP_ID1), eq(GroupState.PENDING_AFTER_RESET)
172+
)
173+
}.wasInvoked(once)
174+
175+
// Only the second group should be joined (first was marked pending due to ConversationNotFound)
176+
coVerify {
177+
arrangement.joinExistingMLSConversationUseCase.invoke(any(), eq(Arrangement.MLS_CONVERSATION2.id), any())
178+
}.wasInvoked(once)
179+
180+
// Overall result should be Success since ConversationNotFound is non-fatal
181+
assertIs<RecoverMLSConversationsResult.Success>(actual)
182+
}
183+
184+
@Test
185+
fun givenAllGroupsFailWithConversationNotFound_ThenAllMarkedPendingAndResultIsSuccess() = runTest {
186+
val conversations = listOf(Arrangement.MLS_CONVERSATION1, Arrangement.MLS_CONVERSATION2)
187+
val (arrangement, recoverMLSConversationsUseCase) = Arrangement()
188+
.withConversationsByGroupStateReturns(Either.Right(conversations))
189+
.withJoinExistingMLSConversationUseCaseSuccessful()
190+
.withIsMLSSupported(true)
191+
.withHasRegisteredMLSClient(true)
192+
.withUpdateConversationGroupStateSuccessful()
193+
.withEpochCheckFailsWithConversationNotFoundFor(Arrangement.GROUP_ID1)
194+
.withEpochCheckFailsWithConversationNotFoundFor(Arrangement.GROUP_ID2)
195+
.arrange()
196+
197+
val actual = recoverMLSConversationsUseCase(arrangement.transactionContext)
198+
199+
// Both groups should be marked as PENDING_AFTER_RESET
200+
coVerify {
201+
arrangement.conversationRepository.updateConversationGroupState(
202+
any(), eq(GroupState.PENDING_AFTER_RESET)
203+
)
204+
}.wasInvoked(conversations.size)
205+
206+
// No groups should be joined since all were marked pending
207+
coVerify {
208+
arrangement.joinExistingMLSConversationUseCase.invoke(any(), any(), any())
209+
}.wasNotInvoked()
210+
211+
assertIs<RecoverMLSConversationsResult.Success>(actual)
212+
}
213+
146214
@Test
147215
fun whenFetchingListOfConversationsFails_ThenShouldReturnFailure() = runTest {
148216
val (arrangement, recoverMLSConversationsUseCase) = Arrangement()
@@ -216,6 +284,18 @@ class RecoverMLSConversationsUseCaseTests {
216284
}.returns(Either.Right(true))
217285
}
218286

287+
suspend fun withUpdateConversationGroupStateSuccessful() = apply {
288+
coEvery {
289+
conversationRepository.updateConversationGroupState(any(), any())
290+
}.returns(Either.Right(Unit))
291+
}
292+
293+
suspend fun withEpochCheckFailsWithConversationNotFoundFor(groupID: GroupID) = apply {
294+
coEvery {
295+
mlsConversationRepository.isLocalGroupEpochStale(any(), eq(groupID), any())
296+
}.returns(Either.Left(MLSFailure.ConversationNotFound))
297+
}
298+
219299
suspend fun withJoinExistingMLSConversationUseCaseFailsFor(failedGroupId: ConversationId) = apply {
220300
coEvery {
221301
joinExistingMLSConversationUseCase.invoke(any(), eq(failedGroupId), any())

0 commit comments

Comments
 (0)