BIT-2129 Show descriptive error message when Send creation fails (#1186)

2026-06-02 11:12:00 -05:00 · 2024-04-01 10:54:24 -04:00
parent 90ff2897f5
commit 1150f01666
14 changed files with 277 additions and 101 deletions
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/api/SendsApi.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/api/SendsApi.kt
@@ -1,7 +1,7 @@
 package com.x8bit.bitwarden.data.vault.datasource.network.api

 import androidx.annotation.Keep
-import com.x8bit.bitwarden.data.vault.datasource.network.model.SendFileResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateFileSendResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SendJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
 import okhttp3.MultipartBody
@@ -28,7 +28,7 @@ interface SendsApi {
     * Create a file send.
     */
    @POST("sends/file/v2")
-    suspend fun createFileSend(@Body body: SendJsonRequest): Result<SendFileResponseJson>
+    suspend fun createFileSend(@Body body: SendJsonRequest): Result<CreateFileSendResponseJson>

    /**
     * Updates a send.
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/CreateFileSendResponse.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/CreateFileSendResponse.kt
@@ -0,0 +1,36 @@
+package com.x8bit.bitwarden.data.vault.datasource.network.model
+
+import kotlinx.serialization.SerialName
+import kotlinx.serialization.Serializable
+
+/**
+ * Represents a response from create file send.
+ */
+sealed class CreateFileSendResponse {
+
+    /**
+     * Represents the response from a successful create file send request.
+     *
+     * @property createFileJsonResponse Response JSON received from create file send request.
+     */
+    data class Success(
+        val createFileJsonResponse: CreateFileSendResponseJson,
+    ) : CreateFileSendResponse()
+
+    /**
+     * Represents the json body of an invalid create request.
+     *
+     * @property message A general, user-displayable error message.
+     * @property validationErrors a map where each value is a list of error messages for each
+     * key. The values in the array should be used for display to the user, since the keys tend
+     * to come back as nonsense. (eg: empty string key)
+     */
+    @Serializable
+    data class Invalid(
+        @SerialName("message")
+        override val message: String,
+
+        @SerialName("validationErrors")
+        override val validationErrors: Map<String, List<String>>?,
+    ) : CreateFileSendResponse(), InvalidJsonResponse
+}
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/CreateFileSendResponseJson.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/CreateFileSendResponseJson.kt
@@ -7,7 +7,7 @@ import kotlinx.serialization.Serializable
 * Represents the JSON response from creating a new file send.
 */
@Serializable
-data class SendFileResponseJson(
+data class CreateFileSendResponseJson(
    @SerialName("url")
    val url: String,

--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/CreateSendJsonResponse.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/CreateSendJsonResponse.kt
@@ -0,0 +1,34 @@
+package com.x8bit.bitwarden.data.vault.datasource.network.model
+
+import kotlinx.serialization.SerialName
+import kotlinx.serialization.Serializable
+
+/**
+ * Models a response from either "create text send" or "create file send" requests.
+ */
+sealed class CreateSendJsonResponse {
+    /**
+     * Represents a successful response from either "create text send" or "create file send"
+     * request.
+     *
+     * @property send The created send object.
+     */
+    data class Success(val send: SyncResponseJson.Send) : CreateSendJsonResponse()
+
+    /**
+     * Represents the json body of an invalid create request.
+     *
+     * @property message A general, user-displayable error message.
+     * @property validationErrors a map where each value is a list of error messages for each
+     * key. The values in the array should be used for display to the user, since the keys tend
+     * to come back as nonsense. (eg: empty string key)
+     */
+    @Serializable
+    data class Invalid(
+        @SerialName("message")
+        override val message: String,
+
+        @SerialName("validationErrors")
+        override val validationErrors: Map<String, List<String>>?,
+    ) : CreateSendJsonResponse(), InvalidJsonResponse
+}
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/InvalidJsonResponse.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/model/InvalidJsonResponse.kt
@@ -0,0 +1,28 @@
+package com.x8bit.bitwarden.data.vault.datasource.network.model
+
+/**
+ * Represents the json body of an invalid send json request.
+ */
+sealed interface InvalidJsonResponse {
+
+    /**
+     * A general, user-displayable error message.
+     */
+    val message: String
+
+    /**
+     * a map where each value is a list of error messages for each key.
+     * The values in the array should be used for display to the user, since the keys tend to come
+     * back as nonsense. (eg: empty string key)
+     */
+    val validationErrors: Map<String, List<String>>?
+
+    /**
+     * Returns the first error message found in [validationErrors], or [message] if there are no
+     * [validationErrors] present.
+     */
+    val firstValidationErrorMessage: String?
+        get() = validationErrors
+            ?.flatMap { it.value }
+            ?.first()
+}
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsService.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsService.kt
@@ -1,6 +1,8 @@
 package com.x8bit.bitwarden.data.vault.datasource.network.service

-import com.x8bit.bitwarden.data.vault.datasource.network.model.SendFileResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateFileSendResponse
+import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateFileSendResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateSendJsonResponse
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SendJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateSendResponseJson
@@ -14,20 +16,20 @@ interface SendsService {
     */
    suspend fun createTextSend(
        body: SendJsonRequest,
-    ): Result<SyncResponseJson.Send>
+    ): Result<CreateSendJsonResponse>

    /**
     * Attempt to create a file send.
     */
    suspend fun createFileSend(
        body: SendJsonRequest,
-    ): Result<SendFileResponseJson>
+    ): Result<CreateFileSendResponse>

    /**
     * Attempt to upload the given [encryptedFile] associated with the [sendFileResponse].
     */
    suspend fun uploadFile(
-        sendFileResponse: SendFileResponseJson,
+        sendFileResponse: CreateFileSendResponseJson,
        encryptedFile: ByteArray,
    ): Result<SyncResponseJson.Send>

--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceImpl.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/network/service/SendsServiceImpl.kt
@@ -5,8 +5,10 @@ import com.x8bit.bitwarden.data.platform.datasource.network.model.toBitwardenErr
 import com.x8bit.bitwarden.data.platform.datasource.network.util.parseErrorBodyOrNull
 import com.x8bit.bitwarden.data.vault.datasource.network.api.AzureApi
 import com.x8bit.bitwarden.data.vault.datasource.network.api.SendsApi
+import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateFileSendResponse
+import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateFileSendResponseJson
+import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateSendJsonResponse
 import com.x8bit.bitwarden.data.vault.datasource.network.model.FileUploadType
-import com.x8bit.bitwarden.data.vault.datasource.network.model.SendFileResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SendJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateSendResponseJson
@@ -28,11 +30,33 @@ class SendsServiceImpl(
    private val clock: Clock,
    private val json: Json,
 ) : SendsService {
-    override suspend fun createTextSend(body: SendJsonRequest): Result<SyncResponseJson.Send> =
+    override suspend fun createTextSend(
+        body: SendJsonRequest,
+    ): Result<CreateSendJsonResponse> =
        sendsApi.createTextSend(body = body)
+            .map { CreateSendJsonResponse.Success(send = it) }
+            .recoverCatching { throwable ->
+                throwable.toBitwardenError()
+                    .parseErrorBodyOrNull<CreateSendJsonResponse.Invalid>(
+                        code = 400,
+                        json = json,
+                    )
+                    ?: throw throwable
+            }

-    override suspend fun createFileSend(body: SendJsonRequest): Result<SendFileResponseJson> =
+    override suspend fun createFileSend(
+        body: SendJsonRequest,
+    ): Result<CreateFileSendResponse> =
        sendsApi.createFileSend(body = body)
+            .map { CreateFileSendResponse.Success(it) }
+            .recoverCatching { throwable ->
+                throwable.toBitwardenError()
+                    .parseErrorBodyOrNull<CreateFileSendResponse.Invalid>(
+                        code = 400,
+                        json = json,
+                    )
+                    ?: throw throwable
+            }

    override suspend fun updateSend(
        sendId: String,
@@ -55,7 +79,7 @@ class SendsServiceImpl(
            }

    override suspend fun uploadFile(
-        sendFileResponse: SendFileResponseJson,
+        sendFileResponse: CreateFileSendResponseJson,
        encryptedFile: ByteArray,
    ): Result<SyncResponseJson.Send> {
        val send = sendFileResponse.sendResponse
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt
@@ -10,6 +10,7 @@ import com.bitwarden.core.ExportFormat
 import com.bitwarden.core.FolderView
 import com.bitwarden.core.InitOrgCryptoRequest
 import com.bitwarden.core.InitUserCryptoMethod
+import com.bitwarden.core.Send
 import com.bitwarden.core.SendType
 import com.bitwarden.core.SendView
 import com.bitwarden.crypto.Kdf
@@ -35,10 +36,13 @@ import com.x8bit.bitwarden.data.platform.repository.util.map
 import com.x8bit.bitwarden.data.platform.repository.util.observeWhenSubscribedAndLoggedIn
 import com.x8bit.bitwarden.data.platform.repository.util.updateToPendingOrLoading
 import com.x8bit.bitwarden.data.platform.util.asFailure
+import com.x8bit.bitwarden.data.platform.util.asSuccess
 import com.x8bit.bitwarden.data.platform.util.flatMap
 import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
 import com.x8bit.bitwarden.data.vault.datasource.network.model.AttachmentJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateCipherInOrganizationJsonRequest
+import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateFileSendResponse
+import com.x8bit.bitwarden.data.vault.datasource.network.model.CreateSendJsonResponse
 import com.x8bit.bitwarden.data.vault.datasource.network.model.ShareCipherJsonRequest
 import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
 import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherCollectionsJsonRequest
@@ -980,11 +984,12 @@ class VaultRepositoryImpl(
            )
    }

+    @Suppress("ReturnCount")
    override suspend fun createSend(
        sendView: SendView,
        fileUri: Uri?,
    ): CreateSendResult {
-        val userId = activeUserId ?: return CreateSendResult.Error
+        val userId = activeUserId ?: return CreateSendResult.Error(message = null)
        return vaultSdkSource
            .encryptSend(
                userId = userId,
@@ -992,54 +997,33 @@ class VaultRepositoryImpl(
            )
            .flatMap { send ->
                when (send.type) {
-                    SendType.TEXT -> {
-                        sendsService.createTextSend(body = send.toEncryptedNetworkSend())
+                    SendType.TEXT -> sendsService.createTextSend(send.toEncryptedNetworkSend())
+                    SendType.FILE -> createFileSend(fileUri, userId, send)
+                }
+            }
+            .map { createSendResponse ->
+                when (createSendResponse) {
+                    is CreateSendJsonResponse.Invalid -> {
+                        return CreateSendResult.Error(
+                            message = createSendResponse.firstValidationErrorMessage,
+                        )
                    }

-                    SendType.FILE -> {
-                        val uri = fileUri ?: return@flatMap IllegalArgumentException(
-                            "File URI must be present to create a File Send.",
-                        )
-                            .asFailure()
-
-                        fileManager
-                            .uriToByteArray(fileUri = uri)
-                            .flatMap {
-                                vaultSdkSource.encryptBuffer(
-                                    userId = userId,
-                                    send = send,
-                                    fileBuffer = it,
-                                )
-                            }
-                            .flatMap { encryptedFile ->
-                                sendsService
-                                    .createFileSend(
-                                        body = send.toEncryptedNetworkSend(
-                                            fileLength = encryptedFile.size,
-                                        ),
-                                    )
-                                    .flatMap { sendFileResponse ->
-                                        sendsService.uploadFile(
-                                            sendFileResponse = sendFileResponse,
-                                            encryptedFile = encryptedFile,
-                                        )
-                                    }
-                            }
+                    is CreateSendJsonResponse.Success -> {
+                        // Save the send immediately, regardless of whether the decrypt succeeds
+                        vaultDiskSource.saveSend(userId = userId, send = createSendResponse.send)
+                        createSendResponse
                    }
                }
            }
-            .onSuccess {
-                // Save the send immediately, regardless of whether the decrypt succeeds
-                vaultDiskSource.saveSend(userId = userId, send = it)
-            }
-            .flatMap {
+            .flatMap { createSendSuccessResponse ->
                vaultSdkSource.decryptSend(
                    userId = userId,
-                    send = it.toEncryptedSdkSend(),
+                    send = createSendSuccessResponse.send.toEncryptedSdkSend(),
                )
            }
            .fold(
-                onFailure = { CreateSendResult.Error },
+                onFailure = { CreateSendResult.Error(message = null) },
                onSuccess = { CreateSendResult.Success(it) },
            )
    }
@@ -1594,6 +1578,56 @@ class VaultRepositoryImpl(
            )
    }

+    private suspend fun createFileSend(
+        uri: Uri?,
+        userId: String,
+        send: Send,
+    ): Result<CreateSendJsonResponse> {
+        uri ?: return IllegalArgumentException(
+            "File URI must be present to create a File Send.",
+        )
+            .asFailure()
+
+        return fileManager
+            .uriToByteArray(fileUri = uri)
+            .flatMap {
+                vaultSdkSource.encryptBuffer(
+                    userId = userId,
+                    send = send,
+                    fileBuffer = it,
+                )
+            }
+            .flatMap { encryptedFile ->
+                sendsService
+                    .createFileSend(
+                        body = send.toEncryptedNetworkSend(
+                            fileLength = encryptedFile.size,
+                        ),
+                    )
+                    .flatMap { sendFileResponse ->
+                        when (sendFileResponse) {
+                            is CreateFileSendResponse.Invalid -> {
+                                CreateSendJsonResponse
+                                    .Invalid(
+                                        message = sendFileResponse.message,
+                                        validationErrors = sendFileResponse.validationErrors,
+                                    )
+                                    .asSuccess()
+                            }
+
+                            is CreateFileSendResponse.Success -> {
+                                sendsService
+                                    .uploadFile(
+                                        sendFileResponse = sendFileResponse.createFileJsonResponse,
+                                        encryptedFile = encryptedFile,
+                                    )
+                                    .map { CreateSendJsonResponse.Success(it) }
+                            }
+                        }
+                    }
+            }
+    }
+
    /**
     * Deletes the send specified by [syncSendDeleteData] from disk.
     */
--- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/CreateSendResult.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/CreateSendResult.kt
@@ -15,5 +15,5 @@ sealed class CreateSendResult {
    /**
     * Generic error while creating a send.
     */
-    data object Error : CreateSendResult()
+    data class Error(val message: String?) : CreateSendResult()
 }
--- a/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt
+++ b/app/src/main/java/com/x8bit/bitwarden/ui/tools/feature/send/addsend/AddSendViewModel.kt
@@ -184,12 +184,13 @@ class AddSendViewModel @Inject constructor(
        action: AddSendAction.Internal.CreateSendResultReceive,
    ) {
        when (val result = action.result) {
-            CreateSendResult.Error -> {
+            is CreateSendResult.Error -> {
                mutableStateFlow.update {
                    it.copy(
                        dialogState = AddSendState.DialogState.Error(
                            title = R.string.an_error_has_occurred.asText(),
-                            message = R.string.generic_error_message.asText(),
+                            message = result.message?.asText()
+                                ?: R.string.generic_error_message.asText(),
                        ),
                    )
                }