From b6a2d1afedc53d3728c330a88a8d4435393f1220 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Sat, 4 Jul 2026 14:50:19 +0530 Subject: [PATCH] fix(android): harden download polling against DB crashes and zombie rows Wrap startProgressPolling in try/catch so a Room/SQLite failure cannot crash the app, and reconcile QUEUED/RUNNING DB rows whose WorkManager tasks were cancelled or dropped after an app update. Mark zombies FAILED with download_interrupted so the Download Manager shows a retry button. --- .../models/downloadZombieRecovery.test.ts | 76 +++++++++++++++++++ .../download/DownloadManagerModule.kt | 36 ++++++++- .../offgridmobile/download/DownloadPolling.kt | 14 ++++ .../download/DownloadManagerModuleTest.kt | 71 +++++++++++++++++ docs/PENDING_FIXES.md | 6 +- 5 files changed, 197 insertions(+), 6 deletions(-) create mode 100644 __tests__/integration/models/downloadZombieRecovery.test.ts create mode 100644 android/app/src/main/java/ai/offgridmobile/download/DownloadPolling.kt diff --git a/__tests__/integration/models/downloadZombieRecovery.test.ts b/__tests__/integration/models/downloadZombieRecovery.test.ts new file mode 100644 index 000000000..c9b2e1f35 --- /dev/null +++ b/__tests__/integration/models/downloadZombieRecovery.test.ts @@ -0,0 +1,76 @@ +const mockBackgroundDownloadService = { + isAvailable: jest.fn(), + getActiveDownloads: jest.fn(), +}; + +jest.mock('../../../src/services/backgroundDownloadService', () => ({ + backgroundDownloadService: mockBackgroundDownloadService, +})); + +const { hydrateDownloadStore } = require('../../../src/services/downloadHydration'); +const { useDownloadStore } = require('../../../src/stores/downloadStore'); +const { isRetryable } = require('../../../src/utils/downloadErrors'); + +describe('download zombie recovery integration', () => { + beforeEach(() => { + jest.clearAllMocks(); + useDownloadStore.setState({ + downloads: {}, + downloadIdIndex: {}, + repairingVisionIds: {}, + }); + mockBackgroundDownloadService.isAvailable.mockReturnValue(true); + }); + + it('hydrates interrupted downloads as failed with a retryable reason code', async () => { + mockBackgroundDownloadService.getActiveDownloads.mockResolvedValue([ + { + downloadId: 'dl-zombie-1', + modelId: 'qwen3-4b', + modelKey: 'qwen3-4b:Q4_K_M.gguf', + fileName: 'Q4_K_M.gguf', + modelType: 'text', + status: 'failed', + bytesDownloaded: 0, + totalBytes: 2_500_000_000, + reason: 'The download was interrupted.', + reasonCode: 'download_interrupted', + createdAt: 1_700_000_000_000, + }, + ]); + + await hydrateDownloadStore(); + + const entry = useDownloadStore.getState().downloads['qwen3-4b:Q4_K_M.gguf']; + expect(entry).toBeDefined(); + expect(entry.status).toBe('failed'); + expect(entry.errorCode).toBe('download_interrupted'); + expect(entry.errorMessage).toBe('The download was interrupted.'); + expect(isRetryable(entry.errorCode)).toBe(true); + }); + + it('does not drop failed rows so the Download Manager can show retry', async () => { + mockBackgroundDownloadService.getActiveDownloads.mockResolvedValue([ + { + downloadId: 'dl-zombie-2', + modelId: 'whisper-base', + modelKey: 'whisper-base:ggml-base.en.bin', + fileName: 'ggml-base.en.bin', + modelType: 'stt', + status: 'failed', + bytesDownloaded: 32, + totalBytes: 148_000_000, + reason: 'The download was interrupted.', + reasonCode: 'download_interrupted', + createdAt: 1_700_000_000_100, + }, + ]); + + await hydrateDownloadStore(); + + expect(Object.keys(useDownloadStore.getState().downloads)).toHaveLength(1); + expect(useDownloadStore.getState().downloadIdIndex['dl-zombie-2']).toBe( + 'whisper-base:ggml-base.en.bin', + ); + }); +}); diff --git a/android/app/src/main/java/ai/offgridmobile/download/DownloadManagerModule.kt b/android/app/src/main/java/ai/offgridmobile/download/DownloadManagerModule.kt index dc06ca102..3b436e7dc 100644 --- a/android/app/src/main/java/ai/offgridmobile/download/DownloadManagerModule.kt +++ b/android/app/src/main/java/ai/offgridmobile/download/DownloadManagerModule.kt @@ -312,12 +312,40 @@ class DownloadManagerModule(reactContext: ReactApplicationContext) : @ReactMethod fun startProgressPolling() { scope.launch { - val active = withContext(Dispatchers.IO) { - downloadDao.getAllDownloads().first().filter { - it.status == DownloadStatus.QUEUED || it.status == DownloadStatus.RUNNING + try { + val active = withContext(Dispatchers.IO) { + downloadDao.getAllDownloads().first().filter { + it.status == DownloadStatus.QUEUED || it.status == DownloadStatus.RUNNING + } + } + for (download in active) { + if (workObservers.containsKey(download.id)) continue + val workInfos = withContext(Dispatchers.IO) { + workManager.getWorkInfosForUniqueWork(WorkerDownload.workName(download.id)).get() + } + if (DownloadPolling.isZombieWorkStates(workInfos.map { it.state })) { + withContext(Dispatchers.IO) { + downloadDao.updateStatus( + download.id, + DownloadStatus.FAILED, + DownloadReason.DOWNLOAD_INTERRUPTED, + ) + } + DownloadEventBridge.error( + download.id, + download.fileName, + download.modelId, + DownloadReason.messageFor(DownloadReason.DOWNLOAD_INTERRUPTED) + ?: "Download was interrupted.", + DownloadReason.DOWNLOAD_INTERRUPTED, + ) + } else { + registerObserver(download.id, download.fileName, download.modelId) + } } + } catch (_: Exception) { + // DB unavailable — polling skipped, no crash } - active.forEach { if (!workObservers.containsKey(it.id)) registerObserver(it.id, it.fileName, it.modelId) } } } diff --git a/android/app/src/main/java/ai/offgridmobile/download/DownloadPolling.kt b/android/app/src/main/java/ai/offgridmobile/download/DownloadPolling.kt new file mode 100644 index 000000000..6716a562b --- /dev/null +++ b/android/app/src/main/java/ai/offgridmobile/download/DownloadPolling.kt @@ -0,0 +1,14 @@ +package ai.offgridmobile.download + +import androidx.work.WorkInfo + +/** + * Pure helpers for [DownloadManagerModule.startProgressPolling]. + * Extracted so zombie-detection rules are unit-testable without Room/WorkManager. + */ +internal object DownloadPolling { + fun isZombieWorkStates(states: List): Boolean = + states.isEmpty() || states.all { + it == WorkInfo.State.CANCELLED || it == WorkInfo.State.FAILED + } +} diff --git a/android/app/src/test/java/ai/offgridmobile/download/DownloadManagerModuleTest.kt b/android/app/src/test/java/ai/offgridmobile/download/DownloadManagerModuleTest.kt index b1051bedb..005784cce 100644 --- a/android/app/src/test/java/ai/offgridmobile/download/DownloadManagerModuleTest.kt +++ b/android/app/src/test/java/ai/offgridmobile/download/DownloadManagerModuleTest.kt @@ -1,6 +1,7 @@ package ai.offgridmobile.download import android.app.Application +import androidx.work.WorkInfo import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull @@ -303,5 +304,75 @@ class DownloadManagerModuleTest { as android.app.NotificationManager assertNotNull(mgr.getNotificationChannel("model_downloads")) } + + // ── DownloadPolling.isZombieWorkStates ──────────────────────────────────── + + @Test + fun isZombieWorkStatesTreatsEmptyListAsZombie() { + assertTrue(DownloadPolling.isZombieWorkStates(emptyList())) + } + + @Test + fun isZombieWorkStatesTreatsAllCancelledAsZombie() { + assertTrue( + DownloadPolling.isZombieWorkStates( + listOf(WorkInfo.State.CANCELLED, WorkInfo.State.CANCELLED), + ), + ) + } + + @Test + fun isZombieWorkStatesTreatsAllFailedAsZombie() { + assertTrue( + DownloadPolling.isZombieWorkStates( + listOf(WorkInfo.State.FAILED), + ), + ) + } + + @Test + fun isZombieWorkStatesTreatsMixedCancelledAndFailedAsZombie() { + assertTrue( + DownloadPolling.isZombieWorkStates( + listOf(WorkInfo.State.CANCELLED, WorkInfo.State.FAILED), + ), + ) + } + + @Test + fun isZombieWorkStatesRejectsRunningWork() { + assertFalse( + DownloadPolling.isZombieWorkStates( + listOf(WorkInfo.State.RUNNING), + ), + ) + } + + @Test + fun isZombieWorkStatesRejectsEnqueuedWork() { + assertFalse( + DownloadPolling.isZombieWorkStates( + listOf(WorkInfo.State.ENQUEUED), + ), + ) + } + + @Test + fun isZombieWorkStatesRejectsCancelledPlusRunning() { + assertFalse( + DownloadPolling.isZombieWorkStates( + listOf(WorkInfo.State.CANCELLED, WorkInfo.State.RUNNING), + ), + ) + } + + @Test + fun downloadInterruptedReasonIsRetryable() { + assertTrue(DownloadReason.isRetryable(DownloadReason.DOWNLOAD_INTERRUPTED)) + assertEquals( + "The download was interrupted.", + DownloadReason.messageFor(DownloadReason.DOWNLOAD_INTERRUPTED), + ) + } } diff --git a/docs/PENDING_FIXES.md b/docs/PENDING_FIXES.md index af824ddc1..f814f298d 100644 --- a/docs/PENDING_FIXES.md +++ b/docs/PENDING_FIXES.md @@ -4,7 +4,8 @@ Issues identified but not yet applied to code. Address before or alongside the n --- -## 1. SQLiteException not caught in `startProgressPolling` +## ~~1. SQLiteException not caught in `startProgressPolling`~~ ✅ Fixed in `fix/android-download-zombie-polling` + **File:** `android/app/src/main/java/ai/offgridmobile/download/DownloadManagerModule.kt` @@ -36,7 +37,8 @@ fun startProgressPolling() { --- -## 2. Zombie download entries after app update +## ~~2. Zombie download entries after app update~~ ✅ Fixed in `fix/android-download-zombie-polling` + **File:** `android/app/src/main/java/ai/offgridmobile/download/DownloadManagerModule.kt`