From bf7951a1b8f3e9b278bb7942e379f5bba4296e46 Mon Sep 17 00:00:00 2001 From: Alex Kirk Date: Wed, 1 Apr 2026 00:02:01 +0200 Subject: [PATCH 1/2] Fix app crash when assignments cache file is empty or corrupted 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 --- .../experimentation/local/FileBasedCache.kt | 29 ++++++++++++++----- .../local/FileBasedCacheTest.kt | 25 ++++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/experimentation/src/main/java/com/automattic/android/experimentation/local/FileBasedCache.kt b/experimentation/src/main/java/com/automattic/android/experimentation/local/FileBasedCache.kt index 2fe3895..765a072 100644 --- a/experimentation/src/main/java/com/automattic/android/experimentation/local/FileBasedCache.kt +++ b/experimentation/src/main/java/com/automattic/android/experimentation/local/FileBasedCache.kt @@ -31,20 +31,33 @@ internal class FileBasedCache( init { scope.launch { withContext(context = dispatcher) { - latestMutable = getAssignments() + runCatching { latestMutable = getAssignments() } + .onFailure { logger.e("Failed to load cached assignments: $it") } } } } suspend fun getAssignments(): Assignments? { return withContext(dispatcher) { - assignmentsFile.takeIf { it.exists() }?.readText()?.let { json: String -> - val fromJson = cacheDtoJsonAdapter.fromJson(json) ?: return@let null - - fromJson.assignmentsDto.toAssignments( - fetchedAt = fromJson.fetchedAt, - anonymousId = fromJson.anonymousId, - ) + assignmentsFile.takeIf { it.exists() }?.let { file -> + val json = file.readText() + if (json.isBlank()) { + logger.e("Cached assignments file is empty, deleting: ${file.path}") + file.delete() + return@withContext null + } + runCatching { cacheDtoJsonAdapter.fromJson(json) } + .onFailure { + logger.e("Cached assignments file is corrupted, deleting: ${file.path}") + file.delete() + } + .getOrNull() + ?.let { fromJson -> + fromJson.assignmentsDto.toAssignments( + fetchedAt = fromJson.fetchedAt, + anonymousId = fromJson.anonymousId, + ) + } } } } diff --git a/experimentation/src/test/java/com/automattic/android/experimentation/local/FileBasedCacheTest.kt b/experimentation/src/test/java/com/automattic/android/experimentation/local/FileBasedCacheTest.kt index 166d52c..2ed78e9 100644 --- a/experimentation/src/test/java/com/automattic/android/experimentation/local/FileBasedCacheTest.kt +++ b/experimentation/src/test/java/com/automattic/android/experimentation/local/FileBasedCacheTest.kt @@ -9,6 +9,7 @@ import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Rule import org.junit.Test @@ -66,6 +67,30 @@ internal class FileBasedCacheTest { sut.clear() } + @Test + fun `getting assignments from empty file returns null and deletes the file`() = runTest { + val cacheDir = tempDir.newFolder() + val assignmentsFile = File(cacheDir, "assignments.json").apply { createNewFile() } + val sut = fileBasedCache(this, cacheDir = cacheDir) + + val result = sut.getAssignments() + + assertNull(result) + assertFalse(assignmentsFile.exists()) + } + + @Test + fun `getting assignments from corrupted file returns null and deletes the file`() = runTest { + val cacheDir = tempDir.newFolder() + val assignmentsFile = File(cacheDir, "assignments.json").apply { writeText("{corrupted") } + val sut = fileBasedCache(this, cacheDir = cacheDir) + + val result = sut.getAssignments() + + assertNull(result) + assertFalse(assignmentsFile.exists()) + } + @Test fun `saving cache when cache dir doesnt exist is successful`() = runTest { val cacheDir = File(tempDir.newFolder(), "cache").apply { assert(!this.exists()) } From 00b84eebd084ed70975ad3ae43a0c96037328237 Mon Sep 17 00:00:00 2001 From: Alex Kirk Date: Wed, 1 Apr 2026 12:45:59 +0200 Subject: [PATCH 2/2] Pass throwable to logger.e instead of interpolating into message Preserves stack traces in logs when cache reads fail. --- .../android/experimentation/local/FileBasedCache.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/experimentation/src/main/java/com/automattic/android/experimentation/local/FileBasedCache.kt b/experimentation/src/main/java/com/automattic/android/experimentation/local/FileBasedCache.kt index 765a072..580adc7 100644 --- a/experimentation/src/main/java/com/automattic/android/experimentation/local/FileBasedCache.kt +++ b/experimentation/src/main/java/com/automattic/android/experimentation/local/FileBasedCache.kt @@ -32,7 +32,7 @@ internal class FileBasedCache( scope.launch { withContext(context = dispatcher) { runCatching { latestMutable = getAssignments() } - .onFailure { logger.e("Failed to load cached assignments: $it") } + .onFailure { throwable -> logger.e("Failed to load cached assignments", throwable) } } } } @@ -47,8 +47,8 @@ internal class FileBasedCache( return@withContext null } runCatching { cacheDtoJsonAdapter.fromJson(json) } - .onFailure { - logger.e("Cached assignments file is corrupted, deleting: ${file.path}") + .onFailure { throwable -> + logger.e("Cached assignments file is corrupted, deleting: ${file.path}", throwable) file.delete() } .getOrNull()