diff options
3 files changed, 123 insertions, 2 deletions
diff --git a/app/src/main/java/org/terst/listen/ui/ShowRefreshError.kt b/app/src/main/java/org/terst/listen/ui/ShowRefreshError.kt new file mode 100644 index 0000000..31754d5 --- /dev/null +++ b/app/src/main/java/org/terst/listen/ui/ShowRefreshError.kt @@ -0,0 +1,39 @@ +package org.terst.listen.ui + +import java.util.concurrent.TimeUnit + +/** + * Whether a show's last refresh failure is worth showing, and what it says. + * + * The error itself is worth storing: a feed that has quietly stopped working looks exactly like + * one with nothing new. Showing every failure the moment it happens is a different matter. + * Peter opened the app to six shows in red, all of them reachable when checked a minute later -- + * his connection had dropped during a background refresh hours before, and the line survives + * until that feed next succeeds. Nothing refreshes on launch, so he was reading a stale report + * of a moment that was already over. + * + * One miss is noise; a pattern is information. The difference is already recorded, in the last + * time the feed actually worked. + */ +object ShowRefreshError { + + /** + * How long a feed may go without a successful refresh before its failure is worth saying. + * + * A day, against a six-hourly refresh: about four missed attempts. Long enough that a bad + * commute does not paint the library red, short enough that something genuinely broken is + * reported the same day. + */ + private val GRACE_MS = TimeUnit.DAYS.toMillis(1) + + /** The message to show under a show, or null to stay quiet. */ + fun visible(lastError: String?, lastFetchedAtMs: Long?, nowMs: Long): String? { + val error = lastError ?: return null + // Never succeeded: not a blip. A feed that failed when it was subscribed has given no + // reason to expect better, and somebody is waiting on it right now. + val lastGood = lastFetchedAtMs ?: return error + // A success in the future means the clock moved, not that the feed is healthy. + if (lastGood > nowMs) return error + return error.takeIf { nowMs - lastGood > GRACE_MS } + } +} diff --git a/app/src/main/java/org/terst/listen/ui/SubscriptionRow.kt b/app/src/main/java/org/terst/listen/ui/SubscriptionRow.kt index 49ea000..316b72f 100644 --- a/app/src/main/java/org/terst/listen/ui/SubscriptionRow.kt +++ b/app/src/main/java/org/terst/listen/ui/SubscriptionRow.kt @@ -72,8 +72,15 @@ internal fun SubscriptionRow( if (latest != null && actions != null) LatestEpisodeLine(latest, actions) // A feed that has quietly stopped working is otherwise indistinguishable from one - // with nothing new, which is the whole reason the error is stored rather than logged. - subscription.lastError?.let { + // with nothing new, which is the whole reason the error is stored rather than logged + // -- but only once it has gone a day without working. See ShowRefreshError: a + // dropped connection during one background refresh used to paint the library red + // until each feed happened to succeed again, hours later. + ShowRefreshError.visible( + subscription.lastError, + subscription.lastFetchedAtMs, + System.currentTimeMillis(), + )?.let { Text( "Last refresh failed: $it", style = MaterialTheme.typography.bodySmall, diff --git a/app/src/test/java/org/terst/listen/ui/ShowRefreshErrorTest.kt b/app/src/test/java/org/terst/listen/ui/ShowRefreshErrorTest.kt new file mode 100644 index 0000000..ed9dece --- /dev/null +++ b/app/src/test/java/org/terst/listen/ui/ShowRefreshErrorTest.kt @@ -0,0 +1,75 @@ +package org.terst.listen.ui + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import java.util.concurrent.TimeUnit + +/** + * When a feed's refresh failure is worth putting on the screen. + * + * Peter opened the app to six shows in red, all saying "Could not reach that feed". All six feeds + * answered 200 when checked a minute later: his phone's connection had dropped during a + * background refresh hours earlier, and the line persists until that feed next succeeds. Nothing + * refreshes on launch, so what he was looking at was a stale report of a moment long past. + * + * The line is worth keeping -- a feed that has quietly stopped working is otherwise + * indistinguishable from one with nothing new, which is why the error is stored at all. But one + * miss is noise and a pattern is information, and the difference is already recorded: + * `lastFetchedAtMs` is the last time this feed actually worked. + * + * So a failure is shown once the feed has not succeeded for a day. The periodic refresh runs + * every six hours, so that is about four missed attempts -- a blip clears itself, and something + * genuinely broken still goes red and stays red. + */ +class ShowRefreshErrorTest { + + private val now = 1_800_000_000_000L + private val hour = TimeUnit.HOURS.toMillis(1) + + @Test + fun `no error means nothing to say`() { + assertNull(ShowRefreshError.visible(null, lastFetchedAtMs = now - 100 * hour, nowMs = now)) + } + + /** The case that put six red lines on the screen: one bad moment, fine an hour ago. */ + @Test + fun `a failure since a recent success is not shown`() { + assertNull(ShowRefreshError.visible("Could not reach that feed.", now - 3 * hour, now)) + } + + @Test + fun `a feed that has not worked for a day is shown`() { + assertEquals( + "Could not reach that feed.", + ShowRefreshError.visible("Could not reach that feed.", now - 25 * hour, now), + ) + } + + /** + * Never fetched at all is not a blip. A feed that failed the moment it was subscribed has + * given no reason to think it will work, and the listener is right there waiting to hear. + */ + @Test + fun `a feed that has never succeeded is shown at once`() { + assertEquals( + "That address is not a podcast feed.", + ShowRefreshError.visible("That address is not a podcast feed.", lastFetchedAtMs = null, nowMs = now), + ) + } + + /** A clock that has gone backwards must not hide a real failure for ever. */ + @Test + fun `a success timestamp in the future is treated as no success`() { + assertEquals( + "Could not reach that feed.", + ShowRefreshError.visible("Could not reach that feed.", now + 100 * hour, now), + ) + } + + @Test + fun `the boundary is a day`() { + assertNull(ShowRefreshError.visible("x", now - 23 * hour, now)) + assertEquals("x", ShowRefreshError.visible("x", now - 24 * hour - 1, now)) + } +} |