iris/dev-updater
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't report a failure nobody was there to see

Browse Source 
Leaving the app for a while and coming back to it greeted you with a
five-second read timeout. Work started before the screen went away keeps
running -- a build being followed, a download, the manifest poll -- and when
the device sleeps or the link drops, that work fails and reported itself.
The message said nothing about the server and there was nothing left to do
about it, because the thing it described was over before it was read.

Every catch now goes through one `failure(e)`, which answers null while the
screen is not resumed. Null means *clear*, never *leave*, at every site: a
dropped failure that left the card alone would leave a spinner up for an
operation that has already stopped. `setProject` joins `setComponent` so
both levels are written the same way and a caller can hand the answer over
whichever it is.

That is only safe because the resume re-read can now run from any state.
It was guarded on the list being loaded, which meant a failure ruled out
the one thing that would have replaced it -- a card that had gone red
stayed red until somebody found the Refresh button. Arriving and resuming
are also one event now rather than a LaunchedEffect beside a resume effect,
with `loadingList` claimed before the coroutine launches so the two cannot
stack two reads in one frame.

The other half of the same complaint is the read on the way back in: the
link may have been asleep as long as the app was, and the first request
across one still coming back times out at the five seconds every request
gets. The resume's read alone retries once. It costs nothing when the
server really is down, because a refused connection comes back at once
rather than waiting out a timeout -- and it is a retry, not a guess.

