Fix crash loop caused by corrupted A/B testing cache file#297
Fix crash loop caused by corrupted A/B testing cache file#297ParaskP7 merged 2 commits intoAutomattic:trunkfrom
Conversation
An interrupted write to assignments.json (e.g. app killed mid-write) leaves an empty or truncated file. On next launch, Moshi throws EOFException parsing it, which propagates out of the FileBasedCache init coroutine and crashes the app in a loop that Clear Cache cannot fix (file lives in filesDir, not cacheDir). - Treat empty files as a cache miss and delete them - Catch JSON parse failures, log and delete the corrupted file - Wrap the init block's coroutine in runCatching so any future read failure degrades gracefully instead of crashing the process
There was a problem hiding this comment.
Pull request overview
Fixes a crash loop in the experimentation cache layer by treating empty or unreadable assignments.json as a cache miss, deleting the bad file, and allowing assignments to be re-fetched instead of crashing on startup.
Changes:
- Harden
FileBasedCache.getAssignments()to delete and ignore empty (isBlank) cache files. - Catch JSON parsing failures when reading cached assignments, delete the corrupted file, and return
null. - Add unit tests covering empty and corrupted cache-file scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| experimentation/src/main/java/com/automattic/android/experimentation/local/FileBasedCache.kt | Adds defensive handling for empty/corrupt cache content and wraps init-time load in runCatching to avoid process crashes. |
| experimentation/src/test/java/com/automattic/android/experimentation/local/FileBasedCacheTest.kt | Adds tests ensuring empty/corrupted cache files return null and are deleted. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| withContext(context = dispatcher) { | ||
| latestMutable = getAssignments() | ||
| runCatching { latestMutable = getAssignments() } | ||
| .onFailure { logger.e("Failed to load cached assignments: $it") } |
There was a problem hiding this comment.
runCatching failure logging interpolates the Throwable into the message but doesn’t pass it to logger.e(...). Since ExperimentLogger.e supports a throwable parameter, pass the exception so stacktraces are preserved (and avoid relying on Throwable.toString() in the message).
| .onFailure { logger.e("Failed to load cached assignments: $it") } | |
| .onFailure { throwable -> logger.e("Failed to load cached assignments", throwable) } |
| runCatching { cacheDtoJsonAdapter.fromJson(json) } | ||
| .onFailure { | ||
| logger.e("Cached assignments file is corrupted, deleting: ${file.path}") | ||
| file.delete() | ||
| } |
There was a problem hiding this comment.
When JSON parsing fails, the log message doesn’t include the underlying exception (it’s not passed to logger.e). Passing the Throwable will make it easier to diagnose which parsing failure occurred (EOF vs JsonDataException, etc.) while still deleting the file as a cache miss.
Preserves stack traces in logs when cache reads fail.
|
👋 @akirk ! Thank you so much for your contribution to I have reviewed this and everything LGTM. I also triggered CI on it via a #298 PR, which I just pushed on the main repo, everything works as expected, thanks for making this A/B testing experience better for everyone! 🌟 Things Worth Noting
Overall, this is a 👍 from my side and if @MiSikora doesn't have anything more to share, let's merge this and create a new version of |
MiSikora
left a comment
There was a problem hiding this comment.
Looks good, no objections.
Problem
My Pocket Casts has been stuck in a crash loop. Every launch fails with:
After digging through the source, I found that
assignments.jsoninfiles/experiments/must have been left empty by an interrupted write. The stack trace makes this clear:FileNotFoundExceptionwould mean the file doesn't exist; a partial/truncated JSON would giveJsonDataException.EOFException: End of inputis specifically what Moshi throws whenfromJson()receives an empty string — meaningreadText()succeeded but returned"".The reason I can't recover without a code fix: the file lives in
filesDir, notcacheDir, so clearing the app cache does nothing. Clearing app data would fix the crash but I have starred episodes stored locally that are no longer available in the feed — that data would be gone permanently.Fix
Treat an empty or unparseable file as a cache miss, delete it, and let the library re-fetch from the server. I also wrapped the
initcoroutine inrunCatchingso future read failures degrade gracefully rather than crashing the process.Follow-up
Worth discussing whether this file should live in
cacheDirinstead offilesDir— it's a network cache that can always be re-fetched, and usingcacheDirwould mean Clear Cache recovers users from this situation automatically in the future.Test plan
assignments.jsonreturnsnulland deletes the fileassignments.jsonreturnsnulland deletes the file