Log messages/crashes in CoreDataManager in Storage framework

[jaclync]

Fixes #2364

Background

Since only WooCommerce framework knows about AutomatticTracks framework and thus the CrashLogging helper is only accessible within WooCommerce. However, there are also cases we'd like to log crashes/messages in frameworks other than WooCommerce like Storage and Yosemite frameworks for any potential crashes (ref p91TBi-2PM-p2). This PR created a protocol for WooCommerce to implement with AutomatticTracks's CrashLogging via Sentry, and pass this implementation to CoreDataManager as a start.

Changes

• Created a CrashLogger protocol in the Storage framework that allows logging a message of any severity level
  • The naming is just following the implementation CrashLogging in AutomatticTracks, please lemme know if you think some other name would be more clear
• Added CrashLogger dependency to CoreDataManager
• Created SentryCrashLogger to implement CrashLogger with AutomatticTracks's CrashLogging, and passed this implementation to CoreDataManager in ServiceLocator
• Separated copying and removing the old persistent store so that we have more insight about where the error comes from fatalError (in case we can't get logging from Sentry)
• Updated unit tests in Storage and Yosemite with MockCrashLogger mock

Testing

You can try putting a similar call to CrashLogger in CoreDataManager's loadPersistentStores' success block followed by a fatalError, then build with "Release-Alpha" configuration, and check on Sentry for the event.

Update release notes:

• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[peril-woocommerce]

You can trigger an installable build for these changes by visiting CircleCI here.

[peril-woocommerce]

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

[pmusolino]

I really hope this will help us to investigate better all the crashes in Storage 💯

[pmusolino]

good idea to separate the removeItem: from the copyItem: 👍

[shiki]

Related to #2371

[shiki]

Hey, @jaclync! Thanks for working on this. 🙇

I have not fully tested this yet. I still have to do the installable build route. I'm submitting this review since there's one thing that I think needs your attention (marked with ⚠️ ).

[jaclync]

👋 Platform 9 @oguzkocer @loremattei since we'd like to understand more about one of the top crashes in WCiOS (p91TBi-2PM-p2) that isn't currently logged on Sentry yet, it'd be great if this PR can be merged to release 4.4 when it's ready!

[jaclync]

@shiki thanks so much for catching the JSON serialization issue! that was the reason why the test events weren't logged to Sentry before. I responded to your comments for another pass 😄

[shiki]

Hey, @jaclync! Thanks for the changes. I was finally able to test this in Sentry.

I have a few additional comments.

Unrecoverable Crash

I'd like to double-check this with you. I tested the crash by adding using this in lazy var persistentContainer:

        container.loadPersistentStores { [weak self] (storeDescription, error) in
            guard let self = self else {
                return
            }
            let persistentStoreLoadingError = NSError(domain: "test by shiki", code: 9_999, userInfo: nil)

I used WooCommerce Alpha and Release-Alpha Build Configuration.

I then ran the app. Stopped Xcode debug when it crashed. And then I opened the app multiple times in the simulator to make it crash a few more times.

I was hoping that the events will turn up in Sentry. I waited for 5 minutes but they never did.

So, I removed the code above so the app will not crash anymore. And that's when the crashes turned up in Sentry. This hints that Sentry never got a chance to submit the events to the API because the app crashed right away. I believe this is what will happen in production too. We will still not receive the crashes in Sentry because the app keeps crashing immediately after opening.

This is all just coming back to me, my apologies. I now remember that we implemented a thread-blocking submission of events in WPiOS so that the Sentry client will have a chance to upload the events to the API.

We can implement the same thing here. Possibly just replacing the logMessage calls with the logErrorAndWait() from WP.

This is not ideal. I still believe that we should not crash and show a message to the user instead. But in the meantime, we can go with the forced logging.

[jaclync]

I used WooCommerce Alpha and Release-Alpha Build Configuration.
I then ran the app. Stopped Xcode debug when it crashed. And then I opened the app multiple times in the simulator to make it crash a few more times.
I was hoping that the events will turn up in Sentry. I waited for 5 minutes but they never did.
So, I removed the code above so the app will not crash anymore. And that's when the crashes turned up in Sentry. This hints that Sentry never got a chance to submit the events to the API because the app crashed right away. I believe this is what will happen in production too. We will still not receive the crashes in Sentry because the app keeps crashing immediately after opening.
This is all just coming back to me, my apologies. I now remember that we implemented a thread-blocking submission of events in WPiOS so that the Sentry client will have a chance to upload the events to the API.

this is a really good point, I didn't think of this case and test with a fatalError after the logging call. I updated the logging that is followed by a fatalError to be logErrorAndWait like in WPiOS in 121ba3f

Additionally, I added the application state to the logging in b16bab2 since this could help us understand more about the permission issues based on Jeremy's feedback in p91TBi-2R9-p2

Ready for another pass and lemme know if you still don't see the events on Sentry!

[shiki]

🚀

Thank you for working on this, @jaclync!

I retested the flow I mentioned yesterday and it now works!

[jaclync]

@loremattei merging this PR to release 4.4 now for the next beta build, thanks for wrangling the release!

[loremattei]

@jaclync perfect! Thank you!
I'm going to release a new beta tomorrow!