mirror of
https://github.com/permissionlesstech/bitchat-android.git
synced 2026-08-22 07:06:05 +00:00
fix: name the retry backoff instead of calling it a network wait
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) <noreply@anthropic.com>
This commit is contained in:
parent
700e9aa0e5
commit
408d1c760a
@ -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 ->
|
||||
|
||||
@ -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 -> {
|
||||
|
||||
@ -252,6 +252,7 @@
|
||||
<string name="prepare_apk_status_downloading">Downloading… %1$d%%</string>
|
||||
<!-- Stages of preparing the APK. Only the transfer has a meaningful percentage. -->
|
||||
<string name="prepare_apk_phase_awaiting_connectivity">Waiting for network…</string>
|
||||
<string name="prepare_apk_phase_retrying">Retrying…</string>
|
||||
<string name="prepare_apk_phase_selecting_source">Selecting download source…</string>
|
||||
<string name="prepare_apk_phase_awaiting_route">Waiting for Tor…</string>
|
||||
<string name="prepare_apk_phase_transferring">Downloading…</string>
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user