Merge pull request #1982 from Infomaniak/attachments-sentry-breadcrumbs

Add more breadcrumbs to better understand Attachments issues

app/src/main/java/com/infomaniak/mail/data/cache/mailboxContent/DraftController.kt

@@ -26,6 +26,7 @@ import com.infomaniak.mail.data.models.draft.Draft
 import com.infomaniak.mail.data.models.draft.Draft.DraftMode
 import com.infomaniak.mail.data.models.message.Message
 import com.infomaniak.mail.utils.AccountUtils
+import com.infomaniak.mail.utils.SentryDebug
 import io.realm.kotlin.MutableRealm
 import io.realm.kotlin.Realm
 import io.realm.kotlin.TypedRealm
@@ -100,6 +101,7 @@ class DraftController @Inject constructor(
                         resource = previousMessage.attachments.find { it.name == name }?.resource
                         setUploadStatus(UploadStatus.FINISHED)
                     }
+                    SentryDebug.addAttachmentsBreadcrumb(draft, step = "set previousMessage when reply/replyAll/Forward")
                 }
 
                 draft.uiQuote = replyForwardFooterManager.createForwardFooter(previousMessage, draft.attachments)

app/src/main/java/com/infomaniak/mail/data/models/Attachment.kt

@@ -21,8 +21,10 @@ import android.content.Context
 import androidx.core.net.toFile
 import androidx.core.net.toUri
 import com.infomaniak.lib.core.utils.Utils.enumValueOfOrNull
+import com.infomaniak.mail.data.models.draft.Draft
 import com.infomaniak.mail.utils.AttachableMimeTypeUtils
 import com.infomaniak.mail.utils.LocalStorageUtils
+import com.infomaniak.mail.utils.SentryDebug
 import io.realm.kotlin.types.EmbeddedRealmObject
 import kotlinx.serialization.SerialName
 import kotlinx.serialization.Serializable
