From 5d8695cf5e117a31735f327a6c6ba084b7c48629 Mon Sep 17 00:00:00 2001 From: Yamil Medina Date: Fri, 8 Dec 2023 11:53:20 +0100 Subject: [PATCH] fix: commit bundles special api response handling (WPB-2229) (#2296) * fix: commit bundles special handling * fix: detekt * fix: detekt --- .../wire/kalium/network/utils/NetworkUtils.kt | 41 +++++++--- .../wire/kalium/api/v5/MLSMessageApiV5Test.kt | 77 +++++++++++++++++++ 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/utils/NetworkUtils.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/utils/NetworkUtils.kt index a91ed94a815..d1617aee9e1 100644 --- a/network/src/commonMain/kotlin/com/wire/kalium/network/utils/NetworkUtils.kt +++ b/network/src/commonMain/kotlin/com/wire/kalium/network/utils/NetworkUtils.kt @@ -32,6 +32,7 @@ import io.ktor.http.HttpStatusCode import io.ktor.http.URLProtocol import io.ktor.http.Url import io.ktor.http.isSuccess +import io.ktor.serialization.JsonConvertException internal fun HttpRequestBuilder.setWSSUrl(baseUrl: Url, vararg path: String) { url { @@ -237,23 +238,19 @@ private fun toStatusCodeBasedKaliumException( /** * Wrap and handles federation aware endpoints that can send errors responses - * And raise proper federated exceptions + * And raise specific federated context exceptions, * - * @delegatedHandler the fallback handler when the response is not a federation error + * i.e. FederationError, FederationUnreachableException, FederationConflictException + * + * @param response the response to wrap + * @param delegatedHandler the fallback handler when the response cannot be handled as a federation error */ suspend fun wrapFederationResponse( response: HttpResponse, delegatedHandler: suspend (HttpResponse) -> NetworkResponse ) = when (response.status.value) { - HttpStatusCode.Conflict.value -> { - val errorResponse = try { - response.body() - } catch (_: NoTransformationFoundException) { - FederationConflictResponse(emptyList()) - } - NetworkResponse.Error(KaliumException.FederationConflictException(errorResponse)) - } + HttpStatusCode.Conflict.value -> resolveStatusCodeBasedFirstOrFederated(response) HttpStatusCode.UnprocessableEntity.value -> { val errorResponse = try { @@ -277,3 +274,27 @@ suspend fun wrapFederationResponse( delegatedHandler.invoke(response) } } + +/** + * Due to the "shared" status code limitations nature of some endpoints. + * We need to first delegate to status code based exceptions and if parse fails go for federated error, + * + * i.e.: '/commit-bundles' 409 for "mls-stale-message" and 409 for "federation-conflict" + */ +private suspend fun resolveStatusCodeBasedFirstOrFederated(response: HttpResponse): NetworkResponse.Error { + val kaliumException = try { + val errorResponse = response.body() + toStatusCodeBasedKaliumException( + response.status, + response, + errorResponse + ) + } catch (exception: JsonConvertException) { + try { + KaliumException.FederationConflictException(response.body()) + } catch (_: NoTransformationFoundException) { + KaliumException.FederationConflictException(FederationConflictResponse(emptyList())) + } + } + return NetworkResponse.Error(kaliumException) +} diff --git a/network/src/commonTest/kotlin/com/wire/kalium/api/v5/MLSMessageApiV5Test.kt b/network/src/commonTest/kotlin/com/wire/kalium/api/v5/MLSMessageApiV5Test.kt index d08d1a3cb2f..5b9959b2c66 100644 --- a/network/src/commonTest/kotlin/com/wire/kalium/api/v5/MLSMessageApiV5Test.kt +++ b/network/src/commonTest/kotlin/com/wire/kalium/api/v5/MLSMessageApiV5Test.kt @@ -19,17 +19,24 @@ package com.wire.kalium.api.v5 import com.wire.kalium.api.ApiTest +import com.wire.kalium.api.json.model.ErrorResponseJson +import com.wire.kalium.model.EventContentDTOJson import com.wire.kalium.model.SendMLSMessageResponseJson import com.wire.kalium.network.api.base.authenticated.message.MLSMessageApi +import com.wire.kalium.network.api.base.model.ErrorResponse +import com.wire.kalium.network.api.base.model.FederationConflictResponse import com.wire.kalium.network.api.v0.authenticated.MLSMessageApiV0 import com.wire.kalium.network.api.v5.authenticated.MLSMessageApiV5 +import com.wire.kalium.network.exceptions.KaliumException import com.wire.kalium.network.serialization.Mls +import com.wire.kalium.network.utils.UnreachableRemoteBackends import com.wire.kalium.network.utils.isSuccessful import io.ktor.http.ContentType import io.ktor.http.HttpStatusCode import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue @@ -80,6 +87,76 @@ internal class MLSMessageApiV5Test : ApiTest() { assertFalse(response.isSuccessful()) } + @Test + fun givenCommitBundle_whenSendingBundleFailsUnreachable_theRequestShouldFailWithUnreachableError() = runTest { + val networkClient = mockAuthenticatedNetworkClient( + EventContentDTOJson.jsonProviderMemberJoinFailureUnreachable, + statusCode = HttpStatusCode.UnreachableRemoteBackends, + assertion = + { + assertPost() + assertContentType(ContentType.Message.Mls) + assertPathEqual(PATH_COMMIT_BUNDLES) + } + ) + val mlsMessageApi: MLSMessageApi = MLSMessageApiV5(networkClient) + val response = mlsMessageApi.sendCommitBundle(COMMIT_BUNDLE) + + assertFalse(response.isSuccessful()) + assertEquals(KaliumException.FederationUnreachableException::class, response.kException::class) + } + + @Test + fun givenCommitBundle_whenSendingBundleFailsConflict_theRequestShouldFailWithConflictDomainsError() = runTest { + val nonFederatingBackends = listOf("bella.wire.link", "foma.wire.link") + val networkClient = mockAuthenticatedNetworkClient( + ErrorResponseJson.validFederationConflictingBackends( + FederationConflictResponse(nonFederatingBackends) + ).rawJson, + statusCode = HttpStatusCode.Conflict, + assertion = + { + assertPost() + assertContentType(ContentType.Message.Mls) + assertPathEqual(PATH_COMMIT_BUNDLES) + } + ) + val mlsMessageApi: MLSMessageApi = MLSMessageApiV5(networkClient) + val response = mlsMessageApi.sendCommitBundle(COMMIT_BUNDLE) + + assertFalse(response.isSuccessful()) + assertEquals(KaliumException.FederationConflictException::class, response.kException::class) + assertEquals( + expected = nonFederatingBackends, + actual = (response.kException as KaliumException.FederationConflictException).errorResponse.nonFederatingBackends + ) + } + + @Test + fun givenCommitBundle_whenSendingBundleFailsConflictNonFederated_theRequestShouldFailWithRegularInvalidRequestError() = runTest { + val networkClient = mockAuthenticatedNetworkClient( + ErrorResponseJson.valid( + ErrorResponse( + code = HttpStatusCode.Conflict.value, + message = "some other error", + label = "mls-stale-message" + ) + ).rawJson, + statusCode = HttpStatusCode.Conflict, + assertion = + { + assertPost() + assertContentType(ContentType.Message.Mls) + assertPathEqual(PATH_COMMIT_BUNDLES) + } + ) + val mlsMessageApi: MLSMessageApi = MLSMessageApiV5(networkClient) + val response = mlsMessageApi.sendCommitBundle(COMMIT_BUNDLE) + + assertFalse(response.isSuccessful()) + assertEquals(KaliumException.InvalidRequestError::class, response.kException::class) + } + private companion object { const val PATH_MESSAGE = "/mls/messages" const val PATH_COMMIT_BUNDLES = "mls/commit-bundles"