Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wrong failType for MLS federation errors [WPB-9087] 🍒 #2767

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ internal class ConversationGroupRepositoryImpl(
Either.Right(Unit)
} else {
newGroupConversationSystemMessagesCreator.value.conversationFailedToAddMembers(
conversationEntity.id.toModel(), protocolSpecificAdditionFailures.toList(), FailedToAdd.Type.Unknown
conversationId = conversationEntity.id.toModel(),
userIdList = protocolSpecificAdditionFailures.toList(),
type = FailedToAdd.Type.Federation
)
}
}.flatMap {
Expand Down Expand Up @@ -427,7 +429,7 @@ internal class ConversationGroupRepositoryImpl(
}
}
} else {
val failType = (lastUsersAttempt as? LastUsersAttempt.Failed)?.failType ?: FailedToAdd.Type.Unknown
val failType = apiResult.value.toFailedToAddType()
newGroupConversationSystemMessagesCreator.value.conversationFailedToAddMembers(
conversationId, userIdList + lastUsersAttempt.failedUsers, failType
).flatMap {
Expand Down Expand Up @@ -597,6 +599,12 @@ internal class ConversationGroupRepositoryImpl(
Either.Right(ValidToInvalidUsers(userIdList, emptyList(), FailedToAdd.Type.Unknown))
}

private fun CoreFailure.toFailedToAddType() = when {
this is NetworkFailure.FederatedBackendFailure -> FailedToAdd.Type.Federation
this.isMissingLegalHoldConsentError -> FailedToAdd.Type.LegalHold
else -> FailedToAdd.Type.Unknown
}

/**
* Filter the initial [userIdList] into valid and invalid users where valid users are only team members.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ class ConversationGroupRepositoryTest {
arrangement.newGroupConversationSystemMessagesCreator.conversationFailedToAddMembers(
any(),
matches { it.containsAll(missingMembersFromMLSGroup) },
any()
eq(MessageContent.MemberChange.FailedToAdd.Type.Federation)
)
}.wasInvoked(once)
}
Expand Down Expand Up @@ -1162,6 +1162,43 @@ class ConversationGroupRepositoryTest {
}.wasInvoked(once)
}

@Test
fun givenAConversationFailsWithGeneralFederationError_whenAddingMembers_thenRetryIsNotExecutedAndCreateSysMessage() =
runTest {
// given
val (arrangement, conversationGroupRepository) = Arrangement()
.withConversationDetailsById(TestConversation.CONVERSATION)
.withProtocolInfoById(PROTEUS_PROTOCOL_INFO)
.withFetchUsersIfUnknownByIdsSuccessful()
.withAddMemberAPIFailsFirstWithUnreachableThenSucceed(arrayOf(FEDERATION_ERROR_GENERAL, FEDERATION_ERROR_GENERAL))
.withSuccessfulHandleMemberJoinEvent()
.withInsertFailedToAddSystemMessageSuccess()
.arrange()

// when
val expectedInitialUsersNotFromUnreachableInformed = listOf(TestConversation.USER_1)
conversationGroupRepository.addMembers(expectedInitialUsersNotFromUnreachableInformed, TestConversation.ID).shouldFail()

// then
coVerify {
arrangement.conversationApi.addMember(any(), any())
}.wasInvoked(exactly = once)

coVerify {
arrangement.memberJoinEventHandler.handle(any())
}.wasNotInvoked()

coVerify {
arrangement.newGroupConversationSystemMessagesCreator.conversationFailedToAddMembers(
conversationId = any(),
userIdList = matches {
it.containsAll(expectedInitialUsersNotFromUnreachableInformed)
},
type = eq(MessageContent.MemberChange.FailedToAdd.Type.Federation)
)
}.wasInvoked(once)
}

@Test
fun givenAValidConversation_whenCreating_thenConversationIsCreatedAndUnverifiedWarningSystemMessagePersisted() = runTest {
val (arrangement, conversationGroupRepository) = Arrangement()
Expand Down Expand Up @@ -1887,6 +1924,10 @@ class ConversationGroupRepositoryTest {
)
)

val FEDERATION_ERROR_GENERAL = NetworkResponse.Error(
KaliumException.FederationError(ErrorResponse(422, "", "federation-remote-error"))
)

val ERROR_MISSING_LEGALHOLD_CONSENT = NetworkResponse.Error(
KaliumException.InvalidRequestError(
ErrorResponse(
Expand Down
Loading