From 976c42612dd48af107c6b9b088e0e8aa286b6cf3 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Fri, 29 May 2020 11:31:06 +0800 Subject: [PATCH 01/17] Create a `CrashLogger` for `CoreDataManager` to log fatal errors in Storage. --- Storage/Storage.xcodeproj/project.pbxproj | 4 +++ .../Storage/CoreData/CoreDataManager.swift | 22 ++++++++++++---- Storage/Storage/Protocols/CrashLogger.swift | 22 ++++++++++++++++ .../ServiceLocator/ServiceLocator.swift | 2 +- .../Tools/Logging/DefaultCrashLogger.swift | 26 +++++++++++++++++++ .../WooCommerce.xcodeproj/project.pbxproj | 4 +++ 6 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 Storage/Storage/Protocols/CrashLogger.swift create mode 100644 WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift diff --git a/Storage/Storage.xcodeproj/project.pbxproj b/Storage/Storage.xcodeproj/project.pbxproj index 09a5eac84a1..822ed0fb9a3 100644 --- a/Storage/Storage.xcodeproj/project.pbxproj +++ b/Storage/Storage.xcodeproj/project.pbxproj @@ -20,6 +20,7 @@ 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 */; }; @@ -161,6 +162,7 @@ 02DA64162313C26400284168 /* StatsVersionBySite.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsVersionBySite.swift; sourceTree = ""; }; 02DA64182313C2AA00284168 /* StatsVersion.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsVersion.swift; sourceTree = ""; }; 02E4F5E223CD552F003B0010 /* Model 26.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Model 26.xcdatamodel"; sourceTree = ""; }; + 02EAB6D62480A86D00FD873C /* CrashLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrashLogger.swift; sourceTree = ""; }; 262CD20224317C2F00932241 /* Model 27.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Model 27.xcdatamodel"; sourceTree = ""; }; 26577518243D808B003168A5 /* WooCommerceModelV26toV27.xcmappingmodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcmappingmodel; path = WooCommerceModelV26toV27.xcmappingmodel; sourceTree = ""; }; 450106882399AC7400E24722 /* TaxClass+CoreDataClass.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TaxClass+CoreDataClass.swift"; sourceTree = ""; }; @@ -402,6 +404,7 @@ B52B0F7820AA287C00477698 /* StorageManagerType.swift */, B505F6DF20BEEA8100BB1B69 /* StorageType.swift */, D87F61522265AA230031A13B /* FileStorage.swift */, + 02EAB6D62480A86D00FD873C /* CrashLogger.swift */, ); path = Protocols; sourceTree = ""; @@ -814,6 +817,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 */, diff --git a/Storage/Storage/CoreData/CoreDataManager.swift b/Storage/Storage/CoreData/CoreDataManager.swift index f25281d7e27..46d5e7de383 100644 --- a/Storage/Storage/CoreData/CoreDataManager.swift +++ b/Storage/Storage/CoreData/CoreDataManager.swift @@ -10,6 +10,7 @@ public class CoreDataManager: StorageManagerType { /// public let name: String + private let crashLogger: CrashLogger /// Designated Initializer. /// @@ -17,8 +18,9 @@ public class CoreDataManager: StorageManagerType { /// /// - 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,11 +39,11 @@ 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 errorLoadingPersistentStores = error else { return } - DDLogError("⛔️ [CoreDataManager] loadPersistentStore failed. Attempting to recover... \(error)") + DDLogError("⛔️ [CoreDataManager] loadPersistentStore failed. Attempting to recover... \(errorLoadingPersistentStores)") /// Backup the old Store /// @@ -51,7 +53,12 @@ public class CoreDataManager: StorageManagerType { 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: ["loadPersistentStoresError": errorLoadingPersistentStores, + "backupError": error], + level: .fatal) + fatalError(message) } /// Retry! @@ -61,7 +68,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: ["loadPersistentStoresError": errorLoadingPersistentStores, + "retryError": error], + level: .fatal) + fatalError(message) } } diff --git a/Storage/Storage/Protocols/CrashLogger.swift b/Storage/Storage/Protocols/CrashLogger.swift new file mode 100644 index 00000000000..65302e65d78 --- /dev/null +++ b/Storage/Storage/Protocols/CrashLogger.swift @@ -0,0 +1,22 @@ +//import Foundation + +/// 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) +} diff --git a/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift b/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift index 5465fb21a92..c096a130d4b 100644 --- a/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift +++ b/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: DefaultCrashLogger()) /// Cocoalumberjack DDLog /// diff --git a/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift b/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift new file mode 100644 index 00000000000..2348fff3449 --- /dev/null +++ b/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift @@ -0,0 +1,26 @@ +import AutomatticTracks +import Sentry +import Storage + +final class DefaultCrashLogger: CrashLogger { + func logMessage(_ message: String, properties: [String : Any]?, level: SeverityLevel) { + CrashLogging.logMessage(message, properties: properties, 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 + } + } +} diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index 12f62de712c..30480213fdb 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -242,6 +242,7 @@ 02EA6BF82435E80600FFF90A /* ImageDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EA6BF72435E80600FFF90A /* ImageDownloader.swift */; }; 02EA6BFA2435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EA6BF92435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift */; }; 02EA6BFC2435EC3500FFF90A /* MockImageDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EA6BFB2435EC3500FFF90A /* MockImageDownloader.swift */; }; + 02EAB6D92480AA4900FD873C /* DefaultCrashLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EAB6D82480AA4900FD873C /* DefaultCrashLogger.swift */; }; 02EEB5C42424AFAA00B8A701 /* TextFieldTableViewCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EEB5C22424AFAA00B8A701 /* TextFieldTableViewCell.swift */; }; 02EEB5C52424AFAA00B8A701 /* TextFieldTableViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 02EEB5C32424AFAA00B8A701 /* TextFieldTableViewCell.xib */; }; 02F49ADA23BF356E00FA0BFA /* TitleAndTextFieldTableViewCell.ViewModel+State.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02F49AD923BF356E00FA0BFA /* TitleAndTextFieldTableViewCell.ViewModel+State.swift */; }; @@ -1096,6 +1097,7 @@ 02EA6BF72435E80600FFF90A /* ImageDownloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageDownloader.swift; sourceTree = ""; }; 02EA6BF92435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "KingfisherImageDownloader+ImageDownloadable.swift"; sourceTree = ""; }; 02EA6BFB2435EC3500FFF90A /* MockImageDownloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockImageDownloader.swift; sourceTree = ""; }; + 02EAB6D82480AA4900FD873C /* DefaultCrashLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultCrashLogger.swift; sourceTree = ""; }; 02EEB5C22424AFAA00B8A701 /* TextFieldTableViewCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextFieldTableViewCell.swift; sourceTree = ""; }; 02EEB5C32424AFAA00B8A701 /* TextFieldTableViewCell.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = TextFieldTableViewCell.xib; sourceTree = ""; }; 02F49AD923BF356E00FA0BFA /* TitleAndTextFieldTableViewCell.ViewModel+State.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TitleAndTextFieldTableViewCell.ViewModel+State.swift"; sourceTree = ""; }; @@ -3102,6 +3104,7 @@ children = ( 933A27362222354600C2143A /* Logging.swift */, B5DB01B42114AB2D00A4F797 /* CrashLogging.swift */, + 02EAB6D82480AA4900FD873C /* DefaultCrashLogger.swift */, ); path = Logging; sourceTree = ""; @@ -4748,6 +4751,7 @@ B517EA1D218B41F200730EC4 /* String+Woo.swift in Sources */, 02564A8C246CE38E00D6DB2A /* SwappableSubviewContainerView.swift in Sources */, 024DF31223742B18006658FE /* AztecItalicFormatBarCommand.swift in Sources */, + 02EAB6D92480AA4900FD873C /* DefaultCrashLogger.swift in Sources */, D83F5933225B2EB900626E75 /* ManualTrackingViewController.swift in Sources */, B57C744A20F5649300EEFC87 /* EmptyStoresTableViewCell.swift in Sources */, 027D67D1245ADDF40036B8DB /* FilterTypeViewModel+Helpers.swift in Sources */, From 6bb64e3dd0f605536aec0f8a2316b9ee692ff5c8 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Fri, 29 May 2020 13:15:04 +0800 Subject: [PATCH 02/17] Create mock for `CrashLogger` in Storage and Yosemite tests for `CoreDataManager`. --- Storage/Storage.xcodeproj/project.pbxproj | 12 ++++++++++++ .../StorageTests/CoreData/CoreDataManagerTests.swift | 12 ++++++------ Storage/StorageTests/Mocks/MockCrashLogger.swift | 7 +++++++ Yosemite/Yosemite.xcodeproj/project.pbxproj | 4 ++++ Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift | 7 +++++++ .../StorageShippingSettingsServiceTests.swift | 2 +- 6 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 Storage/StorageTests/Mocks/MockCrashLogger.swift create mode 100644 Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift diff --git a/Storage/Storage.xcodeproj/project.pbxproj b/Storage/Storage.xcodeproj/project.pbxproj index 822ed0fb9a3..5d398d14f54 100644 --- a/Storage/Storage.xcodeproj/project.pbxproj +++ b/Storage/Storage.xcodeproj/project.pbxproj @@ -17,6 +17,7 @@ 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 */; }; @@ -157,6 +158,7 @@ 028296F0237D404F00E84012 /* Attribute+CoreDataClass.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Attribute+CoreDataClass.swift"; sourceTree = ""; }; 028296F1237D404F00E84012 /* Attribute+CoreDataProperties.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Attribute+CoreDataProperties.swift"; sourceTree = ""; }; 028F00652331605000E6C283 /* Model 20.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Model 20.xcdatamodel"; sourceTree = ""; }; + 02A098262480D160002F8C7A /* MockCrashLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockCrashLogger.swift; sourceTree = ""; }; 02A9F16A22F9873600EE36EA /* Model 19.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "Model 19.xcdatamodel"; sourceTree = ""; }; 02D45648231CFB27008CF0A9 /* StatsVersionBannerVisibility.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsVersionBannerVisibility.swift; sourceTree = ""; }; 02DA64162313C26400284168 /* StatsVersionBySite.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsVersionBySite.swift; sourceTree = ""; }; @@ -328,6 +330,14 @@ /* End PBXFrameworksBuildPhase section */ /* Begin PBXGroup section */ + 02A098252480D155002F8C7A /* Mocks */ = { + isa = PBXGroup; + children = ( + 02A098262480D160002F8C7A /* MockCrashLogger.swift */, + ); + path = Mocks; + sourceTree = ""; + }; 4DD3D0BDDE216A90FC1335D7 /* Pods */ = { isa = PBXGroup; children = ( @@ -457,6 +467,7 @@ isa = PBXGroup; children = ( D87F61582265AED70031A13B /* Mock data */, + 02A098252480D155002F8C7A /* Mocks */, B54CA5C520A4BFC800F38CD1 /* Tools */, B52B0F7D20AA2F1200477698 /* Extensions */, B59E11DC20A9F1E6004121A4 /* CoreData */, @@ -896,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 */, diff --git a/Storage/StorageTests/CoreData/CoreDataManagerTests.swift b/Storage/StorageTests/CoreData/CoreDataManagerTests.swift index 20c70b4c5b0..9824751d2ed 100644 --- a/Storage/StorageTests/CoreData/CoreDataManagerTests.swift +++ b/Storage/StorageTests/CoreData/CoreDataManagerTests.swift @@ -10,7 +10,7 @@ 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) } @@ -18,7 +18,7 @@ class CoreDataManagerTests: XCTestCase { /// 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") } @@ -26,7 +26,7 @@ class CoreDataManagerTests: XCTestCase { /// 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) diff --git a/Storage/StorageTests/Mocks/MockCrashLogger.swift b/Storage/StorageTests/Mocks/MockCrashLogger.swift new file mode 100644 index 00000000000..d2e06235847 --- /dev/null +++ b/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 + } +} diff --git a/Yosemite/Yosemite.xcodeproj/project.pbxproj b/Yosemite/Yosemite.xcodeproj/project.pbxproj index ddb3af97380..1df6c63975d 100644 --- a/Yosemite/Yosemite.xcodeproj/project.pbxproj +++ b/Yosemite/Yosemite.xcodeproj/project.pbxproj @@ -47,6 +47,7 @@ 026D52C0238235930092AE05 /* ProductVariationStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 026D52BF238235930092AE05 /* ProductVariationStoreTests.swift */; }; 028BCE2422DE22BB00056966 /* SiteVisitStatsStoreErrorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 028BCE2322DE22BB00056966 /* SiteVisitStatsStoreErrorTests.swift */; }; 029B00A7230D64E800B0AE66 /* StatsTimeRangeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 029B00A6230D64E800B0AE66 /* StatsTimeRangeTests.swift */; }; + 02A098242480D0D8002F8C7A /* MockCrashLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02A098232480D0D8002F8C7A /* MockCrashLogger.swift */; }; 02BA23C222EEEABC009539E7 /* AvailabilityStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02BA23C122EEEABC009539E7 /* AvailabilityStore.swift */; }; 02BA23C422EEEB3B009539E7 /* AvailabilityAction.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02BA23C322EEEB3B009539E7 /* AvailabilityAction.swift */; }; 02BA23C622EEF092009539E7 /* StatsV4AvailabilityStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02BA23C522EEF092009539E7 /* StatsV4AvailabilityStoreTests.swift */; }; @@ -280,6 +281,7 @@ 026D52BF238235930092AE05 /* ProductVariationStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductVariationStoreTests.swift; sourceTree = ""; }; 028BCE2322DE22BB00056966 /* SiteVisitStatsStoreErrorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SiteVisitStatsStoreErrorTests.swift; sourceTree = ""; }; 029B00A6230D64E800B0AE66 /* StatsTimeRangeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsTimeRangeTests.swift; sourceTree = ""; }; + 02A098232480D0D8002F8C7A /* MockCrashLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockCrashLogger.swift; sourceTree = ""; }; 02BA23C122EEEABC009539E7 /* AvailabilityStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AvailabilityStore.swift; sourceTree = ""; }; 02BA23C322EEEB3B009539E7 /* AvailabilityAction.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AvailabilityAction.swift; sourceTree = ""; }; 02BA23C522EEF092009539E7 /* StatsV4AvailabilityStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatsV4AvailabilityStoreTests.swift; sourceTree = ""; }; @@ -991,6 +993,7 @@ 0202B6962387AFBF00F3EBE0 /* MockInMemoryStorage.swift */, 020220E32396969E00290165 /* MockProduct.swift */, 0248B36A2459127200A271A4 /* MockupNetwork+Path.swift */, + 02A098232480D0D8002F8C7A /* MockCrashLogger.swift */, ); path = Mockups; sourceTree = ""; @@ -1382,6 +1385,7 @@ 029B00A7230D64E800B0AE66 /* StatsTimeRangeTests.swift in Sources */, 02E493ED245C0EBC000AEA9E /* Product+SettingsTests.swift in Sources */, 0248B36924590FC300A271A4 /* ProductStore+FilterProductsTests.swift in Sources */, + 02A098242480D0D8002F8C7A /* MockCrashLogger.swift in Sources */, 02FF055F23D985710058E6E7 /* URL+MediaTests.swift in Sources */, 02124DAC24318D6B00980D74 /* Media+MediaTypeTests.swift in Sources */, 025CA2D0238F54E800B05C81 /* ProductShippingClassStoreTests.swift in Sources */, diff --git a/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift new file mode 100644 index 00000000000..d2e06235847 --- /dev/null +++ b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift @@ -0,0 +1,7 @@ +import Storage + +struct MockCrashLogger: CrashLogger { + func logMessage(_ message: String, properties: [String : Any]?, level: SeverityLevel) { + // no-op + } +} diff --git a/Yosemite/YosemiteTests/Tools/ShippingSettings/StorageShippingSettingsServiceTests.swift b/Yosemite/YosemiteTests/Tools/ShippingSettings/StorageShippingSettingsServiceTests.swift index 40c8f02ab7c..f27a58bdcbb 100644 --- a/Yosemite/YosemiteTests/Tools/ShippingSettings/StorageShippingSettingsServiceTests.swift +++ b/Yosemite/YosemiteTests/Tools/ShippingSettings/StorageShippingSettingsServiceTests.swift @@ -10,7 +10,7 @@ final class StorageShippingSettingsServiceTests: XCTestCase { override func setUp() { super.setUp() - storage = CoreDataManager(name: "WooCommerce") + storage = CoreDataManager(name: "WooCommerce", crashLogger: MockCrashLogger()) } func testInitialShippingSettings() { From 1e3c697253e5763e258576348aea2c9b8fa16bda Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Fri, 29 May 2020 14:53:58 +0800 Subject: [PATCH 03/17] Fix lint warnings. --- Storage/Storage/Protocols/CrashLogger.swift | 2 +- Storage/StorageTests/Mocks/MockCrashLogger.swift | 2 +- WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift | 2 +- Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Storage/Storage/Protocols/CrashLogger.swift b/Storage/Storage/Protocols/CrashLogger.swift index 65302e65d78..9015621c8a4 100644 --- a/Storage/Storage/Protocols/CrashLogger.swift +++ b/Storage/Storage/Protocols/CrashLogger.swift @@ -18,5 +18,5 @@ public protocol CrashLogger { - 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) + func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) } diff --git a/Storage/StorageTests/Mocks/MockCrashLogger.swift b/Storage/StorageTests/Mocks/MockCrashLogger.swift index d2e06235847..a8c24ed12fd 100644 --- a/Storage/StorageTests/Mocks/MockCrashLogger.swift +++ b/Storage/StorageTests/Mocks/MockCrashLogger.swift @@ -1,7 +1,7 @@ import Storage struct MockCrashLogger: CrashLogger { - func logMessage(_ message: String, properties: [String : Any]?, level: SeverityLevel) { + func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) { // no-op } } diff --git a/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift b/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift index 2348fff3449..fbf5682238e 100644 --- a/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift +++ b/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift @@ -3,7 +3,7 @@ import Sentry import Storage final class DefaultCrashLogger: CrashLogger { - func logMessage(_ message: String, properties: [String : Any]?, level: SeverityLevel) { + func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) { CrashLogging.logMessage(message, properties: properties, level: SentrySeverity(level: level)) } } diff --git a/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift index d2e06235847..a8c24ed12fd 100644 --- a/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift +++ b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift @@ -1,7 +1,7 @@ import Storage struct MockCrashLogger: CrashLogger { - func logMessage(_ message: String, properties: [String : Any]?, level: SeverityLevel) { + func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) { // no-op } } From 7c9a7a6cc961a1468fb51858da7a1567aa902c04 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Fri, 29 May 2020 15:17:39 +0800 Subject: [PATCH 04/17] Separate copying and removing the old persistent store so that we have more insight about where the error comes from. --- Storage/Storage/CoreData/CoreDataManager.swift | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Storage/Storage/CoreData/CoreDataManager.swift b/Storage/Storage/CoreData/CoreDataManager.swift index 46d5e7de383..91e9e31b831 100644 --- a/Storage/Storage/CoreData/CoreDataManager.swift +++ b/Storage/Storage/CoreData/CoreDataManager.swift @@ -51,7 +51,6 @@ public class CoreDataManager: StorageManagerType { let sourceURL = self.storeURL let backupURL = sourceURL.appendingPathExtension("~") try FileManager.default.copyItem(at: sourceURL, to: backupURL) - try FileManager.default.removeItem(at: sourceURL) } catch { let message = "☠️ [CoreDataManager] Cannot backup Store: \(error)" self.crashLogger.logMessage(message, @@ -61,6 +60,19 @@ public class CoreDataManager: StorageManagerType { 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: ["loadPersistentStoresError": errorLoadingPersistentStores, + "removeStoreError": error], + level: .fatal) + fatalError(message) + } + /// Retry! /// container.loadPersistentStores { [weak self] (storeDescription, error) in From 781b0f6b015f68439b316a4d81bed279d3b84007 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 1 Jun 2020 16:27:37 +0800 Subject: [PATCH 05/17] Create a helper to serialize values for a dictionary for logging purposes. --- .../Tools/Logging/DefaultCrashLogger.swift | 2 +- .../Tools/Logging/Dictionary+Logging.swift | 31 ++++++++++++ .../WooCommerce.xcodeproj/project.pbxproj | 16 +++++++ .../Logging/Dictionary+LoggingTests.swift | 48 +++++++++++++++++++ 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift create mode 100644 WooCommerce/WooCommerceTests/Tools/Logging/Dictionary+LoggingTests.swift diff --git a/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift b/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift index fbf5682238e..5f4f4823b8b 100644 --- a/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift +++ b/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift @@ -4,7 +4,7 @@ import Storage final class DefaultCrashLogger: CrashLogger { func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) { - CrashLogging.logMessage(message, properties: properties, level: SentrySeverity(level: level)) + CrashLogging.logMessage(message, properties: properties?.serializeValuesForLoggingIfNeeded(), level: SentrySeverity(level: level)) } } diff --git a/WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift b/WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift new file mode 100644 index 00000000000..748418f994a --- /dev/null +++ b/WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift @@ -0,0 +1,31 @@ +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 + } + + return formattedProperties + } + } +} diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index 30480213fdb..46af391f275 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -172,6 +172,8 @@ 0286B27C23C7051F003D784B /* ProductImagesViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 0286B27823C7051F003D784B /* ProductImagesViewController.xib */; }; 0286B27D23C7051F003D784B /* ProductImagesViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0286B27923C7051F003D784B /* ProductImagesViewController.swift */; }; 0286B27F23C70557003D784B /* ColumnFlowLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0286B27E23C70557003D784B /* ColumnFlowLayout.swift */; }; + 028AFFB32484ED2800693C09 /* Dictionary+Logging.swift in Sources */ = {isa = PBXBuildFile; fileRef = 028AFFB22484ED2800693C09 /* Dictionary+Logging.swift */; }; + 028AFFB62484EDA000693C09 /* Dictionary+LoggingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 028AFFB52484EDA000693C09 /* Dictionary+LoggingTests.swift */; }; 028BAC3D22F2DECE008BB4AF /* StoreStatsAndTopPerformersViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 028BAC3C22F2DECE008BB4AF /* StoreStatsAndTopPerformersViewController.swift */; }; 028BAC4022F2EFA5008BB4AF /* StoreStatsAndTopPerformersPeriodViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 028BAC3F22F2EFA5008BB4AF /* StoreStatsAndTopPerformersPeriodViewController.swift */; }; 028BAC4222F30B05008BB4AF /* StoreStatsV4PeriodViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 028BAC4122F30B05008BB4AF /* StoreStatsV4PeriodViewController.swift */; }; @@ -1027,6 +1029,8 @@ 0286B27823C7051F003D784B /* ProductImagesViewController.xib */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.xib; path = ProductImagesViewController.xib; sourceTree = ""; }; 0286B27923C7051F003D784B /* ProductImagesViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ProductImagesViewController.swift; sourceTree = ""; }; 0286B27E23C70557003D784B /* ColumnFlowLayout.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ColumnFlowLayout.swift; sourceTree = ""; }; + 028AFFB22484ED2800693C09 /* Dictionary+Logging.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Dictionary+Logging.swift"; sourceTree = ""; }; + 028AFFB52484EDA000693C09 /* Dictionary+LoggingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Dictionary+LoggingTests.swift"; sourceTree = ""; }; 028BAC3C22F2DECE008BB4AF /* StoreStatsAndTopPerformersViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoreStatsAndTopPerformersViewController.swift; sourceTree = ""; }; 028BAC3F22F2EFA5008BB4AF /* StoreStatsAndTopPerformersPeriodViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoreStatsAndTopPerformersPeriodViewController.swift; sourceTree = ""; }; 028BAC4122F30B05008BB4AF /* StoreStatsV4PeriodViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoreStatsV4PeriodViewController.swift; sourceTree = ""; }; @@ -2132,6 +2136,14 @@ path = Media; sourceTree = ""; }; + 028AFFB42484ED7F00693C09 /* Logging */ = { + isa = PBXGroup; + children = ( + 028AFFB52484EDA000693C09 /* Dictionary+LoggingTests.swift */, + ); + path = Logging; + sourceTree = ""; + }; 028BAC4322F3AE3B008BB4AF /* Stats v4 */ = { isa = PBXGroup; children = ( @@ -2742,6 +2754,7 @@ B53A569521123D27000776C9 /* Tools */ = { isa = PBXGroup; children = ( + 028AFFB42484ED7F00693C09 /* Logging */, 5798191124526FC7000817F8 /* Observables */, D8A8C4F22268288F001C72BF /* AddManualCustomTrackingViewModelTests.swift */, D83F5938225B424B00626E75 /* AddManualTrackingViewModelTests.swift */, @@ -3105,6 +3118,7 @@ 933A27362222354600C2143A /* Logging.swift */, B5DB01B42114AB2D00A4F797 /* CrashLogging.swift */, 02EAB6D82480AA4900FD873C /* DefaultCrashLogger.swift */, + 028AFFB22484ED2800693C09 /* Dictionary+Logging.swift */, ); path = Logging; sourceTree = ""; @@ -4686,6 +4700,7 @@ 02162729237965E8000208D2 /* ProductFormTableViewModel.swift in Sources */, CE2A9FBF23BFB1BE002BEC1C /* LedgerTableViewCell.swift in Sources */, B58B4AC02108FF6100076FDD /* Array+Helpers.swift in Sources */, + 028AFFB32484ED2800693C09 /* Dictionary+Logging.swift in Sources */, B5A56BF0219F2CE90065A902 /* VerticalButton.swift in Sources */, 748C7780211E18A600814F2C /* OrderStats+Woo.swift in Sources */, D831E2DC230E0558000037D0 /* Authentication.swift in Sources */, @@ -5098,6 +5113,7 @@ CECC759923D6160000486676 /* AggregateDataHelperTests.swift in Sources */, 020BE76F23B4A468007FE54C /* AztecBlockquoteFormatBarCommandTests.swift in Sources */, 748C7784211E2D8400814F2C /* DoubleWooTests.swift in Sources */, + 028AFFB62484EDA000693C09 /* Dictionary+LoggingTests.swift in Sources */, 02A275BE23FE57DC005C560F /* ProductUIImageLoaderTests.swift in Sources */, 027B8BBF23FE0F850040944E /* MockMediaStoresManager.swift in Sources */, 453770D12431FF4700AC718D /* ProductSettingsViewModelTests.swift in Sources */, diff --git a/WooCommerce/WooCommerceTests/Tools/Logging/Dictionary+LoggingTests.swift b/WooCommerce/WooCommerceTests/Tools/Logging/Dictionary+LoggingTests.swift new file mode 100644 index 00000000000..573cc68ca93 --- /dev/null +++ b/WooCommerce/WooCommerceTests/Tools/Logging/Dictionary+LoggingTests.swift @@ -0,0 +1,48 @@ +import XCTest +@testable import WooCommerce + +final class Dictionary_LoggingTests: XCTestCase { + func testSerializingValuesForAnEmptyDictionary() { + // Arrange + let dictionary: [String: Any] = [:] + + // Action + let serializableDictionary = dictionary.serializeValuesForLoggingIfNeeded() + + // Assert + XCTAssertTrue(JSONSerialization.isValidJSONObject(serializableDictionary)) + XCTAssertEqual(serializableDictionary.count, 0) + } + + func testSerializingValuesForADictionaryWithNSErrors() { + // Arrange + let error = NSError(domain: "Testing crash logging in Sentry on Reviews tab launch", + code: 100, + userInfo: ["reason": "Testing only"]) + let error1Key = "Error 1" + let error2Key = "Error 2" + let dictionary: [String: Any] = [error1Key: error, error2Key: error, "message": "hello"] + + // Action + let serializableDictionary = dictionary.serializeValuesForLoggingIfNeeded() + + // Assert + XCTAssertTrue(JSONSerialization.isValidJSONObject(serializableDictionary)) + XCTAssertEqual(serializableDictionary.count, 3) + } + + func testSerializingValuesForADictionaryWithUnsupportedType() { + // Arrange + struct Value { + let message: String + } + let dictionary: [String: Any] = ["test": Value(message: "😃")] + + // Action + let serializableDictionary = dictionary.serializeValuesForLoggingIfNeeded() + + // Assert + XCTAssertTrue(JSONSerialization.isValidJSONObject(serializableDictionary)) + XCTAssertEqual(serializableDictionary.count, 0) + } +} From 149d64927cae9cf43e46a42183e2e44493b27a32 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 1 Jun 2020 16:30:51 +0800 Subject: [PATCH 06/17] Rename `DefaultCrashLogger` to `SentryCrashLogger`. --- WooCommerce/Classes/ServiceLocator/ServiceLocator.swift | 2 +- .../{DefaultCrashLogger.swift => SentryCrashLogger.swift} | 3 ++- WooCommerce/WooCommerce.xcodeproj/project.pbxproj | 8 ++++---- 3 files changed, 7 insertions(+), 6 deletions(-) rename WooCommerce/Classes/Tools/Logging/{DefaultCrashLogger.swift => SentryCrashLogger.swift} (88%) diff --git a/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift b/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift index c096a130d4b..948b107f4fe 100644 --- a/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift +++ b/WooCommerce/Classes/ServiceLocator/ServiceLocator.swift @@ -43,7 +43,7 @@ final class ServiceLocator { /// CoreData Stack /// - private static var _storageManager = CoreDataManager(name: WooConstants.databaseStackName, crashLogger: DefaultCrashLogger()) + private static var _storageManager = CoreDataManager(name: WooConstants.databaseStackName, crashLogger: SentryCrashLogger()) /// Cocoalumberjack DDLog /// diff --git a/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift b/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift similarity index 88% rename from WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift rename to WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift index 5f4f4823b8b..8dbd9adb846 100644 --- a/WooCommerce/Classes/Tools/Logging/DefaultCrashLogger.swift +++ b/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift @@ -2,7 +2,8 @@ import AutomatticTracks import Sentry import Storage -final class DefaultCrashLogger: CrashLogger { +/// 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)) } diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index 46af391f275..c250eae34fe 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -244,7 +244,7 @@ 02EA6BF82435E80600FFF90A /* ImageDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EA6BF72435E80600FFF90A /* ImageDownloader.swift */; }; 02EA6BFA2435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EA6BF92435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift */; }; 02EA6BFC2435EC3500FFF90A /* MockImageDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EA6BFB2435EC3500FFF90A /* MockImageDownloader.swift */; }; - 02EAB6D92480AA4900FD873C /* DefaultCrashLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EAB6D82480AA4900FD873C /* DefaultCrashLogger.swift */; }; + 02EAB6D92480AA4900FD873C /* SentryCrashLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EAB6D82480AA4900FD873C /* SentryCrashLogger.swift */; }; 02EEB5C42424AFAA00B8A701 /* TextFieldTableViewCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02EEB5C22424AFAA00B8A701 /* TextFieldTableViewCell.swift */; }; 02EEB5C52424AFAA00B8A701 /* TextFieldTableViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 02EEB5C32424AFAA00B8A701 /* TextFieldTableViewCell.xib */; }; 02F49ADA23BF356E00FA0BFA /* TitleAndTextFieldTableViewCell.ViewModel+State.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02F49AD923BF356E00FA0BFA /* TitleAndTextFieldTableViewCell.ViewModel+State.swift */; }; @@ -1101,7 +1101,7 @@ 02EA6BF72435E80600FFF90A /* ImageDownloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageDownloader.swift; sourceTree = ""; }; 02EA6BF92435E92600FFF90A /* KingfisherImageDownloader+ImageDownloadable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "KingfisherImageDownloader+ImageDownloadable.swift"; sourceTree = ""; }; 02EA6BFB2435EC3500FFF90A /* MockImageDownloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockImageDownloader.swift; sourceTree = ""; }; - 02EAB6D82480AA4900FD873C /* DefaultCrashLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultCrashLogger.swift; sourceTree = ""; }; + 02EAB6D82480AA4900FD873C /* SentryCrashLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryCrashLogger.swift; sourceTree = ""; }; 02EEB5C22424AFAA00B8A701 /* TextFieldTableViewCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextFieldTableViewCell.swift; sourceTree = ""; }; 02EEB5C32424AFAA00B8A701 /* TextFieldTableViewCell.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = TextFieldTableViewCell.xib; sourceTree = ""; }; 02F49AD923BF356E00FA0BFA /* TitleAndTextFieldTableViewCell.ViewModel+State.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TitleAndTextFieldTableViewCell.ViewModel+State.swift"; sourceTree = ""; }; @@ -3117,7 +3117,7 @@ children = ( 933A27362222354600C2143A /* Logging.swift */, B5DB01B42114AB2D00A4F797 /* CrashLogging.swift */, - 02EAB6D82480AA4900FD873C /* DefaultCrashLogger.swift */, + 02EAB6D82480AA4900FD873C /* SentryCrashLogger.swift */, 028AFFB22484ED2800693C09 /* Dictionary+Logging.swift */, ); path = Logging; @@ -4766,7 +4766,7 @@ B517EA1D218B41F200730EC4 /* String+Woo.swift in Sources */, 02564A8C246CE38E00D6DB2A /* SwappableSubviewContainerView.swift in Sources */, 024DF31223742B18006658FE /* AztecItalicFormatBarCommand.swift in Sources */, - 02EAB6D92480AA4900FD873C /* DefaultCrashLogger.swift in Sources */, + 02EAB6D92480AA4900FD873C /* SentryCrashLogger.swift in Sources */, D83F5933225B2EB900626E75 /* ManualTrackingViewController.swift in Sources */, B57C744A20F5649300EEFC87 /* EmptyStoresTableViewCell.swift in Sources */, 027D67D1245ADDF40036B8DB /* FilterTypeViewModel+Helpers.swift in Sources */, From 573bf0f94663732bf05cc4a9d2e7128df54469a3 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 1 Jun 2020 16:32:43 +0800 Subject: [PATCH 07/17] Rename `errorLoadingPersistentStores` to `persistentStoreLoadingError`. --- Storage/Storage/CoreData/CoreDataManager.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Storage/Storage/CoreData/CoreDataManager.swift b/Storage/Storage/CoreData/CoreDataManager.swift index 91e9e31b831..8c4f128f0ec 100644 --- a/Storage/Storage/CoreData/CoreDataManager.swift +++ b/Storage/Storage/CoreData/CoreDataManager.swift @@ -39,11 +39,11 @@ public class CoreDataManager: StorageManagerType { container.persistentStoreDescriptions = [storeDescription] container.loadPersistentStores { [weak self] (storeDescription, error) in - guard let `self` = self, let errorLoadingPersistentStores = error else { + guard let `self` = self, let persistentStoreLoadingError = error else { return } - DDLogError("⛔️ [CoreDataManager] loadPersistentStore failed. Attempting to recover... \(errorLoadingPersistentStores)") + DDLogError("⛔️ [CoreDataManager] loadPersistentStore failed. Attempting to recover... \(persistentStoreLoadingError)") /// Backup the old Store /// @@ -54,7 +54,7 @@ public class CoreDataManager: StorageManagerType { } catch { let message = "☠️ [CoreDataManager] Cannot backup Store: \(error)" self.crashLogger.logMessage(message, - properties: ["loadPersistentStoresError": errorLoadingPersistentStores, + properties: ["persistentStoreLoadingError": persistentStoreLoadingError, "backupError": error], level: .fatal) fatalError(message) @@ -67,7 +67,7 @@ public class CoreDataManager: StorageManagerType { } catch { let message = "☠️ [CoreDataManager] Cannot remove Store: \(error)" self.crashLogger.logMessage(message, - properties: ["loadPersistentStoresError": errorLoadingPersistentStores, + properties: ["persistentStoreLoadingError": persistentStoreLoadingError, "removeStoreError": error], level: .fatal) fatalError(message) @@ -82,7 +82,7 @@ public class CoreDataManager: StorageManagerType { let message = "☠️ [CoreDataManager] Recovery Failed! \(error) [\(error.userInfo)]" self?.crashLogger.logMessage(message, - properties: ["loadPersistentStoresError": errorLoadingPersistentStores, + properties: ["persistentStoreLoadingError": persistentStoreLoadingError, "retryError": error], level: .fatal) fatalError(message) From c4634acfcfd2b821fb832d8284a57489da952ffc Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 1 Jun 2020 16:34:24 +0800 Subject: [PATCH 08/17] Add a missing `tearDown`. --- .../StorageShippingSettingsServiceTests.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Yosemite/YosemiteTests/Tools/ShippingSettings/StorageShippingSettingsServiceTests.swift b/Yosemite/YosemiteTests/Tools/ShippingSettings/StorageShippingSettingsServiceTests.swift index f27a58bdcbb..8a5dee023a6 100644 --- a/Yosemite/YosemiteTests/Tools/ShippingSettings/StorageShippingSettingsServiceTests.swift +++ b/Yosemite/YosemiteTests/Tools/ShippingSettings/StorageShippingSettingsServiceTests.swift @@ -13,6 +13,11 @@ final class StorageShippingSettingsServiceTests: XCTestCase { storage = CoreDataManager(name: "WooCommerce", crashLogger: MockCrashLogger()) } + override func tearDown() { + storage = nil + super.tearDown() + } + func testInitialShippingSettings() { let service = StorageShippingSettingsService(siteID: sampleSiteID, storageManager: storage) XCTAssertNil(service.dimensionUnit) From 69542fe40dd434a46b8502a251e551930c3d2a5d Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Mon, 1 Jun 2020 16:43:14 +0800 Subject: [PATCH 09/17] Remove unnecessary import. --- Storage/Storage/Protocols/CrashLogger.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Storage/Storage/Protocols/CrashLogger.swift b/Storage/Storage/Protocols/CrashLogger.swift index 9015621c8a4..aceead13bbf 100644 --- a/Storage/Storage/Protocols/CrashLogger.swift +++ b/Storage/Storage/Protocols/CrashLogger.swift @@ -1,5 +1,3 @@ -//import Foundation - /// The level of severity, that is currently based on `SentrySeverity`. public enum SeverityLevel { case fatal From 3ffb980d6778144d43462ec31b5f69c5df369724 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Tue, 2 Jun 2020 12:46:34 +0800 Subject: [PATCH 10/17] Set the value to a string if not serializable. --- WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift | 1 + .../Tools/Logging/Dictionary+LoggingTests.swift | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift b/WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift index 748418f994a..42583171d44 100644 --- a/WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift +++ b/WooCommerce/Classes/Tools/Logging/Dictionary+Logging.swift @@ -25,6 +25,7 @@ extension Dictionary where Key == String { return formattedProperties } + formattedProperties[key] = "\(value)" return formattedProperties } } diff --git a/WooCommerce/WooCommerceTests/Tools/Logging/Dictionary+LoggingTests.swift b/WooCommerce/WooCommerceTests/Tools/Logging/Dictionary+LoggingTests.swift index 573cc68ca93..f8be1a96e50 100644 --- a/WooCommerce/WooCommerceTests/Tools/Logging/Dictionary+LoggingTests.swift +++ b/WooCommerce/WooCommerceTests/Tools/Logging/Dictionary+LoggingTests.swift @@ -43,6 +43,6 @@ final class Dictionary_LoggingTests: XCTestCase { // Assert XCTAssertTrue(JSONSerialization.isValidJSONObject(serializableDictionary)) - XCTAssertEqual(serializableDictionary.count, 0) + XCTAssertEqual(serializableDictionary.count, 1) } } From 121ba3fa0cf8b3ccb8f5892ca2f90e94edaecfaa Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Tue, 2 Jun 2020 13:13:18 +0800 Subject: [PATCH 11/17] Update `CrashLogger` to wait for the message to be sent, otherwise the event might be lost if it is followed by a `fatalError`. --- .../Storage/CoreData/CoreDataManager.swift | 24 ++++++------- Storage/Storage/Protocols/CrashLogger.swift | 4 +-- .../StorageTests/Mocks/MockCrashLogger.swift | 2 +- .../Tools/Logging/SentryCrashLogger.swift | 35 +++++++++++++++++-- .../Mockups/MockCrashLogger.swift | 2 +- 5 files changed, 49 insertions(+), 18 deletions(-) diff --git a/Storage/Storage/CoreData/CoreDataManager.swift b/Storage/Storage/CoreData/CoreDataManager.swift index 8c4f128f0ec..ba96e1bfc33 100644 --- a/Storage/Storage/CoreData/CoreDataManager.swift +++ b/Storage/Storage/CoreData/CoreDataManager.swift @@ -53,10 +53,10 @@ public class CoreDataManager: StorageManagerType { try FileManager.default.copyItem(at: sourceURL, to: backupURL) } catch { let message = "☠️ [CoreDataManager] Cannot backup Store: \(error)" - self.crashLogger.logMessage(message, - properties: ["persistentStoreLoadingError": persistentStoreLoadingError, - "backupError": error], - level: .fatal) + self.crashLogger.logMessageAndWait(message, + properties: ["persistentStoreLoadingError": persistentStoreLoadingError, + "backupError": error], + level: .fatal) fatalError(message) } @@ -66,10 +66,10 @@ public class CoreDataManager: StorageManagerType { 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) + self.crashLogger.logMessageAndWait(message, + properties: ["persistentStoreLoadingError": persistentStoreLoadingError, + "removeStoreError": error], + level: .fatal) fatalError(message) } @@ -81,10 +81,10 @@ public class CoreDataManager: StorageManagerType { } let message = "☠️ [CoreDataManager] Recovery Failed! \(error) [\(error.userInfo)]" - self?.crashLogger.logMessage(message, - properties: ["persistentStoreLoadingError": persistentStoreLoadingError, - "retryError": error], - level: .fatal) + self?.crashLogger.logMessageAndWait(message, + properties: ["persistentStoreLoadingError": persistentStoreLoadingError, + "retryError": error], + level: .fatal) fatalError(message) } } diff --git a/Storage/Storage/Protocols/CrashLogger.swift b/Storage/Storage/Protocols/CrashLogger.swift index aceead13bbf..4a86511bb40 100644 --- a/Storage/Storage/Protocols/CrashLogger.swift +++ b/Storage/Storage/Protocols/CrashLogger.swift @@ -10,11 +10,11 @@ public enum SeverityLevel { /// Logs crashes or messages at a given severity level. public protocol CrashLogger { /** - Writes a message to the Crash Logging system. + Writes a message to the Crash Logging system and waits until the message is sent. - 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) + func logMessageAndWait(_ message: String, properties: [String: Any]?, level: SeverityLevel) } diff --git a/Storage/StorageTests/Mocks/MockCrashLogger.swift b/Storage/StorageTests/Mocks/MockCrashLogger.swift index a8c24ed12fd..32c6dd5b74a 100644 --- a/Storage/StorageTests/Mocks/MockCrashLogger.swift +++ b/Storage/StorageTests/Mocks/MockCrashLogger.swift @@ -1,7 +1,7 @@ import Storage struct MockCrashLogger: CrashLogger { - func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) { + func logMessageAndWait(_ message: String, properties: [String: Any]?, level: SeverityLevel) { // no-op } } diff --git a/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift b/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift index 8dbd9adb846..2eb8ac17f7c 100644 --- a/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift +++ b/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift @@ -4,8 +4,39 @@ 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)) + func logMessageAndWait(_ message: String, properties: [String: Any]?, level: SeverityLevel) { + CrashLogging.logMessageAndWait(message, properties: properties?.serializeValuesForLoggingIfNeeded(), level: SentrySeverity(level: level)) + } +} + +private extension CrashLogging { + /** + Mostly similar to `logMessage(_:properties:level:)`, but this function blocks the thread until the event is fired. + - Parameters: + - message: The message + - properties: A dictionary containing additional information about this error + - level: The level of severity to report in Sentry + */ + static func logMessageAndWait(_ message: String, properties: [String: Any]?, level: SentrySeverity) { + let event = Event(level: level) + event.message = message + event.extra = properties + + Client.shared?.snapshotStacktrace { + Client.shared?.appendStacktrace(to: event) + } + + guard let client = Client.shared else { + return + } + + let semaphore = DispatchSemaphore(value: 0) + + client.send(event: event) { _ in + semaphore.signal() + } + + semaphore.wait() } } diff --git a/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift index a8c24ed12fd..32c6dd5b74a 100644 --- a/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift +++ b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift @@ -1,7 +1,7 @@ import Storage struct MockCrashLogger: CrashLogger { - func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) { + func logMessageAndWait(_ message: String, properties: [String: Any]?, level: SeverityLevel) { // no-op } } From 4a6709410c65b90d6e0768796efeee0775738190 Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Tue, 2 Jun 2020 13:37:59 +0800 Subject: [PATCH 12/17] Log message when the app recovered from persistent store loading error. --- Storage/Storage/CoreData/CoreDataManager.swift | 5 +++++ Storage/Storage/Protocols/CrashLogger.swift | 9 +++++++++ Storage/StorageTests/Mocks/MockCrashLogger.swift | 4 ++++ .../Classes/Tools/Logging/SentryCrashLogger.swift | 4 ++++ Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift | 4 ++++ 5 files changed, 26 insertions(+) diff --git a/Storage/Storage/CoreData/CoreDataManager.swift b/Storage/Storage/CoreData/CoreDataManager.swift index ba96e1bfc33..13bb81ad648 100644 --- a/Storage/Storage/CoreData/CoreDataManager.swift +++ b/Storage/Storage/CoreData/CoreDataManager.swift @@ -87,6 +87,11 @@ public class CoreDataManager: StorageManagerType { level: .fatal) fatalError(message) } + + self.crashLogger.logMessage("[CoreDataManager] Recovered from persistent store loading error", + properties: ["persistentStoreLoadingError": persistentStoreLoadingError, + "appState": UIApplication.shared.applicationState.rawValue], + level: .info) } return container diff --git a/Storage/Storage/Protocols/CrashLogger.swift b/Storage/Storage/Protocols/CrashLogger.swift index 4a86511bb40..5339c2389b7 100644 --- a/Storage/Storage/Protocols/CrashLogger.swift +++ b/Storage/Storage/Protocols/CrashLogger.swift @@ -9,6 +9,15 @@ public enum SeverityLevel { /// 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) + /** Writes a message to the Crash Logging system and waits until the message is sent. - Parameters: diff --git a/Storage/StorageTests/Mocks/MockCrashLogger.swift b/Storage/StorageTests/Mocks/MockCrashLogger.swift index 32c6dd5b74a..d398997e201 100644 --- a/Storage/StorageTests/Mocks/MockCrashLogger.swift +++ b/Storage/StorageTests/Mocks/MockCrashLogger.swift @@ -1,6 +1,10 @@ import Storage struct MockCrashLogger: CrashLogger { + func logMessage(_ message: String, properties: [String : Any]?, level: SeverityLevel) { + // no-op + } + func logMessageAndWait(_ message: String, properties: [String: Any]?, level: SeverityLevel) { // no-op } diff --git a/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift b/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift index 2eb8ac17f7c..2f803847368 100644 --- a/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift +++ b/WooCommerce/Classes/Tools/Logging/SentryCrashLogger.swift @@ -4,6 +4,10 @@ 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)) + } + func logMessageAndWait(_ message: String, properties: [String: Any]?, level: SeverityLevel) { CrashLogging.logMessageAndWait(message, properties: properties?.serializeValuesForLoggingIfNeeded(), level: SentrySeverity(level: level)) } diff --git a/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift index 32c6dd5b74a..d398997e201 100644 --- a/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift +++ b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift @@ -1,6 +1,10 @@ import Storage struct MockCrashLogger: CrashLogger { + func logMessage(_ message: String, properties: [String : Any]?, level: SeverityLevel) { + // no-op + } + func logMessageAndWait(_ message: String, properties: [String: Any]?, level: SeverityLevel) { // no-op } From b16bab2be5014ee1bc4f70d1b7fe3aa1e9a292eb Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Tue, 2 Jun 2020 13:39:07 +0800 Subject: [PATCH 13/17] Also log app state. --- Storage/Storage/CoreData/CoreDataManager.swift | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Storage/Storage/CoreData/CoreDataManager.swift b/Storage/Storage/CoreData/CoreDataManager.swift index 13bb81ad648..a1380cf4e2d 100644 --- a/Storage/Storage/CoreData/CoreDataManager.swift +++ b/Storage/Storage/CoreData/CoreDataManager.swift @@ -55,7 +55,8 @@ public class CoreDataManager: StorageManagerType { let message = "☠️ [CoreDataManager] Cannot backup Store: \(error)" self.crashLogger.logMessageAndWait(message, properties: ["persistentStoreLoadingError": persistentStoreLoadingError, - "backupError": error], + "backupError": error, + "appState": UIApplication.shared.applicationState.rawValue], level: .fatal) fatalError(message) } @@ -68,7 +69,8 @@ public class CoreDataManager: StorageManagerType { let message = "☠️ [CoreDataManager] Cannot remove Store: \(error)" self.crashLogger.logMessageAndWait(message, properties: ["persistentStoreLoadingError": persistentStoreLoadingError, - "removeStoreError": error], + "removeStoreError": error, + "appState": UIApplication.shared.applicationState.rawValue], level: .fatal) fatalError(message) } @@ -83,7 +85,8 @@ public class CoreDataManager: StorageManagerType { let message = "☠️ [CoreDataManager] Recovery Failed! \(error) [\(error.userInfo)]" self?.crashLogger.logMessageAndWait(message, properties: ["persistentStoreLoadingError": persistentStoreLoadingError, - "retryError": error], + "retryError": error, + "appState": UIApplication.shared.applicationState.rawValue], level: .fatal) fatalError(message) } From 6a6b4e30901094b961e70fa1f11251b639c3cc1a Mon Sep 17 00:00:00 2001 From: Jaclyn Chen Date: Tue, 2 Jun 2020 13:43:06 +0800 Subject: [PATCH 14/17] Fix lint warnings. --- Storage/StorageTests/Mocks/MockCrashLogger.swift | 2 +- Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Storage/StorageTests/Mocks/MockCrashLogger.swift b/Storage/StorageTests/Mocks/MockCrashLogger.swift index d398997e201..b93d3f830e8 100644 --- a/Storage/StorageTests/Mocks/MockCrashLogger.swift +++ b/Storage/StorageTests/Mocks/MockCrashLogger.swift @@ -1,7 +1,7 @@ import Storage struct MockCrashLogger: CrashLogger { - func logMessage(_ message: String, properties: [String : Any]?, level: SeverityLevel) { + func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) { // no-op } diff --git a/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift index d398997e201..b93d3f830e8 100644 --- a/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift +++ b/Yosemite/YosemiteTests/Mockups/MockCrashLogger.swift @@ -1,7 +1,7 @@ import Storage struct MockCrashLogger: CrashLogger { - func logMessage(_ message: String, properties: [String : Any]?, level: SeverityLevel) { + func logMessage(_ message: String, properties: [String: Any]?, level: SeverityLevel) { // no-op } From a36786187e2c3a598a7d1e2ee57b7e92aec7edc0 Mon Sep 17 00:00:00 2001 From: Lorenzo Mattei Date: Wed, 3 Jun 2020 16:36:30 +0200 Subject: [PATCH 15/17] Update relase notes for 4.4 --- WooCommerce/Resources/release_notes.txt | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/WooCommerce/Resources/release_notes.txt b/WooCommerce/Resources/release_notes.txt index 6a734c25df4..c9ca2221410 100644 --- a/WooCommerce/Resources/release_notes.txt +++ b/WooCommerce/Resources/release_notes.txt @@ -1,4 +1,12 @@ -* You can now turn Products M2 Features on and off for simple products in Settings > Experimental Features. Products M2 features the ability to update product images, product short description, product viewing, sharing, and settings (Product settings include status, visibility, catalog visibility, slug, purchase note, and menu order.) -* You can now edit and save details on the Order details or Top Performers tabs. We’ve also improved accessibility with better VoiceOver support for Product Price settings. -* We’ve made some visual refinements: In Order Details, the Payment card now appears right after the Products and Refunded Products cards. The WIP banner on the Products tab is now collapsed by default. Yay for more vertical space! -* We’ve dropped support for iOS 11 and lower. +We’re continuing to add functionality to product editing. Look out for improvements to image and product management — head to “Settings” in the My Store tab to learn more and turn on the new features! + +Push notifications and update badges work better together. The Reviews tab no longer shows a badge after you receive a new order push notification, and navigating to the Orders and/or the Reviews tab now clears the badge number (depending on the types of notifications). + +Nudges are a bit less nudge-y. When editing products, the “discard changes” prompt now only appears if you’ve deleted images, and not when you’re leaving product settings detail screens with a text field (slug, purchase note, and menu order). + +We also straightened out a few little UI foibles: +- The product name displays in the product details navigation bar, so it’s always visible. +- We fixed an issue that prevented the product details from scrolling all the way to the bottom in landscape mode after dismissing the keyboard. +- Images pending upload will be visible after editing product images from product details. +- The “View product in store” action displays only if the product is published. +- HTML codes in the shipping method are now shown correctly. From 03744d8fc953f5e5bab839d903be26ef180ac451 Mon Sep 17 00:00:00 2001 From: Lorenzo Mattei Date: Wed, 3 Jun 2020 16:36:50 +0200 Subject: [PATCH 16/17] Update metadata strings --- WooCommerce/Resources/AppStoreStrings.pot | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/WooCommerce/Resources/AppStoreStrings.pot b/WooCommerce/Resources/AppStoreStrings.pot index ef48c3d1408..853404d0eb3 100644 --- a/WooCommerce/Resources/AppStoreStrings.pot +++ b/WooCommerce/Resources/AppStoreStrings.pot @@ -47,12 +47,20 @@ msgctxt "app_store_promo_text" msgid "Run your store wherever you are. The WooCommerce app makes it easy to manage orders and inventory, track sales, and monitor store activity like new orders and reviews." msgstr "" -msgctxt "v4.3-whats-new" +msgctxt "v4.4-whats-new" msgid "" -"* You can now turn Products M2 Features on and off for simple products in Settings > Experimental Features. Products M2 features the ability to update product images, product short description, product viewing, sharing, and settings (Product settings include status, visibility, catalog visibility, slug, purchase note, and menu order.)\n" -"* You can now edit and save details on the Order details or Top Performers tabs. We’ve also improved accessibility with better VoiceOver support for Product Price settings.\n" -"* We’ve made some visual refinements: In Order Details, the Payment card now appears right after the Products and Refunded Products cards. The WIP banner on the Products tab is now collapsed by default. Yay for more vertical space!\n" -"* We’ve dropped support for iOS 11 and lower.\n" +"We’re continuing to add functionality to product editing. Look out for improvements to image and product management — head to “Settings” in the My Store tab to learn more and turn on the new features!\n" +"\n" +"Push notifications and update badges work better together. The Reviews tab no longer shows a badge after you receive a new order push notification, and navigating to the Orders and/or the Reviews tab now clears the badge number (depending on the types of notifications).\n" +"\n" +"Nudges are a bit less nudge-y. When editing products, the “discard changes” prompt now only appears if you’ve deleted images, and not when you’re leaving product settings detail screens with a text field (slug, purchase note, and menu order).\n" +"\n" +"We also straightened out a few little UI foibles:\n" +"- The product name displays in the product details navigation bar, so it’s always visible.\n" +"- We fixed an issue that prevented the product details from scrolling all the way to the bottom in landscape mode after dismissing the keyboard.\n" +"- Images pending upload will be visible after editing product images from product details.\n" +"- The “View product in store” action displays only if the product is published.\n" +"- HTML codes in the shipping method are now shown correctly.\n" msgstr "" #. translators: This is a promo message that will be attached on top of a screenshot in the App Store. From 8f86fc7b3b6c1f5bbbb319efa92ecb49b185e509 Mon Sep 17 00:00:00 2001 From: Paolo Musolino Date: Wed, 3 Jun 2020 18:03:36 +0200 Subject: [PATCH 17/17] fix: failing tests --- Storage/Storage/CoreData/CoreDataManager.swift | 1 + .../YosemiteTests/Storage/StorageManagerConcurrencyTests.swift | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Storage/Storage/CoreData/CoreDataManager.swift b/Storage/Storage/CoreData/CoreDataManager.swift index a1380cf4e2d..4ae81d55834 100644 --- a/Storage/Storage/CoreData/CoreDataManager.swift +++ b/Storage/Storage/CoreData/CoreDataManager.swift @@ -15,6 +15,7 @@ public class CoreDataManager: StorageManagerType { /// Designated Initializer. /// /// - Parameter name: Identifier to be used for: [database, data model, container]. + /// - Parameter crashLogger: allows logging a message of any severity level /// /// - Important: This should *match* with your actual Data Model file!. /// diff --git a/Yosemite/YosemiteTests/Storage/StorageManagerConcurrencyTests.swift b/Yosemite/YosemiteTests/Storage/StorageManagerConcurrencyTests.swift index 00418d36498..2cbb885f0fc 100644 --- a/Yosemite/YosemiteTests/Storage/StorageManagerConcurrencyTests.swift +++ b/Yosemite/YosemiteTests/Storage/StorageManagerConcurrencyTests.swift @@ -63,7 +63,7 @@ final class StorageManagerConcurrencyTests: XCTestCase { override func setUp() { super.setUp() // Use the Sqlite-based StorageManagerType to be closer to the production operations - storageManager = CoreDataManager(name: "WooCommerce") + storageManager = CoreDataManager(name: "WooCommerce", crashLogger: MockCrashLogger()) storageManager.reset() }