From 408d1c760a56c070b5901c43743696a2f233f7e2 Mon Sep 17 00:00:00 2001 From: Moe Hamade <69801237+moehamade@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:28:33 +0300 Subject: [PATCH] fix: name the retry backoff instead of calling it a network wait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex caught this and it is correct. WorkManager returns a retried request to ENQUEUED for the duration of its backoff whether or not the device is online, and the mapping sent every ENQUEUED record to AwaitingConnectivity. With exponential backoff from 15s over three attempts, a fully connected device claimed "Waiting for network…" in both the row and the notification for roughly 45 seconds. ENQUEUED covers two different waits and the state alone cannot separate them; a non-zero runAttemptCount means the work already ran, so it is the backoff. Adds a Retrying phase for that case, extracted as queuedPhase() so the distinction is testable without a WorkInfo. Co-Authored-By: Claude Opus 5 (1M context) --- .../com/bitchat/android/util/ApkDownloader.kt | 23 +++++++++++++++++++ .../android/util/WorkManagerApkDownloader.kt | 6 +++-- app/src/main/res/values/strings.xml | 1 + .../bitchat/android/util/DownloadPhaseTest.kt | 12 ++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/util/ApkDownloader.kt b/app/src/main/java/com/bitchat/android/util/ApkDownloader.kt index b3f342c1..8e16620f 100644 --- a/app/src/main/java/com/bitchat/android/util/ApkDownloader.kt +++ b/app/src/main/java/com/bitchat/android/util/ApkDownloader.kt @@ -54,6 +54,13 @@ interface ApkDownloader { */ enum class DownloadPhase { AwaitingConnectivity, + /** + * Waiting out the backoff before another attempt. Distinct from + * [AwaitingConnectivity] because WorkManager parks a retry in ENQUEUED + * whether or not the device is online, and claiming a network wait there + * would be false on a connected device. + */ + Retrying, SelectingSource, AwaitingNetworkRoute, Transferring, @@ -74,10 +81,26 @@ interface ApkDownloader { } } +/** + * What a queued work record is actually waiting for. + * + * WorkManager parks both cases in ENQUEUED, so the state alone cannot tell them apart. A non-zero + * [runAttemptCount] means the work already ran and failed, which makes this the retry backoff + * rather than an unmet network constraint. + */ +internal fun queuedPhase(runAttemptCount: Int): ApkDownloader.DownloadPhase = + if (runAttemptCount > 0) { + ApkDownloader.DownloadPhase.Retrying + } else { + ApkDownloader.DownloadPhase.AwaitingConnectivity + } + /** Shared by the notification and the About sheet so both name a phase identically. */ internal fun downloadPhaseLabel(phase: ApkDownloader.DownloadPhase): Int = when (phase) { ApkDownloader.DownloadPhase.AwaitingConnectivity -> com.bitchat.android.R.string.prepare_apk_phase_awaiting_connectivity + ApkDownloader.DownloadPhase.Retrying -> + com.bitchat.android.R.string.prepare_apk_phase_retrying ApkDownloader.DownloadPhase.SelectingSource -> com.bitchat.android.R.string.prepare_apk_phase_selecting_source ApkDownloader.DownloadPhase.AwaitingNetworkRoute -> diff --git a/app/src/main/java/com/bitchat/android/util/WorkManagerApkDownloader.kt b/app/src/main/java/com/bitchat/android/util/WorkManagerApkDownloader.kt index 214b8118..6d482c34 100644 --- a/app/src/main/java/com/bitchat/android/util/WorkManagerApkDownloader.kt +++ b/app/src/main/java/com/bitchat/android/util/WorkManagerApkDownloader.kt @@ -58,11 +58,13 @@ class WorkManagerApkDownloader(context: Context) : ApkDownloader { return when (workInfo.state) { WorkInfo.State.ENQUEUED, WorkInfo.State.BLOCKED -> { - // Waiting for constraints (network). Show existing partial progress if any. + // ENQUEUED covers two different waits. A non-zero attempt count means the work + // already ran and failed, so this is the retry backoff rather than a missing + // network — saying "waiting for network" there would be false while online. val partial = apkManager.getPartialDownloadProgress() ApkDownloader.DownloadState.Downloading( partial ?: 0, - ApkDownloader.DownloadPhase.AwaitingConnectivity + queuedPhase(workInfo.runAttemptCount) ) } WorkInfo.State.RUNNING -> { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f5fde380..074ce540 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -252,6 +252,7 @@ Downloading… %1$d%% Waiting for network… + Retrying… Selecting download source… Waiting for Tor… Downloading… diff --git a/app/src/test/kotlin/com/bitchat/android/util/DownloadPhaseTest.kt b/app/src/test/kotlin/com/bitchat/android/util/DownloadPhaseTest.kt index f08c009c..7e5c7d6e 100644 --- a/app/src/test/kotlin/com/bitchat/android/util/DownloadPhaseTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/util/DownloadPhaseTest.kt @@ -43,6 +43,18 @@ class DownloadPhaseTest { ) } + @Test + fun `a queued retry is not reported as a connectivity wait`() { + // WorkManager returns a retry to ENQUEUED for the backoff even while the device is online, + // so attempt count is the only thing separating the two waits. + assertEquals( + ApkDownloader.DownloadPhase.AwaitingConnectivity, + queuedPhase(runAttemptCount = 0) + ) + assertEquals(ApkDownloader.DownloadPhase.Retrying, queuedPhase(runAttemptCount = 1)) + assertEquals(ApkDownloader.DownloadPhase.Retrying, queuedPhase(runAttemptCount = 2)) + } + @Test fun `only the transfer claims measurable progress`() { assertTrue(ApkDownloader.DownloadPhase.Transferring.hasMeasurableProgress)