From bc333911b8ecef447dc51048fe6d057e8f294fd0 Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Wed, 21 Jun 2023 09:36:12 +0200 Subject: [PATCH 1/4] Revert "Merge pull request #819 from Infomaniak/revert-indexed" This reverts commit b602194d995ebe4ab1c5f74623d56bb2a7adc048, reversing changes made to d04f7a292c698805648f405fbe6728a7de41579c. --- MailCore/Cache/MailboxManager.swift | 2 +- MailCore/Models/Thread.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/MailCore/Cache/MailboxManager.swift b/MailCore/Cache/MailboxManager.swift index efabebae5..ca079bfde 100644 --- a/MailCore/Cache/MailboxManager.swift +++ b/MailCore/Cache/MailboxManager.swift @@ -72,7 +72,7 @@ public class MailboxManager: ObservableObject { let realmName = "\(mailbox.userId)-\(mailbox.mailboxId).realm" realmConfiguration = Realm.Configuration( fileURL: MailboxManager.constants.rootDocumentsURL.appendingPathComponent(realmName), - schemaVersion: 11, + schemaVersion: 12, deleteRealmIfMigrationNeeded: true, objectTypes: [ Folder.self, diff --git a/MailCore/Models/Thread.swift b/MailCore/Models/Thread.swift index 87623f3ca..707627c1a 100644 --- a/MailCore/Models/Thread.swift +++ b/MailCore/Models/Thread.swift @@ -43,7 +43,7 @@ public class Thread: Object, Decodable, Identifiable { @Persisted public var cc: List @Persisted public var bcc: List @Persisted public var subject: String? - @Persisted public var date: Date + @Persisted(indexed: true) public var date: Date @Persisted public var hasAttachments: Bool @Persisted public var hasSwissTransferAttachments: Bool @Persisted public var hasDrafts: Bool From a27408d861915aa1e3522be1204ddcd2a160bb64 Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Wed, 21 Jun 2023 11:09:08 +0200 Subject: [PATCH 2/4] refactor: Move sentry calls to SentryDebug --- MailCore/Cache/MailboxManager.swift | 20 +++----------------- MailCore/Utils/SentryDebug.swift | 13 ++++++++++++- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/MailCore/Cache/MailboxManager.swift b/MailCore/Cache/MailboxManager.swift index ca079bfde..fb86ce826 100644 --- a/MailCore/Cache/MailboxManager.swift +++ b/MailCore/Cache/MailboxManager.swift @@ -302,14 +302,7 @@ public class MailboxManager: ObservableObject { try thread.recomputeOrFail() } catch { threadsToDelete.insert(thread) - SentrySDK.capture(message: "Thread has nil lastMessageFromFolderDate") { scope in - scope.setContext(value: ["dates": "\(thread.messages.map { $0.date })", - "ids": "\(thread.messages.map { $0.id })"], - key: "all messages") - scope.setContext(value: ["id": "\(thread.lastMessageFromFolder?.uid ?? "nil")"], - key: "lastMessageFromFolder") - scope.setContext(value: ["date before error": thread.date], key: "thread") - } + SentryDebug.threadHasNilLastMessageFromFolderDate(thread: thread) } } } @@ -892,16 +885,9 @@ public class MailboxManager: ObservableObject { let folders = Set(threads.compactMap(\.folder)) for thread in threads { do { - try thread.recomputeOrFail() + try thread.recomputeOrFail() } catch { - SentrySDK.capture(message: "Thread has nil lastMessageFromFolderDate") { scope in - scope.setContext(value: ["dates": "\(thread.messages.map { $0.date })", - "ids": "\(thread.messages.map { $0.id })"], - key: "all messages") - scope.setContext(value: ["id": "\(thread.lastMessageFromFolder?.uid ?? "nil")"], - key: "lastMessageFromFolder") - scope.setContext(value: ["date before error": thread.date], key: "thread") - } + SentryDebug.threadHasNilLastMessageFromFolderDate(thread: thread) realm.delete(thread) } } diff --git a/MailCore/Utils/SentryDebug.swift b/MailCore/Utils/SentryDebug.swift index aeeee5fd4..6d91699f4 100644 --- a/MailCore/Utils/SentryDebug.swift +++ b/MailCore/Utils/SentryDebug.swift @@ -20,7 +20,7 @@ import Foundation import RealmSwift import Sentry -struct SentryDebug { +enum SentryDebug { static func sendMissingMessagesSentry(sentUids: [String], receivedMessages: [Message], folder: Folder, newCursor: String?) { if receivedMessages.count != sentUids.count { let receivedUids = Set(receivedMessages.map { Constants.shortUid(from: $0.uid) }) @@ -72,4 +72,15 @@ struct SentryDebug { } } } + + static func threadHasNilLastMessageFromFolderDate(thread: Thread) { + SentrySDK.capture(message: "Thread has nil lastMessageFromFolderDate") { scope in + scope.setContext(value: ["dates": "\(thread.messages.map { $0.date })", + "ids": "\(thread.messages.map { $0.id })"], + key: "all messages") + scope.setContext(value: ["id": "\(thread.lastMessageFromFolder?.uid ?? "nil")"], + key: "lastMessageFromFolder") + scope.setContext(value: ["date before error": thread.date], key: "thread") + } + } } From 59d971dce9fc301cadd0ce7781b30acf4f4401a9 Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Wed, 21 Jun 2023 11:10:18 +0200 Subject: [PATCH 3/4] refactor: Ensure PreviewHelper is only in debug --- Mail/Helpers/PreviewHelper.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Mail/Helpers/PreviewHelper.swift b/Mail/Helpers/PreviewHelper.swift index 3e21ea20d..630247935 100644 --- a/Mail/Helpers/PreviewHelper.swift +++ b/Mail/Helpers/PreviewHelper.swift @@ -24,7 +24,8 @@ import MailCore import RealmSwift import SwiftUI -struct PreviewHelper { +#if DEBUG +enum PreviewHelper { static let sampleMailboxManager = MailboxManager(mailbox: sampleMailbox, apiFetcher: MailApiFetcher()) static let sampleMailbox = Mailbox(uuid: "", @@ -132,3 +133,4 @@ struct PreviewHelper { expirationDate: Date() )) } +#endif From 2f15ed68283acde08566a27290a56352392c934f Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Wed, 21 Jun 2023 13:27:49 +0200 Subject: [PATCH 4/4] feat: Add sentry for each step --- MailCore/Cache/MailboxManager.swift | 29 ++++++++++++++++++++-- MailCore/Models/Message.swift | 24 ++++++++++++++----- MailCore/Utils/SentryDebug.swift | 37 ++++++++++++++++++++++++++++- 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/MailCore/Cache/MailboxManager.swift b/MailCore/Cache/MailboxManager.swift index fb86ce826..6a482537d 100644 --- a/MailCore/Cache/MailboxManager.swift +++ b/MailCore/Cache/MailboxManager.swift @@ -266,7 +266,7 @@ public class MailboxManager: ObservableObject { } } - private func deleteMessages(uids: [String], folder: Folder) async { + private func deleteMessages(uids: [String]) async { guard !uids.isEmpty && !Task.isCancelled else { return } await backgroundRealm.execute { realm in @@ -756,9 +756,34 @@ public class MailboxManager: ObservableObject { } private func handleMessagesUids(messageUids: MessagesUids, folder: Folder) async throws { - await deleteMessages(uids: messageUids.deletedUids, folder: folder) + let alreadyWrongIds = folder.fresh(using: getRealm())?.threads + .where { $0.date == SentryDebug.knownDebugDate } + .map { $0.uid } ?? [] + await deleteMessages(uids: messageUids.deletedUids) + var shouldIgnoreNextEvents = SentryDebug.captureWrongDate( + step: "After delete", + folder: folder, + alreadyWrongIds: alreadyWrongIds, + realm: getRealm() + ) await updateMessages(updates: messageUids.updated, folder: folder) + if !shouldIgnoreNextEvents { + shouldIgnoreNextEvents = SentryDebug.captureWrongDate( + step: "After updateMessages", + folder: folder, + alreadyWrongIds: alreadyWrongIds, + realm: getRealm() + ) + } try await addMessages(shortUids: messageUids.addedShortUids, folder: folder, newCursor: messageUids.cursor) + if !shouldIgnoreNextEvents { + _ = SentryDebug.captureWrongDate( + step: "After addMessages", + folder: folder, + alreadyWrongIds: alreadyWrongIds, + realm: getRealm() + ) + } } private func addMessages(shortUids: [String], folder: Folder, newCursor: String?) async throws { diff --git a/MailCore/Models/Message.swift b/MailCore/Models/Message.swift index 8b5d48232..556078e5f 100644 --- a/MailCore/Models/Message.swift +++ b/MailCore/Models/Message.swift @@ -22,7 +22,7 @@ import MailResources import RealmSwift import Sentry -// TODO move to core +// TODO: move to core public extension String { /// Max length of a string before we need to truncate it. static let closeToMaxRealmSize = 14_000_000 @@ -33,7 +33,7 @@ public extension String { var truncatedForRealmIfNeeded: Self { Self.truncatedForRealmIfNeeded(self) } - + /// Truncate a string for compatibility with Realm if needed /// /// The string will be terminated by " [truncated]" if it was @@ -303,14 +303,27 @@ public final class Message: Object, Decodable, Identifiable { public required init(from decoder: Decoder) throws { let values = try decoder.container(keyedBy: CodingKeys.self) - uid = try values.decode(String.self, forKey: .uid) + let uid = try values.decode(String.self, forKey: .uid) + self.uid = uid if let msgId = try? values.decode(String.self, forKey: .messageId) { messageId = msgId linkedUids = [msgId].toRealmSet() } subject = try values.decodeIfPresent(String.self, forKey: .subject) priority = try values.decode(MessagePriority.self, forKey: .priority) - date = (try? values.decode(Date.self, forKey: .date)) ?? Date() + if let date = (try? values.decode(Date.self, forKey: .date)) { + self.date = date + } else { + // FIXME: Remove after thread date bug fix + date = SentryDebug.knownDebugDate + SentrySDK + .addBreadcrumb(SentryDebug.createBreadcrumb( + level: .warning, + category: "Thread algo", + message: "Nil message date decoded", + data: ["uid": uid] + )) + } size = try values.decode(Int.self, forKey: .size) from = try values.decode(List.self, forKey: .from) to = try values.decode(List.self, forKey: .to) @@ -454,10 +467,9 @@ final class ProxyBody: Codable { /// Generate a new persisted realm object on the fly public func realmObject() -> Body { - // truncate message if needed let truncatedValue = value?.truncatedForRealmIfNeeded - + let body = Body() body.value = truncatedValue body.type = type diff --git a/MailCore/Utils/SentryDebug.swift b/MailCore/Utils/SentryDebug.swift index 6d91699f4..beee39c98 100644 --- a/MailCore/Utils/SentryDebug.swift +++ b/MailCore/Utils/SentryDebug.swift @@ -21,6 +21,7 @@ import RealmSwift import Sentry enum SentryDebug { + static let knownDebugDate = Date(timeIntervalSince1970: 1_893_456_000) static func sendMissingMessagesSentry(sentUids: [String], receivedMessages: [Message], folder: Folder, newCursor: String?) { if receivedMessages.count != sentUids.count { let receivedUids = Set(receivedMessages.map { Constants.shortUid(from: $0.uid) }) @@ -72,7 +73,7 @@ enum SentryDebug { } } } - + static func threadHasNilLastMessageFromFolderDate(thread: Thread) { SentrySDK.capture(message: "Thread has nil lastMessageFromFolderDate") { scope in scope.setContext(value: ["dates": "\(thread.messages.map { $0.date })", @@ -83,4 +84,38 @@ enum SentryDebug { scope.setContext(value: ["date before error": thread.date], key: "thread") } } + + static func createBreadcrumb(level: SentryLevel, + category: String, + message: String, + data: [String: Any]? = nil) -> Breadcrumb { + let crumb = Breadcrumb(level: level, category: category) + crumb.type = level == .info ? "info" : "error" + crumb.message = message + crumb.data = data + return crumb + } + + static func captureWrongDate(step: String, folder: Folder, alreadyWrongIds: [String], realm: Realm) -> Bool { + guard let freshFolder = folder.fresh(using: realm) else { return false } + + let threads = freshFolder.threads.where { $0.date == knownDebugDate }.filter { !alreadyWrongIds.contains($0.uid) } + guard !threads.isEmpty else { return false } + + SentrySDK.capture(message: "Threads with wrong date on step \(step)") { scope in + scope.setLevel(.error) + scope.setContext(value: ["threads": Array(threads).map { + [ + "uid": "\($0.uid)", + "subject": $0.subject ?? "No subject", + "messageIds": "\($0.messageIds.joined(separator: ","))", + "lastMessageFromFolder": $0.lastMessageFromFolder?.uid ?? "nil", + "messages": Array($0.messages) + .map { ["message uid": $0.uid, "message subject": $0.subject ?? "No subject", "message date": $0.date] } + ] + }], + key: "threads") + } + return true + } }