Checked all four ways on the emulator, including the two the change was not
written for. Killing the server while the app was backgrounded mid-poll and
returning: the list loads, no timeout. Pressing Refresh with the server
down: the failure is shown at once, in full. Returning while it is still
down: retried, then reported, no stuck spinner. Returning once it is back:
the list recovers, which the old guard would have prevented.
This commit is contained in:
iris committed 2026-09-01 03:39:00 -04:00
1 parent a7f7f4550e
commit 3685ab107d
2 files changed
+168 -46
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt
+133 -46
View File
@@ -140,6 +140,14 @@ private const val CHECK_POLL_INTERVAL_MS = 700L
// second; these cover that with room to spare without making a genuine
// outage take long to report.
private const val REFRESH_ATTEMPTS_AFTER_PULL = 4
/**
* How long to wait before the one retry a resume gets.
*
* Short, because this is not waiting for a server to come up -- it is giving a link that woke with
* the screen a moment to finish doing so.
*/
private const val WAKE_RETRY_MS = 600L
private const val RESTART_WAIT_MS = 1000L
private sealed class ManifestState {
@@ -489,6 +497,42 @@ private fun AppListScreen(
// that is busy is the one that shows it.
var serviceBusy by remember { mutableStateOf<Map<String, String>>(emptyMap()) }
// Whether this screen is actually in front of somebody. Set by the
// resume effect further down, and read only by `failure` below.
var foreground by remember { mutableStateOf(true) }
/**
* What to show for a failed request, or null when there is nobody to show it to.
*
* Work started before the app went away keeps running — a poll loop, a download — and when the
* device sleeps or the link drops it fails. Reporting that means coming back an hour later is
* greeted by a five-second read timeout that says nothing about the server and that there is
* nothing left to do about, because the thing it describes is over. So a failure that lands
* while nothing is on screen is dropped, and the re-read on the way back in is what produces
* the truth instead.
*
* Null means *clear*, never *leave*, at every call site: a dropped failure that left the card
* as it was would leave a spinner up for an operation that has already stopped.
*
* A failure of something somebody pressed is never dropped, because pressing it is what put
* them in front of the screen.
*/
fun failure(e: DownloadServerException): String? = (e.message ?: "Failed").takeIf { foreground }
/**
* Puts one project's state, or takes it away for null.
*
* The pair of [setComponent], so the two levels are written the same way and a caller with a
* message that may or may not be worth showing can hand the answer straight over.
*/
fun setProject(key: String, state: ProjectState?) {
projectStates =
when (state) {
null -> projectStates - key
else -> projectStates + (key to state)
}
}
/**
* Puts one component's state, or takes it away for null.
*
@@ -549,34 +593,70 @@ private fun AppListScreen(
// of the gesture is that the list stays where it is.
var pulling by remember { mutableStateOf(false) }
// Whether a whole-list read is already running, so the resume below
// cannot start a second one beside the one that is already going.
var loadingList by remember { mutableStateOf(false) }
/**
* Reads the whole list into [manifestState], however it was asked for.
*
* A failure that lands while nothing is on screen leaves the list on the spinner rather than
* replacing it with a timeout nobody is there to read — see [failure]. That is only safe
* because the resume effect re-reads from *any* state, spinner included, so the way back into
* the app is what resolves it.
*/
// Clears the flag rather than setting it: the callers set it before
// they launch, because two of them can run in the same frame and both
// would pass the guard if the first one only claimed it once its
// coroutine got going.
//
// [afterAGap] is the resume's, and nothing else passes it. The link may
// have been asleep as long as the app was, and the first request across
// one that is still coming back times out at the five seconds every
// request here gets -- which is the server being reported unreachable
// for no reason except that somebody had the app closed. One more try
// is the difference between saying that and giving it a second. It
// costs nothing when the server really is down, because a refused
// connection comes back at once rather than waiting out the timeout.
suspend fun loadInto(afterAGap: Boolean = false) {
val attempts = if (afterAGap) 2 else 1
try {
repeat(attempts) { attempt ->
try {
manifestState = load()
return
} catch (e: DownloadServerException) {
if (attempt == attempts - 1) {
failure(e)?.let { manifestState = ManifestState.Error(it) }
return
}
delay(WAKE_RETRY_MS)
}
}
} finally {
loadingList = false
}
}
fun refreshByPull() {
if (pulling) return
if (pulling || loadingList) return
pulling = true
loadingList = true
projectStates = emptyMap()
componentStates = emptyMap()
scope.launch {
manifestState =
try {
load()
} catch (e: DownloadServerException) {
ManifestState.Error(e.message ?: "Unknown error")
}
loadInto()
pulling = false
}
}
fun refresh() {
fun refresh(afterAGap: Boolean = false) {
if (loadingList) return
loadingList = true
manifestState = ManifestState.Loading
projectStates = emptyMap()
componentStates = emptyMap()
scope.launch {
manifestState =
try {
load()
} catch (e: DownloadServerException) {
ManifestState.Error(e.message ?: "Unknown error")
}
}
scope.launch { loadInto(afterAGap) }
}
/**
@@ -664,8 +744,7 @@ private fun AppListScreen(
try {
awaitCheck(entry.key)
} catch (e: DownloadServerException) {
projectStates =
projectStates + (entry.key to ProjectState.Error(e.message ?: "Failed"))
setProject(entry.key, failure(e)?.let(ProjectState::Error))
}
}
}
@@ -692,11 +771,11 @@ private fun AppListScreen(
start: suspend () -> BuildStatus,
progress: (BuildStatus?) -> ProjectState,
) {
projectStates = projectStates + (entry.key to progress(null))
setProject(entry.key, progress(null))
try {
var status = withContext(Dispatchers.IO) { start() }
while (status.building) {
projectStates = projectStates + (entry.key to progress(status))
setProject(entry.key, progress(status))
delay(BUILD_POLL_INTERVAL_MS)
status = withContext(Dispatchers.IO) { buildStatus(entry.key) }
}
@@ -706,9 +785,9 @@ private fun AppListScreen(
// fell over belongs to the component whose command it was,
// since a run builds every component at once and two of them
// can fail differently.
val failure = status.error
if (failure != null) {
projectStates = projectStates + (entry.key to ProjectState.Error(failure))
val pullFailure = status.error
if (pullFailure != null) {
setProject(entry.key, ProjectState.Error(pullFailure))
// The card keeps the message either way -- the dialog is
// dismissible, and a failure that vanished with it would
// leave the card looking as though nothing had happened.
@@ -745,7 +824,7 @@ private fun AppListScreen(
// failed inside this run has just been given its own,
// and clearing that here would take away the only
// thing saying so.
projectStates = projectStates - entry.key
setProject(entry.key, null)
// Building this app's own project is what produces the
// newer copy of it, and restarts the server it has to
// keep talking to. So this is the moment to offer it,
@@ -756,15 +835,15 @@ private fun AppListScreen(
return
} catch (e: DownloadServerException) {
if (attempt == REFRESH_ATTEMPTS_AFTER_PULL - 1) {
projectStates = projectStates - entry.key
manifestState = ManifestState.Error(e.message ?: "Unknown error")
setProject(entry.key, null)
failure(e)?.let { manifestState = ManifestState.Error(it) }
} else {
delay(RESTART_WAIT_MS)
}
}
}
} catch (e: DownloadServerException) {
projectStates = projectStates + (entry.key to ProjectState.Error(e.message ?: "Failed"))
setProject(entry.key, failure(e)?.let(ProjectState::Error))
}
}
@@ -835,11 +914,7 @@ private fun AppListScreen(
return@launch
}
} catch (e: DownloadServerException) {
setComponent(
entry.key,
component,
ComponentState.Error(e.message ?: "Couldn't prepare phone build"),
)
setComponent(entry.key, component, failure(e)?.let(ComponentState::Error))
return@launch
}
}
@@ -860,11 +935,7 @@ private fun AppListScreen(
}
}
} catch (e: DownloadServerException) {
setComponent(
entry.key,
component,
ComponentState.Error(e.message ?: "Download failed"),
)
setComponent(entry.key, component, failure(e)?.let(ComponentState::Error))
return@launch
}
setComponent(entry.key, component, null)
@@ -881,11 +952,10 @@ private fun AppListScreen(
withContext(Dispatchers.IO) { action() }
// This card's state only: another card's error is its own
// and has nothing to do with what just happened here.
projectStates = projectStates - entry.key
setProject(entry.key, null)
if (removes) dropEntry(entry.key) else applyOne(entry.key)
} catch (e: DownloadServerException) {
projectStates =
projectStates + (entry.key to ProjectState.Error(e.message ?: "Failed"))
setProject(entry.key, failure(e)?.let(ProjectState::Error))
}
}
}
@@ -926,7 +996,7 @@ private fun AppListScreen(
)
applyOne(entry.key)
} catch (e: DownloadServerException) {
setComponent(entry.key, component, ComponentState.Error(e.message ?: "Failed"))
setComponent(entry.key, component, failure(e)?.let(ComponentState::Error))
} finally {
serviceBusy = serviceBusy - entry.key
}
@@ -961,7 +1031,10 @@ private fun AppListScreen(
}
}
LaunchedEffect(Unit) { refresh() }
// The first load is the resume effect's too, rather than a
// LaunchedEffect(Unit) beside it: `LifecycleResumeEffect` runs when the
// screen first reaches RESUMED, so arriving and returning are the same
// event and there is one thing that decides when the list is read.
// An app added on the Add screen is fetched on its own and appended.
// Reloading the list instead would put every other card back through
@@ -1016,11 +1089,25 @@ private fun AppListScreen(
// true now rather than whenever it was last opened. A checkout already
// being asked about is not asked again (RemoteChecks::refresh), so a
// burst of resumes doesn't pile up round trips.
//
// From *any* state, not only a loaded one, and that is load-bearing
// twice over. It is what makes dropping a background failure safe:
// that leaves the list on its spinner, and this is the only thing that
// will take it off. And a state that failed is the one most worth
// retrying on the way back in -- guarded on "loaded", a card that had
// gone red stayed red until somebody found the Refresh button, which
// is the opposite of what returning to an app should do.
//
// `refresh` declines when a read is already running, so this and the
// first-composition run below cannot stack two.
LifecycleResumeEffect(Unit) {
if (manifestState is ManifestState.Loaded) {
refresh()
}
onPauseOrDispose {}
foreground = true
refresh(afterAGap = true)
// Not "the app is gone" -- the coroutines started above keep
// running, deliberately, because a download that finishes while
// somebody is in another app is a download that worked. This only
// says there is nobody to report a failure to.
onPauseOrDispose { foreground = false }
}
// PackageManager reports a completed install as soon as it happens,