Exception handling when reading datastore.

[nur-shuvo]

What I have done and why

Datastore file can be corrupted in cases, or simply invalid. So there needs to handle runtime Fatal exception like I found:

FATAL EXCEPTION: main
Process: com.google.samples.apps.nowinandroid.demo.debug, PID: 10565
androidx.datastore.core.CorruptionException: Cannot read proto.
...

So exception handling is needed, and replaced with UserPreferences.getDefaultInstance() when there occurs such exception

[SimonMarquis]

It would be great to add tests for this new code.
Also, using the Android Log here means we won't be able to run standard JVM unit test 😕

[nur-shuvo]

It would be great to add tests for this new code. Also, using the Android Log here means we won't be able to run standard JVM unit test 😕

I will try to add tests, though it seems that corrupting the file in unit test environment is challenging, maybe I might have to write androidTest. I will look into this. Thank you for your review. @SimonMarquis

[SimonMarquis]

I'm not sure exactly how for now but I think you can fake the datastore and throw the IOException from the loading method.

[hoc081098]

Nice

[nur-shuvo]

I'm not sure exactly how for now but I think you can fake the datastore and throw the IOException from the loading method.

Added unit tests by making fake datastore which throws exception while accessing. Then testing data source whether returning the default values or not.

[hoc081098]

LGTM

[nur-shuvo]

@dturner
Hello.
Could you please review this PR at your convenient?

[dturner]

If datastore isn't available, returning a default UserPreferences isn't the answer. The whole point of the app is that users can save the topics they're interested in and only see news relating to those topics (plus bookmarks etc). Without working data storage, none of that is possible. It's preferable to have the app crash on startup than have users be fooled into thinking it's working when it's not.

What am I missing?

[nur-shuvo]

If datastore isn't available, returning a default UserPreferences isn't the answer. The whole point of the app is that users can save the topics they're interested in and only see news relating to those topics (plus bookmarks etc). Without working data storage, none of that is possible. It's preferable to have the app crash on startup than have users be fooled into thinking it's working when it's not.

What am I missing?

I think application will continuously crash on startUp if this exception unhandled. User may need to clear data forcefully, or uninstalled to recover from this unexpected situation. That's why I put the default UserPreferences and restored to something where user can fresh start using the app.

[SimonMarquis]

@dturner it is not necessarily that "datastore" is not available, but rather that the datastore files are corrupted, and not recoverable.
You can take a look at this report https://issuetracker.google.com/issues/346197747 that hit many of us, for unknown reasons. The only resort for users is to clear app data, or reinstall app, which would be equivalent to restoring an empty datastore file. We will lose the user data anyways.

[nur-shuvo]

@dturner
I would be happy to know your opinions regarding.

[dturner]

Thanks for the additional info, that makes sense. I'll need some time to consider the suggested approach before providing a review. Unfortunately I'm on vacation for a couple of weeks now so it'll likely be mid Jan 2025 now.