Log messages/crashes in CoreDataManager in Storage framework

Storage/Storage.xcodeproj/project.pbxproj

@@ -17,9 +17,11 @@
 		028296F3237D404F00E84012 /* ProductVariation+CoreDataProperties.swift in Sources */ = {isa = PBXBuildFile; fileRef = 028296EF237D404F00E84012 /* ProductVariation+CoreDataProperties.swift */; };
 		028296F4237D404F00E84012 /* Attribute+CoreDataClass.swift in Sources */ = {isa = PBXBuildFile; fileRef = 028296F0237D404F00E84012 /* Attribute+CoreDataClass.swift */; };
 		028296F5237D404F00E84012 /* Attribute+CoreDataProperties.swift in Sources */ = {isa = PBXBuildFile; fileRef = 028296F1237D404F00E84012 /* Attribute+CoreDataProperties.swift */; };
+		02A098272480D160002F8C7A /* MockCrashLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02A098262480D160002F8C7A /* MockCrashLogger.swift */; };
 		02D45649231CFB27008CF0A9 /* StatsVersionBannerVisibility.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02D45648231CFB27008CF0A9 /* StatsVersionBannerVisibility.swift */; };
 		02DA64172313C26400284168 /* StatsVersionBySite.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02DA64162313C26400284168 /* StatsVersionBySite.swift */; };
 		02DA64192313C2AA00284168 /* StatsVersion.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02DA64182313C2AA00284168 /* StatsVersion.swift */; };
+		02EAB6D72480A86D00FD873C /* CrashLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EAB6D62480A86D00FD873C /* CrashLogger.swift */; };
 		26577519243D808B003168A5 /* WooCommerceModelV26toV27.xcmappingmodel in Sources */ = {isa = PBXBuildFile; fileRef = 26577518243D808B003168A5 /* WooCommerceModelV26toV27.xcmappingmodel */; };
 		450106892399AC7400E24722 /* TaxClass+CoreDataClass.swift in Sources */ = {isa = PBXBuildFile; fileRef = 450106882399AC7400E24722 /* TaxClass+CoreDataClass.swift */; };
 		4501068B2399AC9B00E24722 /* TaxClass+CoreDataProperties.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4501068A2399AC9B00E24722 /* TaxClass+CoreDataProperties.swift */; };
@@ -156,11 +158,13 @@
 		028296F0237D404F00E84012 /* Attribute+CoreDataClass.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Attribute+CoreDataClass.swift"; sourceTree = "<group>"; };
 		028296F1237D404F00E84012 /* Attribute+CoreDataProperties.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Attribute+CoreDataProperties.swift"; sourceTree = "<group>"; };
 		028F00652331605000E6C283 /* Model 20.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Model 20.xcdatamodel"; sourceTree = "<group>"; };
+		02A098262480D160002F8C7A /* MockCrashLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockCrashLogger.swift; sourceTree = "<group>"; };
 		02A9F16A22F9873600EE36EA /* Model 19.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Model 19.xcdatamodel"; sourceTree = "<group>"; };
 		02D45648231CFB27008CF0A9 /* StatsVersionBannerVisibility.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsVersionBannerVisibility.swift; sourceTree = "<group>"; };
 		02DA64162313C26400284168 /* StatsVersionBySite.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsVersionBySite.swift; sourceTree = "<group>"; };
 		02DA64182313C2AA00284168 /* StatsVersion.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsVersion.swift; sourceTree = "<group>"; };
 		02E4F5E223CD552F003B0010 /* Model 26.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Model 26.xcdatamodel"; sourceTree = "<group>"; };
+		02EAB6D62480A86D00FD873C /* CrashLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrashLogger.swift; sourceTree = "<group>"; };
 		262CD20224317C2F00932241 /* Model 27.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Model 27.xcdatamodel"; sourceTree = "<group>"; };
 		26577518243D808B003168A5 /* WooCommerceModelV26toV27.xcmappingmodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcmappingmodel; path = WooCommerceModelV26toV27.xcmappingmodel; sourceTree = "<group>"; };
 		450106882399AC7400E24722 /* TaxClass+CoreDataClass.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TaxClass+CoreDataClass.swift"; sourceTree = "<group>"; };
@@ -326,6 +330,14 @@
 /* End PBXFrameworksBuildPhase section */
 
 /* Begin PBXGroup section */
+		02A098252480D155002F8C7A /* Mocks */ = {
+			isa = PBXGroup;
+			children = (
+				02A098262480D160002F8C7A /* MockCrashLogger.swift */,
+			);
+			path = Mocks;
+			sourceTree = "<group>";
+		};
 		4DD3D0BDDE216A90FC1335D7 /* Pods */ = {
 			isa = PBXGroup;
 			children = (
@@ -402,6 +414,7 @@
 				B52B0F7820AA287C00477698 /* StorageManagerType.swift */,
 				B505F6DF20BEEA8100BB1B69 /* StorageType.swift */,
 				D87F61522265AA230031A13B /* FileStorage.swift */,
+				02EAB6D62480A86D00FD873C /* CrashLogger.swift */,
 			);
 			path = Protocols;
 			sourceTree = "<group>";
@@ -454,6 +467,7 @@
 			isa = PBXGroup;
 			children = (
 				D87F61582265AED70031A13B /* Mock data */,
+				02A098252480D155002F8C7A /* Mocks */,
 				B54CA5C520A4BFC800F38CD1 /* Tools */,
 				B52B0F7D20AA2F1200477698 /* Extensions */,
 				B59E11DC20A9F1E6004121A4 /* CoreData */,
@@ -814,6 +828,7 @@
 				B54CA5BD20A4BD3B00F38CD1 /* NSManagedObjectContext+Storage.swift in Sources */,
 				933A27302222344D00C2143A /* Logging.swift in Sources */,
 				747453A52242C85E00E0B5EE /* ProductDefaultAttribute+CoreDataClass.swift in Sources */,
+				02EAB6D72480A86D00FD873C /* CrashLogger.swift in Sources */,
 				746A9D21214078080013F6FF /* TopEarnerStats+CoreDataClass.swift in Sources */,
 				74B7D6AD20F90CBB002667AC /* OrderNote+CoreDataClass.swift in Sources */,
 				B52B0F7920AA287C00477698 /* StorageManagerType.swift in Sources */,
@@ -892,6 +907,7 @@
 				9302E3AC220E1CE900DA5018 /* CoreDataIterativeMigratorTests.swift in Sources */,
 				B59E11E020A9F5E6004121A4 /* Constants.swift in Sources */,
 				B54CA5C320A4BF6900F38CD1 /* NSManagedObjectContextStorageTests.swift in Sources */,
+				02A098272480D160002F8C7A /* MockCrashLogger.swift in Sources */,
 				B54CA5C720A4BFDC00F38CD1 /* DummyStack.swift in Sources */,
 				B54CA5C220A4BF6900F38CD1 /* NSManagedObjectStorageTests.swift in Sources */,
 				D87F61572265AD980031A13B /* FileStorageTests.swift in Sources */,

Storage/Storage/CoreData/CoreDataManager.swift

@@ -10,15 +10,17 @@ public class CoreDataManager: StorageManagerType {
     ///
     public let name: String
 
+    private let crashLogger: CrashLogger
 
     /// Designated Initializer.
     ///
     /// - Parameter name: Identifier to be used for: [database, data model, container].
     ///
     /// - Important: This should *match* with your actual Data Model file!.
     ///
-    public init(name: String) {
+    public init(name: String, crashLogger: CrashLogger) {
         self.name = name
+        self.crashLogger = crashLogger
     }
 
 
@@ -37,21 +39,38 @@ public class CoreDataManager: StorageManagerType {
         container.persistentStoreDescriptions = [storeDescription]
 
         container.loadPersistentStores { [weak self] (storeDescription, error) in
-            guard let `self` = self, let error = error else {
+            guard let `self` = self, let persistentStoreLoadingError = error else {
                 return
             }
 
-            DDLogError("⛔️ [CoreDataManager] loadPersistentStore failed. Attempting to recover... \(error)")
+            DDLogError("⛔️ [CoreDataManager] loadPersistentStore failed. Attempting to recover... \(persistentStoreLoadingError)")
 
             /// Backup the old Store
             ///
             do {
                 let sourceURL = self.storeURL
                 let backupURL = sourceURL.appendingPathExtension("~")
                 try FileManager.default.copyItem(at: sourceURL, to: backupURL)
-                try FileManager.default.removeItem(at: sourceURL)
             } catch {
-                fatalError("☠️ [CoreDataManager] Cannot backup Store: \(error)")
+                let message = "☠️ [CoreDataManager] Cannot backup Store: \(error)"
+                self.crashLogger.logMessage(message,
+                                            properties: ["persistentStoreLoadingError": persistentStoreLoadingError,
+                                                         "backupError": error],
+                                            level: .fatal)
+                fatalError(message)
+            }
+
+            /// Remove the old Store
+            ///
+            do {
+                try FileManager.default.removeItem(at: self.storeURL)
+            } catch {
+                let message = "☠️ [CoreDataManager] Cannot remove Store: \(error)"
+                self.crashLogger.logMessage(message,
+                                            properties: ["persistentStoreLoadingError": persistentStoreLoadingError,
+                                                         "removeStoreError": error],
+                                            level: .fatal)
+                fatalError(message)
             }
 
             /// Retry!
@@ -61,7 +80,12 @@ public class CoreDataManager: StorageManagerType {
                     return
                 }
 
-                fatalError("☠️ [CoreDataManager] Recovery Failed! \(error) [\(error.userInfo)]")
+                let message = "☠️ [CoreDataManager] Recovery Failed! \(error) [\(error.userInfo)]"
+                self?.crashLogger.logMessage(message,
+                                             properties: ["persistentStoreLoadingError": persistentStoreLoadingError,
+                                                          "retryError": error],
+                                             level: .fatal)
+                fatalError(message)
             }
         }
 

Storage/Storage/Protocols/CrashLogger.swift

@@ -0,0 +1,20 @@
+/// The level of severity, that is currently based on `SentrySeverity`.
+public enum SeverityLevel {
+    case fatal
+    case error
+    case warning
+    case info
+    case debug
+}
+
+/// Logs crashes or messages at a given severity level.
+public protocol CrashLogger {
+    /**
+     Writes a message to the Crash Logging system.
+     - Parameters:
+     - message: The message
+     - properties: A dictionary containing additional information about this message
+     - level: The level of severity to report
+    */
+    func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel)
+}

Storage/StorageTests/CoreData/CoreDataManagerTests.swift

@@ -10,23 +10,23 @@ class CoreDataManagerTests: XCTestCase {
     /// Verifies that the Data Model URL contains the ContextIdentifier String.
     ///
     func testModelUrlMapsToDataModelWithContextIdentifier() {
-        let manager = CoreDataManager(name: "WooCommerce")
+        let manager = CoreDataManager(name: "WooCommerce", crashLogger: MockCrashLogger())
         XCTAssertEqual(manager.modelURL.lastPathComponent, "WooCommerce.momd")
         XCTAssertNoThrow(manager.managedModel)
     }
 
     /// Verifies that the Store URL contains the ContextIdentifier string.
     ///
     func testStorageUrlMapsToSqliteFileWithContextIdentifier() {
-        let manager = CoreDataManager(name: "WooCommerce")
+        let manager = CoreDataManager(name: "WooCommerce", crashLogger: MockCrashLogger())
         XCTAssertEqual(manager.storeURL.lastPathComponent, "WooCommerce.sqlite")
         XCTAssertEqual(manager.storeDescription.url?.lastPathComponent, "WooCommerce.sqlite")
     }
 
     /// Verifies that the PersistentContainer properly loads the sqlite database.
     ///
     func testPersistentContainerLoadsExpectedDataModelAndSqliteDatabase() {
-        let manager = CoreDataManager(name: "WooCommerce")
+        let manager = CoreDataManager(name: "WooCommerce", crashLogger: MockCrashLogger())
         let container = manager.persistentContainer
 
         XCTAssertEqual(container.managedObjectModel, manager.managedModel)
@@ -36,14 +36,14 @@ class CoreDataManagerTests: XCTestCase {
     /// Verifies that the ContextManager's viewContext matches the PersistenContainer.viewContext
     ///
     func testViewContextPropertyReturnsPersistentContainerMainContext() {
-        let manager = CoreDataManager(name: "WooCommerce")
+        let manager = CoreDataManager(name: "WooCommerce", crashLogger: MockCrashLogger())
         XCTAssertEqual(manager.viewStorage as? NSManagedObjectContext, manager.persistentContainer.viewContext)
     }
 
     /// Verifies that performBackgroundTask effectively runs received closure in BG.
     ///
     func testPerformTaskInBackgroundEffectivelyRunsReceivedClosureInBackgroundThread() {
-        let manager = CoreDataManager(name: "WooCommerce")
+        let manager = CoreDataManager(name: "WooCommerce", crashLogger: MockCrashLogger())
         let expectation = self.expectation(description: "Background")
 
         manager.performBackgroundTask { (_) in
@@ -57,7 +57,7 @@ class CoreDataManagerTests: XCTestCase {
     /// Verifies that derived context is instantiated correctly.
     ///
     func testDerivedStorageIsInstantiatedCorrectly() {
-        let manager = CoreDataManager(name: "WooCommerce")
+        let manager = CoreDataManager(name: "WooCommerce", crashLogger: MockCrashLogger())
         let viewContext = (manager.viewStorage as? NSManagedObjectContext)
         let derivedContext = (manager.newDerivedStorage() as? NSManagedObjectContext)
 

Storage/StorageTests/Mocks/MockCrashLogger.swift

@@ -0,0 +1,7 @@
+import Storage
+
+struct MockCrashLogger: CrashLogger {
+    func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) {
+        // no-op
+    }
+}

WooCommerce/Classes/ServiceLocator/ServiceLocator.swift

@@ -43,7 +43,7 @@ final class ServiceLocator {
 
     /// CoreData Stack
     ///
-    private static var _storageManager = CoreDataManager(name: WooConstants.databaseStackName)
+    private static var _storageManager = CoreDataManager(name: WooConstants.databaseStackName, crashLogger: SentryCrashLogger())
 
     /// Cocoalumberjack DDLog
     ///

WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift

@@ -0,0 +1,32 @@
+import Foundation
+
+extension Dictionary where Key == String {
+    /// Manually serializes a value in a dictionary if the value is not already serializable.
+    func serializeValuesForLoggingIfNeeded() -> [String: Any] {
+        guard JSONSerialization.isValidJSONObject(self) == false else {
+            return self
+        }
+
+        return reduce([:]) { (properties, entry) -> [String: Any] in
+            let (key, value) = entry
+            var formattedProperties: [String: Any] = properties
+            guard JSONSerialization.isValidJSONObject([key: value]) == false else {
+                formattedProperties[key] = value
+                return formattedProperties
+            }
+
+            if let nsError = value as? NSError {
+                formattedProperties[key] = [
+                    "Domain": nsError.domain,
+                    "Code": nsError.code,
+                    "Description": nsError.localizedDescription,
+                    "User Info": nsError.userInfo.description
+                ]
+                return formattedProperties
+            }
+
+            formattedProperties[key] = "\(value)"
+            return formattedProperties
+        }
+    }
+}

WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift

@@ -0,0 +1,27 @@
+import AutomatticTracks
+import Sentry
+import Storage
+
+/// Logs crashes/messages to Sentry.
+final class SentryCrashLogger: CrashLogger {
+    func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) {
+        CrashLogging.logMessage(message, properties: properties?.serializeValuesForLoggingIfNeeded(), level: SentrySeverity(level: level))
+    }
+}
+
+private extension SentrySeverity {
+    init(level: SeverityLevel) {
+        switch level {
+        case .fatal:
+            self = .fatal
+        case .error:
+            self = .error
+        case .warning:
+            self = .warning
+        case .info:
+            self = .info
+        case .debug:
+            self = .debug
+        }
+    }
+}