@@ -80,13 +82,14 @@ class Attachment : EmbeddedRealmObject, Attachable {
      * After uploading an Attachment, we replace the local version with the remote one.
      * The remote one doesn't know about local data, so we have to backup them.
      */
-    fun backupLocalData(oldAttachment: Attachment, uploadStatus: UploadStatus) {
+    fun backupLocalData(oldAttachment: Attachment, uploadStatus: UploadStatus, draft: Draft) {
         localUuid = oldAttachment.localUuid
         uploadLocalUri = oldAttachment.uploadLocalUri
-        setUploadStatus(uploadStatus)
+        setUploadStatus(uploadStatus, draft, "backupLocalData -> setUploadStatus")
     }
 
-    fun setUploadStatus(uploadStatus: UploadStatus) {
+    fun setUploadStatus(uploadStatus: UploadStatus, draft: Draft? = null, step: String = "") {
+        draft?.let { SentryDebug.addAttachmentsBreadcrumb(it, step) }
         _uploadStatus = uploadStatus.name
     }
 

app/src/main/java/com/infomaniak/mail/ui/newMessage/NewMessageViewModel.kt

@@ -68,11 +68,16 @@ import com.infomaniak.mail.di.MainDispatcher
 import com.infomaniak.mail.ui.main.SnackbarManager
 import com.infomaniak.mail.ui.newMessage.NewMessageEditorManager.EditorAction
 import com.infomaniak.mail.ui.newMessage.NewMessageRecipientFieldsManager.FieldType
-import com.infomaniak.mail.ui.newMessage.NewMessageViewModel.SignatureScore.*
+import com.infomaniak.mail.ui.newMessage.NewMessageViewModel.SignatureScore.EXACT_MATCH
+import com.infomaniak.mail.ui.newMessage.NewMessageViewModel.SignatureScore.EXACT_MATCH_AND_IS_DEFAULT
+import com.infomaniak.mail.ui.newMessage.NewMessageViewModel.SignatureScore.NO_MATCH
+import com.infomaniak.mail.ui.newMessage.NewMessageViewModel.SignatureScore.ONLY_EMAIL_MATCH
+import com.infomaniak.mail.ui.newMessage.NewMessageViewModel.SignatureScore.ONLY_EMAIL_MATCH_AND_IS_DEFAULT
 import com.infomaniak.mail.utils.AccountUtils
 import com.infomaniak.mail.utils.ContactUtils.arrangeMergedContacts
 import com.infomaniak.mail.utils.LocalStorageUtils
 import com.infomaniak.mail.utils.MessageBodyUtils
+import com.infomaniak.mail.utils.SentryDebug
 import com.infomaniak.mail.utils.SharedUtils
 import com.infomaniak.mail.utils.SignatureUtils
 import com.infomaniak.mail.utils.Utils
@@ -93,14 +98,14 @@ import io.realm.kotlin.ext.realmListOf
 import io.realm.kotlin.ext.toRealmList
 import io.realm.kotlin.types.RealmList
 import io.sentry.Sentry
+import javax.inject.Inject
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import org.jsoup.Jsoup
 import org.jsoup.nodes.Document
-import javax.inject.Inject
 
 @HiltViewModel
 class NewMessageViewModel @Inject constructor(
@@ -339,6 +344,8 @@ class NewMessageViewModel @Inject constructor(
         }
 
         if (mailToUri != null) handleMailTo(draft, mailToUri)
+
+        SentryDebug.addAttachmentsBreadcrumb(draft, step = "populate Draft with external mail data")
     }
 
     private fun Draft.flagRecipientsAsAutomaticallyEntered() {
@@ -747,7 +754,12 @@ class NewMessageViewModel @Inject constructor(
     fun uploadAttachmentsToServer(uiAttachments: List<Attachment>) = viewModelScope.launch(ioDispatcher) {
         val localUuid = draftLocalUuid ?: return@launch
         val localDraft = mailboxContentRealm().writeBlocking {
-            DraftController.getDraft(localUuid, realm = this)?.also { it.updateDraftAttachmentsWithLiveData(uiAttachments) }
+            DraftController.getDraft(localUuid, realm = this)?.also {
+                it.updateDraftAttachmentsWithLiveData(
+                    uiAttachments = uiAttachments,
+                    step = "observeAttachments -> uploadAttachmentsToServer",
+                )
+            }
         } ?: return@launch
 
         runCatching {
@@ -813,7 +825,10 @@ class NewMessageViewModel @Inject constructor(
         cc = ccLiveData.valueOrEmpty().toRealmList()
         bcc = bccLiveData.valueOrEmpty().toRealmList()
 
-        updateDraftAttachmentsWithLiveData(attachmentsLiveData.valueOrEmpty())
+        updateDraftAttachmentsWithLiveData(
+            uiAttachments = attachmentsLiveData.valueOrEmpty(),
+            step = "executeDraftActionWhenStopping (action = ${draftAction.name}) -> updateDraftFromLiveData",
+        )
 
         subject = subjectValue
 
@@ -848,7 +863,7 @@ class NewMessageViewModel @Inject constructor(
         }
     }
 
-    private fun Draft.updateDraftAttachmentsWithLiveData(uiAttachments: List<Attachment>) {
+    private fun Draft.updateDraftAttachmentsWithLiveData(uiAttachments: List<Attachment>, step: String) {
 
         /**
          * If :
@@ -882,6 +897,8 @@ class NewMessageViewModel @Inject constructor(
             clear()
             addAll(updatedAttachments)
         }
+
+        SentryDebug.addAttachmentsBreadcrumb(draft = this, step)
     }
 
     private fun Draft.getWholeBody(): String = uiBody.textToHtml() + (uiSignature ?: "") + (uiQuote ?: "")

app/src/main/java/com/infomaniak/mail/utils/DraftUtils.kt

@@ -49,10 +49,10 @@ private suspend fun Draft.uploadAttachments(mailbox: Mailbox, draftController: D
 
     fun getAwaitingAttachments(): List<Attachment> = attachments.filter { it.uploadStatus == UploadStatus.AWAITING }
 
-    fun setUploadStatus(attachment: Attachment, uploadStatus: UploadStatus) {
+    fun setUploadStatus(attachment: Attachment, uploadStatus: UploadStatus, step: String) {
         realm.writeBlocking {
             draftController.updateDraft(localUuid, realm = this) {
-                it.attachments.findSpecificAttachment(attachment)?.setUploadStatus(uploadStatus)
+                it.attachments.findSpecificAttachment(attachment)?.setUploadStatus(uploadStatus, draft = it, step)
             }
         }
     }
@@ -69,10 +69,10 @@ private suspend fun Draft.uploadAttachments(mailbox: Mailbox, draftController: D
 
     attachmentsToUpload.forEach { attachment ->
         runCatching {
-            setUploadStatus(attachment, UploadStatus.ONGOING)
+            setUploadStatus(attachment, UploadStatus.ONGOING, step = "before starting upload")
             attachment.startUpload(localUuid, mailbox, draftController, realm)
         }.onFailure { exception ->
-            setUploadStatus(attachment, UploadStatus.AWAITING)
+            setUploadStatus(attachment, UploadStatus.AWAITING, step = "after failing upload")
             SentryLog.d(ATTACHMENT_TAG, "${exception.message}", exception)
             if ((exception as Exception).isNetworkException()) throw ApiController.NetworkException()
             throw exception

app/src/main/java/com/infomaniak/mail/utils/SentryDebug.kt

@@ -104,11 +104,44 @@ object SentryDebug {
         )
     }
 
+    fun addAttachmentsBreadcrumb(draft: Draft, step: String) = with(draft) {
+
+        var count = 1
+        val data = mutableMapOf<String, Any>()
+
+        fun String.keyPad(): String = padStart(length = 15)
+        fun Int.countPad(): String = toString().padStart(length = 2, '0')
+        fun count(): String = "${count.countPad().also { count++ }}."
+        fun format(index: Int): String = (index + 1).countPad()
+
+        data[count() + "step".keyPad()] = step
+        data[count() + "email".keyPad()] = AccountUtils.currentMailboxEmail.toString()
+
+        data[count() + "draft".keyPad() + " - localUuid"] = localUuid
+        data[count() + "draft".keyPad() + " - remoteUuid"] = remoteUuid.toString()
+        data[count() + "draft".keyPad() + " - action"] = action?.name.toString()
+        data[count() + "draft".keyPad() + " - mode"] = when {
+            inReplyToUid != null -> "REPLY or REPLY_ALL"
+            forwardedUid != null -> "FORWARD"
+            else -> "NEW_MAIL"
+        }
+
+        data[count() + "attachments".keyPad() + " - count"] = attachments.count()
+
+        attachments.forEachIndexed { index, it ->
+            data[count() + "attachment #${format(index)}".keyPad()] =
+                "localUuid: ${it.localUuid} | uuid: ${it.uuid} | uploadLocalUri: ${it.uploadLocalUri}"
+            data[count() + "attachment #${format(index)}".keyPad()] = "uploadStatus: ${it.uploadStatus.name} | size: ${it.size}"
+        }
+
+        addInfoBreadcrumb(category = "Attachments_Situation", data = data)
+    }
+
     private fun addInfoBreadcrumb(category: String, message: String? = null, data: Map<String, Any>? = null) {
         Breadcrumb().apply {
             this.category = category
             this.message = message
-            data?.let { it.forEach { (key, value) -> this.data[key] = value } }
+            data?.let { it.forEach { (key, value) -> this.setData(key, value) } }
             this.level = SentryLevel.INFO
         }.also(Sentry::addBreadcrumb)
     }

app/src/main/java/com/infomaniak/mail/utils/extensions/AttachmentExtensions.kt

@@ -39,6 +39,7 @@ import com.infomaniak.mail.data.models.mailbox.Mailbox
 import com.infomaniak.mail.ui.main.SnackbarManager
 import com.infomaniak.mail.ui.main.thread.actions.DownloadAttachmentProgressDialogArgs
 import com.infomaniak.mail.utils.AccountUtils
+import com.infomaniak.mail.utils.SentryDebug
 import com.infomaniak.mail.utils.WorkerUtils.UploadMissingLocalFileException
 import com.infomaniak.mail.utils.extensions.AttachmentExtensions.AttachmentIntentType.OPEN_WITH
 import com.infomaniak.mail.utils.extensions.AttachmentExtensions.AttachmentIntentType.SAVE_TO_DRIVE
@@ -189,7 +190,7 @@ object AttachmentExtensions {
                 SentryLog.d(ATTACHMENT_TAG, "When removing uploaded attachment, we found (uuids to localUris): $uuidToLocalUri")
                 SentryLog.d(ATTACHMENT_TAG, "Target uploadLocalUri is: $uploadLocalUri")
 
-                remoteAttachment.backupLocalData(oldAttachment = this@updateLocalAttachment, UploadStatus.FINISHED)
+                remoteAttachment.backupLocalData(oldAttachment = this@updateLocalAttachment, UploadStatus.FINISHED, draft)
 
                 SentryLog.d(ATTACHMENT_TAG, "Uploaded attachment uuid: ${remoteAttachment.uuid}")
                 SentryLog.d(ATTACHMENT_TAG, "Uploaded attachment localUuid: ${remoteAttachment.localUuid}")
@@ -199,6 +200,8 @@ object AttachmentExtensions {
                     delete(findSpecificAttachment(attachment = this@updateLocalAttachment)!!)
                     add(remoteAttachment)
                 }
+
+                SentryDebug.addAttachmentsBreadcrumb(draft, step = "update local Attachment after success upload")
             }
         }
     }

app/src/main/java/com/infomaniak/mail/workers/DraftsActionsWorker.kt

@@ -302,34 +302,24 @@ class DraftsActionsWorker @AssistedInject constructor(
         var scheduledDate: String? = null
         var savedDraftUuid: String? = null
 
-        // TODO: Remove this whole `draft.attachments.forEach { … }` when the Attachment issue is fixed.
-        draft.attachments.forEach { attachment ->
-            if (attachment.uploadStatus != UploadStatus.FINISHED) {
-
-                Sentry.withScope { scope ->
-                    scope.setExtra("attachmentUuid", attachment.uuid)
-                    scope.setExtra("attachmentsCount", "${draft.attachments.count()}")
-                    scope.setExtra(
-                        "attachmentsUuids to attachmentsLocalUuid",
-                        "${draft.attachments.map { it.uuid to it.localUuid }}",
-                    )
-                    scope.setExtra("draftUuid", "${draft.remoteUuid}")
-                    scope.setExtra("draftLocalUuid", draft.localUuid)
-                    scope.setExtra("email", AccountUtils.currentMailboxEmail.toString())
-                    Sentry.captureMessage(
-                        "We tried to [${draft.action?.name}] a Draft, but an Attachment wasn't uploaded.",
-                        SentryLevel.ERROR,
-                    )
-                }
+        SentryDebug.addAttachmentsBreadcrumb(draft, step = "executeDraftAction (action = ${draft.action?.name.toString()})")
 
-                return DraftActionResult(
-                    realmActionOnDraft = null,
-                    scheduledDate = null,
-                    errorMessageResId = R.string.errorCorruptAttachment,
-                    savedDraftUuid = null,
-                    isSuccess = false,
-                )
-            }
+        // TODO: Remove this whole `draft.attachments.any { … }` + `addAttachmentsBreadcrumb()`
+        //  when the Attachments issue is fixed.
+        if (draft.attachments.any { it.uploadStatus != UploadStatus.FINISHED }) {
+
+            Sentry.captureMessage(
+                "We tried to [${draft.action?.name}] a Draft, but an Attachment wasn't uploaded.",
+                SentryLevel.ERROR,
+            )
+
+            return DraftActionResult(
+                realmActionOnDraft = null,
+                scheduledDate = null,
+                errorMessageResId = R.string.errorCorruptAttachment,
+                savedDraftUuid = null,
+                isSuccess = false,
+            )
         }
 
         fun executeSaveAction() = with(ApiRepository.saveDraft(mailboxUuid, draft, okHttpClient)) {