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

Add more breadcrumbs to better understand Attachments issues #1982

Merged
merged 5 commits into from
Jul 29, 2024
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 @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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 :
Expand Down Expand Up @@ -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 ?: "")
Expand Down
8 changes: 4 additions & 4 deletions app/src/main/java/com/infomaniak/mail/utils/DraftUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand All @@ -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
Expand Down
35 changes: 34 additions & 1 deletion app/src/main/java/com/infomaniak/mail/utils/SentryDebug.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}")
Expand All @@ -199,6 +200,8 @@ object AttachmentExtensions {
delete(findSpecificAttachment(attachment = this@updateLocalAttachment)!!)
add(remoteAttachment)
}

SentryDebug.addAttachmentsBreadcrumb(draft, step = "update local Attachment after success upload")
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Loading