-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Dashboard] Add corruption handlers to Data Stores using Protocol Buffer schemas. #13058
[Dashboard] Add corruption handlers to Data Stores using Protocol Buffer schemas. #13058
Conversation
…Handler is called
…orruption Handler is called
…tion Handler is called
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
@malinajirka Regarding the hotfix procedures for this PR, given we're mid week already and a few days away from the Play Store submission, I'm considering avoiding a beta cut and letting go of the release right away instead. WDYT? |
Also, to follow up on what we discussed in peaMlT-XN#comment-2680, I verified the Crash Logging injection instance during the app startup, and it's working as expected. We shouldn't see any issues, just as you predicted. |
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @ThomazFB !
I've tested the following flows:
- Corrupted - /data/data/com.woocommerce.android.dev/files/datastore/custom_date_range_configuration.preferences_pb
- Corrupted - /data/data/com.woocommerce.android.dev/files/datastore/dashboard_configuration_275
- Corrupted - /data/data/com.woocommerce.android.dev/files/datastore/dashboard_coupons_custom_date_range_configuration.preferences_pb
- Corrupted - /data/data/com.woocommerce.android.dev/files/datastore/tracker.preferences_pb
- Login on clear app installation
All of them worked as expected, even those which are loaded on startup seem to have Sentry initialized. Having said that, I haven't tested the events get actually sent to Sentry - I'm having troubles enabling Sentry on a debuggable build and I can't corrupt the files on a production build.
I think it would be good to include flows that you tested in the description. Feel free to check the Manual Testing: The author listed all the...
checkbox after you update the description. Thanks!
Regarding the hotfix procedures for this PR, given we're mid week already and a few days away from the Play Store submission, I'm considering avoiding a beta cut and letting go of the release right away instead. WDYT?
Considering the next release is on Monday, which is still several days away and more importantly, this change might potentially result in crashes on startup, I'd definitely suggest releasing a new beta. @toupper let us know in case you'd have a different opinion.
A new beta, including this fix, has been published with |
Note: This change was originally implemented here, but we recreated it since we decided to branch it off a release branch. Adding it here for reference, since the original PR contains discussions.
Summary
This is a follow-up to #12891, where reverting the Data Store library upgrade didn't clear the corrupted files to some users. We verified that the corruption exception we're getting is very likely to be connected with a parsing failure of the Protocol Buffer schemas we have in the app, affecting two Data Stores:
Dashboard
andCustomDateRange
.This PR adds a corruption handler to the DataStore creation, which simply restarts the DataStore with the default instance of each schema.
Additionally, this PR also updates all the Preferences DataStores used through the app to recover automatically and report the corruption event to Sentry.
Steps to Reproduce
Copy this file to your device in
/data/data/com.woocommerce.android.dev/files/datastore
.Most active coupons
cardUpdate release notes:
RELEASE-NOTES.txt
if necessary.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: