Merge pull request #2481 from woocommerce/issue/2371-remove-fatal-err…

…or-backing-up-store Remove `fatalError` in case of persistent store backup failure and add more logging to store migration
woocommerce · Jun 30, 2020 · 7807a61 · 7807a61
2 parents e19a6c4 + 4350b45
commit 7807a61
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 29 deletions.
diff --git a/Storage/Storage/CoreData/CoreDataIterativeMigrator.swift b/Storage/Storage/CoreData/CoreDataIterativeMigrator.swift
@@ -102,7 +102,16 @@ public struct CoreDataIterativeMigrator {
             debugMessages.append(migrationAttemptMessage)
             DDLogWarn(migrationAttemptMessage)
 
-            guard migrateStore(at: sourceStore, storeType: storeType, fromModel: modelFrom, toModel: modelTo, with: migrateWithModel) == true else {
+            let (success, migrateStoreError) = migrateStore(at: sourceStore,
+                                                            storeType: storeType,
+                                                            fromModel: modelFrom,
+                                                            toModel: modelTo,
+                                                            with: migrateWithModel)
+            guard success else {
+                if let migrateStoreError = migrateStoreError {
+                    let errorInfo = (migrateStoreError as NSError?)?.userInfo ?? [:]
+                    debugMessages.append("Migration error: \(migrateStoreError) [\(errorInfo)]")
+                }
                 return (false, debugMessages)
             }
         }
@@ -197,7 +206,7 @@ private extension CoreDataIterativeMigrator {
                              storeType: String,
                              fromModel: NSManagedObjectModel,
                              toModel: NSManagedObjectModel,
-                             with mappingModel: NSMappingModel) -> Bool {
+                             with mappingModel: NSMappingModel) -> (success: Bool, error: Error?) {
         let tempDestinationURL = createTemporaryFolder(at: url)
 
         // Migrate from the source model to the target model using the mapping,
@@ -212,18 +221,18 @@ private extension CoreDataIterativeMigrator {
                                       destinationType: storeType,
                                       destinationOptions: nil)
         } catch {
-            return false
+            return (false, error)
         }
 
         do {
             let backupURL = try makeBackup(at: url)
             try copyMigratedOverOriginal(from: tempDestinationURL, to: url)
             try deleteBackupCopies(at: backupURL)
         } catch {
-            return false
+            return (false, error)
         }
 
-        return true
+        return (true, nil)
     }
 
     static func metadataForPersistentStore(storeType: String, at url: URL) throws -> [String: Any]? {

diff --git a/Storage/Storage/CoreData/CoreDataManager.swift b/Storage/Storage/CoreData/CoreDataManager.swift
@@ -48,34 +48,22 @@ public class CoreDataManager: StorageManagerType {
 
             /// Backup the old Store
             ///
+            var persistentStoreBackupError: Error?
             do {
                 let sourceURL = self.storeURL
                 let backupURL = sourceURL.appendingPathExtension("~")
                 try FileManager.default.copyItem(at: sourceURL, to: backupURL)
             } catch {
-                let message = "☠️ [CoreDataManager] Cannot backup Store: \(error)"
-                self.crashLogger.logMessageAndWait(message,
-                                                   properties: ["persistentStoreLoadingError": persistentStoreLoadingError,
-                                                                "backupError": error,
-                                                                "appState": UIApplication.shared.applicationState.rawValue,
-                                                                "migrationMessages": migrationDebugMessages],
-                                                   level: .fatal)
-                fatalError(message)
+                persistentStoreBackupError = error
             }
 
             /// Remove the old Store
             ///
+            var persistentStoreRemovalError: Error?
             do {
                 try FileManager.default.removeItem(at: self.storeURL)
             } catch {
-                let message = "☠️ [CoreDataManager] Cannot remove Store: \(error)"
-                self.crashLogger.logMessageAndWait(message,
-                                                   properties: ["persistentStoreLoadingError": persistentStoreLoadingError,
-                                                                "removeStoreError": error,
-                                                                "appState": UIApplication.shared.applicationState.rawValue,
-                                                                "migrationMessages": migrationDebugMessages],
-                                                   level: .fatal)
-                fatalError(message)
+                persistentStoreRemovalError = error
             }
 
             /// Retry!
@@ -85,20 +73,27 @@ public class CoreDataManager: StorageManagerType {
                     return
                 }
 
-                let message = "☠️ [CoreDataManager] Recovery Failed! \(error) [\(error.userInfo)]"
+                let message = "☠️ [CoreDataManager] Recovery Failed!"
+
+                let logProperties: [String: Any?] = ["persistentStoreLoadingError": persistentStoreLoadingError,
+                                                     "persistentStoreBackupError": persistentStoreBackupError,
+                                                     "persistentStoreRemovalError": persistentStoreRemovalError,
+                                                     "retryError": error,
+                                                     "appState": UIApplication.shared.applicationState.rawValue,
+                                                     "migrationMessages": migrationDebugMessages]
                 self?.crashLogger.logMessageAndWait(message,
-                                                    properties: ["persistentStoreLoadingError": persistentStoreLoadingError,
-                                                                 "retryError": error,
-                                                                 "appState": UIApplication.shared.applicationState.rawValue,
-                                                                 "migrationMessages": migrationDebugMessages],
+                                                    properties: logProperties.compactMapValues { $0 },
                                                     level: .fatal)
                 fatalError(message)
             }
 
+            let logProperties: [String: Any?] = ["persistentStoreLoadingError": persistentStoreLoadingError,
+                                                 "persistentStoreBackupError": persistentStoreBackupError,
+                                                 "persistentStoreRemovalError": persistentStoreRemovalError,
+                                                 "appState": UIApplication.shared.applicationState.rawValue,
+                                                 "migrationMessages": migrationDebugMessages]
             self.crashLogger.logMessage("[CoreDataManager] Recovered from persistent store loading error",
-                                        properties: ["persistentStoreLoadingError": persistentStoreLoadingError,
-                                                     "appState": UIApplication.shared.applicationState.rawValue,
-                                                     "migrationMessages": migrationDebugMessages],
+                                        properties: logProperties.compactMapValues { $0 },
                                         level: .info)
         }