From f701ee4fc61ff395339f741a7bb227c2f49c642a Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Tue, 26 Sep 2023 10:27:40 +0200 Subject: [PATCH 1/9] refactor: Extract fetch messageUidsWithBackOff --- .../Cache/MailboxManager/MailboxManager+Message.swift | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift index e191bb60f..d685ca6bd 100644 --- a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift +++ b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift @@ -118,7 +118,7 @@ public extension MailboxManager { } } - func fetchOnePage(folder: Folder, direction: NewMessagesDirection? = nil) async throws -> Bool { + func messageUidsWithBackOff(folder: Folder, direction: NewMessagesDirection? = nil) async throws -> MessageUidsResult { let realm = getRealm() var paginationInfo: PaginationInfo? @@ -144,6 +144,13 @@ public extension MailboxManager { folderId: folder.id, paginationInfo: paginationInfo ) + + return messageUidsResult + } + + func fetchOnePage(folder: Folder, direction: NewMessagesDirection? = nil) async throws -> Bool { + let messageUidsResult = try await messageUidsWithBackOff(folder: folder, direction: direction) + let messagesUids = MessagesUids( addedShortUids: messageUidsResult.messageShortUids, cursor: messageUidsResult.cursor @@ -151,7 +158,7 @@ public extension MailboxManager { try await handleMessagesUids(messageUids: messagesUids, folder: folder) - switch paginationInfo?.direction { + switch direction { case .previous: return await backgroundRealm.execute { realm in let freshFolder = folder.fresh(using: realm) From f9c2efe998521b828d2f432cdac27202ed9d2779 Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Tue, 26 Sep 2023 13:02:43 +0200 Subject: [PATCH 2/9] refactor: Move to SentryDebug listIncoherentMessageUpdate --- .../MailboxManager+Message.swift | 22 +---------------- MailCore/Utils/SentryDebug.swift | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift index d685ca6bd..dd674632f 100644 --- a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift +++ b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift @@ -488,27 +488,7 @@ public extension MailboxManager { try await refreshFolder(from: messages) // TODO: Remove after fix - Task { - for message in messages { - if let liveMessage = message.thaw(), - liveMessage.seen != seen { - SentrySDK.capture(message: "Found incoherent message update") { scope in - scope.setContext(value: ["Message": ["uid": message.uid, - "messageId": message.messageId, - "date": message.date, - "seen": message.seen, - "duplicates": message.duplicates.compactMap(\.messageId), - "references": message.references], - "Seen": ["Expected": seen, "Actual": liveMessage.seen], - "Folder": ["id": message.folder?._id, - "name": message.folder?.name, - "last update": message.folder?.lastUpdate, - "cursor": message.folder?.cursor]], - key: "Message context") - } - } - } - } + SentryDebug.listIncoherentMessageUpdate(messages: messages, actualSeen: seen) } /// Set starred the given messages. diff --git a/MailCore/Utils/SentryDebug.swift b/MailCore/Utils/SentryDebug.swift index 530c82ed1..e461f5be0 100644 --- a/MailCore/Utils/SentryDebug.swift +++ b/MailCore/Utils/SentryDebug.swift @@ -156,4 +156,28 @@ public enum SentryDebug { data: ["uid": uid]) SentrySDK.addBreadcrumb(breadcrumb) } + + static func listIncoherentMessageUpdate(messages: [Message], actualSeen: Bool) { + Task { + for message in messages { + if let liveMessage = message.thaw(), + liveMessage.seen != actualSeen { + SentrySDK.capture(message: "Found incoherent message update") { scope in + scope.setContext(value: ["Message": ["uid": message.uid, + "messageId": message.messageId, + "date": message.date, + "seen": message.seen, + "duplicates": message.duplicates.compactMap(\.messageId), + "references": message.references], + "Seen": ["Expected": actualSeen, "Actual": liveMessage.seen], + "Folder": ["id": message.folder?._id, + "name": message.folder?.name, + "last update": message.folder?.lastUpdate, + "cursor": message.folder?.cursor]], + key: "Message context") + } + } + } + } + } } From 112dac1473b855119ab038073f0d9f35710bad33 Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Wed, 27 Sep 2023 08:01:55 +0200 Subject: [PATCH 3/9] feat: messageUidsWithBackOff add recursivity --- MailCore/API/MailError.swift | 1 + .../MailboxManager+Message.swift | 41 +++++++++++++++---- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/MailCore/API/MailError.swift b/MailCore/API/MailError.swift index d60375597..a6ef7da8a 100644 --- a/MailCore/API/MailError.swift +++ b/MailCore/API/MailError.swift @@ -74,6 +74,7 @@ public class MailError: LocalizedError { public static let noConnection = MailError(code: "noConnection", localizedDescription: MailResourcesStrings.Localizable.noConnection, shouldDisplay: true) + static let lostOffsetMessage = MailError(code: "lostOffsetMessage") /// After an update from the server we are still without a default signature public static let defaultSignatureMissing = MailError(code: "defaultSignatureMissing") diff --git a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift index dd674632f..090cccdc9 100644 --- a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift +++ b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift @@ -118,7 +118,14 @@ public extension MailboxManager { } } - func messageUidsWithBackOff(folder: Folder, direction: NewMessagesDirection? = nil) async throws -> MessageUidsResult { + func messageUidsWithBackOff(folder: Folder, + direction: NewMessagesDirection? = nil, + backoffIndex: Int = 0) async throws -> MessageUidsResult { + let backoffSequence = [1, 1, 2, 8, 34, 144] + guard backoffIndex < backoffSequence.count else { + throw MailError.lostOffsetMessage + } + let realm = getRealm() var paginationInfo: PaginationInfo? @@ -134,18 +141,34 @@ public extension MailboxManager { return firstMessageShortUid > secondMessageShortUid } return firstMessageShortUid < secondMessageShortUid - }).first?.shortUid?.toString(), - let direction { + } + + let backoffOffset = backoffSequence[backoffIndex] - 1 + let currentOffset = min(backoffOffset, sortedMessages.count - 1) + + if currentOffset >= 0, + let offset = sortedMessages[currentOffset].shortUid?.toString(), + let direction { paginationInfo = PaginationInfo(offsetUid: offset, direction: direction) } - let messageUidsResult = try await apiFetcher.messagesUids( - mailboxUuid: mailbox.uuid, - folderId: folder.id, - paginationInfo: paginationInfo - ) + do { + let result = try await apiFetcher.messagesUids( + mailboxUuid: mailbox.uuid, + folderId: folder.id, + paginationInfo: paginationInfo + ) + return result + } catch let error as MailError where error == MailApiError.apiMessageNotFound { + try await Task.sleep(nanoseconds: UInt64(0.5 * Double(NSEC_PER_SEC))) + let result = try await messageUidsWithBackOff( + folder: folder, + direction: direction, + backoffIndex: backoffIndex + 1 + ) - return messageUidsResult + return result + } } func fetchOnePage(folder: Folder, direction: NewMessagesDirection? = nil) async throws -> Bool { From b3000aca8e21ebf298ca1ad2214a3c4cacfedd64 Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Wed, 27 Sep 2023 08:07:09 +0200 Subject: [PATCH 4/9] refactor: guard instead of if let --- .../MailboxManager/MailboxManager+Message.swift | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift index 090cccdc9..0d6adf19e 100644 --- a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift +++ b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift @@ -104,17 +104,16 @@ public extension MailboxManager { } let realmPrevious = getRealm() - if let folderPrevious = folder.fresh(using: realmPrevious) { - var remainingOldMessagesToFetch = folderPrevious.remainingOldMessagesToFetch - while remainingOldMessagesToFetch > 0 { - guard !Task.isCancelled else { return } - - if try await !fetchOnePage(folder: folder, direction: .previous) { - break - } + guard let folderPrevious = folder.fresh(using: realmPrevious) else { return } + var remainingOldMessagesToFetch = folderPrevious.remainingOldMessagesToFetch + while remainingOldMessagesToFetch > 0 { + guard !Task.isCancelled else { return } - remainingOldMessagesToFetch -= Constants.pageSize + if try await !fetchOnePage(folder: folder, direction: .previous) { + break } + + remainingOldMessagesToFetch -= Constants.pageSize } } From e94277a097dc0b4ec80fd33131af9ea35c6d332b Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Wed, 27 Sep 2023 10:13:56 +0200 Subject: [PATCH 5/9] feat: Rebuild from scratch after failed backoff --- .../MailboxManager/MailboxManageable.swift | 2 +- .../MailboxManager+Message.swift | 34 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/MailCore/Cache/MailboxManager/MailboxManageable.swift b/MailCore/Cache/MailboxManager/MailboxManageable.swift index 76df8f560..008ad88fa 100644 --- a/MailCore/Cache/MailboxManager/MailboxManageable.swift +++ b/MailCore/Cache/MailboxManager/MailboxManageable.swift @@ -24,7 +24,7 @@ public typealias MailboxManageable = MailBoxManagerDraftable & MailBoxManagerMes /// An abstract interface on the `MailboxManager` related to messages public protocol MailBoxManagerMessageable { - func messages(folder: Folder) async throws + func messages(folder: Folder, isRetrying: Bool) async throws func fetchOnePage(folder: Folder, direction: NewMessagesDirection?) async throws -> Bool func message(message: Message) async throws func attachmentData(attachment: Attachment) async throws -> Data diff --git a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift index 0d6adf19e..f484eb8a3 100644 --- a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift +++ b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift @@ -16,6 +16,7 @@ along with this program. If not, see . */ +import CocoaLumberjackSwift import Foundation import InfomaniakCoreUI import InfomaniakDI @@ -26,7 +27,7 @@ import Sentry // MARK: - Message public extension MailboxManager { - func messages(folder: Folder) async throws { + func messages(folder: Folder, isRetrying: Bool = false) async throws { guard !Task.isCancelled else { return } let realm = getRealm() @@ -93,8 +94,10 @@ public extension MailboxManager { } if previousCursor != nil { - while try await fetchOnePage(folder: folder, direction: .following) { - guard !Task.isCancelled else { return } + try await catchLostOffsetMessageError(folder: folder, isRetrying: isRetrying) { + while try await fetchOnePage(folder: folder, direction: .following) { + guard !Task.isCancelled else { return } + } } } @@ -117,6 +120,31 @@ public extension MailboxManager { } } + private func catchLostOffsetMessageError(folder: Folder, isRetrying: Bool, block: () async throws -> Void) async throws { + do { + try await block() + } catch let error as MailError where error == MailApiError.lostOffsetMessage { + guard !isRetrying else { + DDLogError("We couldn't rebuild folder history even after retrying from scratch") + throw MailError.unknownError + } + + DDLogWarn("resetHistoryInfo because of lostOffsetMessageError") + await backgroundRealm.execute { realm in + guard let folder = folder.fresh(using: realm) else { return } + + try? realm.write { + folder.lastUpdate = nil + folder.unreadCount = 0 + folder.remainingOldMessagesToFetch = Constants.messageQuantityLimit + folder.isHistoryComplete = false + folder.cursor = nil + } + } + try await messages(folder: folder, isRetrying: true) + } + } + func messageUidsWithBackOff(folder: Folder, direction: NewMessagesDirection? = nil, backoffIndex: Int = 0) async throws -> MessageUidsResult { From 9978a014204a16f45dd76efd3db690b586136b7a Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Wed, 27 Sep 2023 10:33:48 +0200 Subject: [PATCH 6/9] feat: Add Sentry debugging --- .../MailboxManager+Message.swift | 5 ++ MailCore/Utils/SentryDebug.swift | 55 +++++++++++++------ 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift index f484eb8a3..a5741443e 100644 --- a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift +++ b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift @@ -126,10 +126,13 @@ public extension MailboxManager { } catch let error as MailError where error == MailApiError.lostOffsetMessage { guard !isRetrying else { DDLogError("We couldn't rebuild folder history even after retrying from scratch") + SentryDebug.failedResetingAfterBackoff(folder: folder) throw MailError.unknownError } DDLogWarn("resetHistoryInfo because of lostOffsetMessageError") + SentryDebug.addResetingFolderBreadcrumb(folder: folder) + await backgroundRealm.execute { realm in guard let folder = folder.fresh(using: realm) else { return } @@ -153,6 +156,8 @@ public extension MailboxManager { throw MailError.lostOffsetMessage } + SentryDebug.addBackoffBreadcrumb(folder: folder, index: backoffIndex) + let realm = getRealm() var paginationInfo: PaginationInfo? diff --git a/MailCore/Utils/SentryDebug.swift b/MailCore/Utils/SentryDebug.swift index e461f5be0..2d9a43294 100644 --- a/MailCore/Utils/SentryDebug.swift +++ b/MailCore/Utils/SentryDebug.swift @@ -160,24 +160,47 @@ public enum SentryDebug { static func listIncoherentMessageUpdate(messages: [Message], actualSeen: Bool) { Task { for message in messages { - if let liveMessage = message.thaw(), - liveMessage.seen != actualSeen { - SentrySDK.capture(message: "Found incoherent message update") { scope in - scope.setContext(value: ["Message": ["uid": message.uid, - "messageId": message.messageId, - "date": message.date, - "seen": message.seen, - "duplicates": message.duplicates.compactMap(\.messageId), - "references": message.references], - "Seen": ["Expected": actualSeen, "Actual": liveMessage.seen], - "Folder": ["id": message.folder?._id, - "name": message.folder?.name, - "last update": message.folder?.lastUpdate, - "cursor": message.folder?.cursor]], - key: "Message context") - } + guard let liveMessage = message.thaw(), + liveMessage.seen != actualSeen else { continue } + + SentrySDK.capture(message: "Found incoherent message update") { scope in + scope.setContext(value: ["Message": ["uid": message.uid, + "messageId": message.messageId ?? "nil", + "date": message.date, + "seen": message.seen, + "duplicates": message.duplicates.compactMap(\.messageId), + "references": message.references ?? "nil"], + "Seen": ["Expected": actualSeen, "Actual": liveMessage.seen], + "Folder": ["id": message.folder?._id ?? "nil", + "name": message.folder?.name ?? "nil", + "last update": message.folder?.lastUpdate, + "cursor": message.folder?.cursor ?? "nil"]], + key: "Message context") } } } } + + static func addBackoffBreadcrumb(folder: Folder, index: Int) { + let breadcrumb = Breadcrumb() + breadcrumb.message = "Backoff \(index) for folder \(folder.name) - \(folder.id)" + breadcrumb.level = .warning + breadcrumb.type = "debug" + SentrySDK.addBreadcrumb(breadcrumb) + } + + static func addResetingFolderBreadcrumb(folder: Folder) { + let breadcrumb = Breadcrumb() + breadcrumb.message = "Reseting folder after failed backoff \(folder.name) - \(folder.id)" + breadcrumb.level = .warning + breadcrumb.type = "debug" + SentrySDK.addBreadcrumb(breadcrumb) + } + + static func failedResetingAfterBackoff(folder: Folder) { + SentrySDK.capture(message: "Failed reseting folder after backoff") { scope in + scope.setContext(value: ["Folder": ["Id": folder.id, "name": folder.name]], + key: "Folder context") + } + } } From 18b93d6d33469d4103eb7037ab75d92df828c3c1 Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Thu, 28 Sep 2023 11:04:00 +0200 Subject: [PATCH 7/9] fix: Delete threads and messages when reseting folder --- MailCore/Cache/MailboxManager/MailboxManager+Message.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift index a5741443e..d74862f68 100644 --- a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift +++ b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift @@ -137,6 +137,8 @@ public extension MailboxManager { guard let folder = folder.fresh(using: realm) else { return } try? realm.write { + realm.delete(folder.messages) + realm.delete(folder.threads) folder.lastUpdate = nil folder.unreadCount = 0 folder.remainingOldMessagesToFetch = Constants.messageQuantityLimit From f3a8a7ddefcbeb24514ff677a6d5b7b6bf1e2f44 Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Thu, 28 Sep 2023 14:28:14 +0200 Subject: [PATCH 8/9] fix: Prevent wasting calls --- .../Cache/MailboxManager/MailboxManager+Message.swift | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift index d74862f68..a41dc1b0a 100644 --- a/MailCore/Cache/MailboxManager/MailboxManager+Message.swift +++ b/MailCore/Cache/MailboxManager/MailboxManager+Message.swift @@ -163,8 +163,8 @@ public extension MailboxManager { let realm = getRealm() var paginationInfo: PaginationInfo? - if let offset = realm.objects(Message.self).where({ $0.folderId == folder.id && $0.fromSearch == false }) - .sorted(by: { + let sortedMessages = realm.objects(Message.self).where { $0.folderId == folder.id } + .sorted { guard let firstMessageShortUid = $0.shortUid, let secondMessageShortUid = $1.shortUid else { SentryDebug.castToShortUidFailed(firstUid: $0.uid, secondUid: $1.uid) @@ -180,6 +180,11 @@ public extension MailboxManager { let backoffOffset = backoffSequence[backoffIndex] - 1 let currentOffset = min(backoffOffset, sortedMessages.count - 1) + // We already did one call and last call was already above sortedMessages.count so we stop wasting more calls + if backoffIndex > 0 && backoffSequence[backoffIndex - 1] - 1 > sortedMessages.count - 1 { + throw MailError.lostOffsetMessage + } + if currentOffset >= 0, let offset = sortedMessages[currentOffset].shortUid?.toString(), let direction { From 407e572bc7d839d2f042756ef9a434099841b44a Mon Sep 17 00:00:00 2001 From: Philippe Weidmann Date: Tue, 3 Oct 2023 08:47:55 +0200 Subject: [PATCH 9/9] chore: Update core ref --- .package.resolved | 2 +- Project.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.package.resolved b/.package.resolved index 9f1262c33..45102423a 100644 --- a/.package.resolved +++ b/.package.resolved @@ -41,7 +41,7 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/Infomaniak/ios-core", "state" : { - "revision" : "025c5dffa801d0ffe4e55730897466784f42564f" + "revision" : "1c741dfd8096fcd0c83200d958917d246ad9e6f2" } }, { diff --git a/Project.swift b/Project.swift index 8bf073e4f..cf3687c44 100644 --- a/Project.swift +++ b/Project.swift @@ -26,7 +26,7 @@ let project = Project(name: "Mail", .package(url: "https://github.com/Infomaniak/ios-dependency-injection", .upToNextMajor(from: "1.1.6")), .package( url: "https://github.com/Infomaniak/ios-core", - .revision("025c5dffa801d0ffe4e55730897466784f42564f") + .revision("1c741dfd8096fcd0c83200d958917d246ad9e6f2") ), .package(url: "https://github.com/Infomaniak/ios-core-ui", .upToNextMajor(from: "2.5.3")), .package(url: "https://github.com/Infomaniak/ios-notifications", .upToNextMajor(from: "3.0.0")),