diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-09-20 17:39:04 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-09-20 17:39:04 +0000 |
| commit | cf8ff865f7764953ded1b93c8ec42d4aff589e81 (patch) | |
| tree | 186ef308bfd2cfa46504052550cb8ace7a44d2ae /app/src/main/java/org/terst/feedreader/ui | |
| parent | f1d03f7bd675050a088e7baf3de648088e7f7e4b (diff) | |
Fix intermittent extraction failures, and stop them being silent
Bug report: extraction failed on one 1500days.com article while others worked.
It is not that article. Extraction was racing the page load, and the report
could say nothing more because the app discarded the reason.
Two separate defects.
1. The race. onPageFinished fires before the page has finished loading —
observed at progress=25 on this article — and the extraction script was
injected there. Sometimes the DOM is far enough along; sometimes
evaluateJavascript hands back a bare "null" because the document is still
being replaced. Measured against the reported URL: 4 successes in 6
attempts before, 10 in 10 after.
An unusable result is no longer final. A null or empty-content result is
retried (6 attempts, 500ms apart, well inside the existing 25s timeout); a
Readability error still fails immediately, since retrying will not change
what the parser thinks of the markup.
2. The silence. ReaderScreen did `extracted?.content?.let { }`, so every
failure — dead host, HTTP error, timeout, parser throw — arrived as the same
null and was dropped. The reader just kept showing raw feed content. The
only way a failure ever surfaced was the user noticing.
extractResult() now returns a typed ExtractionFailure carrying the reason;
ReaderScreen logs the diagnostic and shows the user a short explanation.
Also replaced a dead error callback. The only override was the four-argument
onReceivedError, which Android stopped calling at API 23; minSdk is 26, so it
never fired on any supported device. A failed load therefore invoked nothing
and hung for the full 25 seconds before surfacing as an indistinguishable null.
Now overrides the request/error and onReceivedHttpError forms Android actually
calls, both filtered to the main frame so a 404'd tracker cannot abort an
article that renders fine.
The instrumented test does NOT reproduce the race and says so in its own
docstring: a MockWebServer page finishes loading before onPageFinished fires,
and slow subresources do not help because WebView waits for them. It is kept as
a reliability guard over repeated back-to-back extractions. The race was
verified by hand against the live URL.
47 unit tests, 1 instrumented test, both green.
Diffstat (limited to 'app/src/main/java/org/terst/feedreader/ui')
| -rw-r--r-- | app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt | 56 |
1 files changed, 45 insertions, 11 deletions
diff --git a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt index 165d421..c701a26 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt @@ -5,6 +5,7 @@ import android.content.ClipboardManager import android.content.Context import android.content.Intent import android.net.Uri +import android.util.Log import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -56,6 +57,9 @@ import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch +import org.terst.feedreader.readability.ExtractionResult +import org.terst.feedreader.readability.diagnostic +import org.terst.feedreader.readability.userMessage import org.terst.feedreader.data.AppDatabase import org.terst.feedreader.data.ArticleEntity import org.terst.feedreader.data.DEFAULT_READER_FONT_SIZE_PX @@ -194,6 +198,10 @@ private fun ReaderContent( var readerContent by remember(articleId) { mutableStateOf<String?>(null) } var isReaderMode by remember(articleId) { mutableStateOf(false) } var isExtracting by remember(articleId) { mutableStateOf(false) } + // Non-null when the last extraction attempt failed. Keyed on articleId like + // the rest of the per-article state so a stale error cannot carry over to + // the next article. + var extractionError by remember(articleId) { mutableStateOf<String?>(null) } var isSaving by remember(articleId) { mutableStateOf(false) } var moreMenuExpanded by remember(articleId) { mutableStateOf(false) } var showBugReportDialog by remember(articleId) { mutableStateOf(false) } @@ -222,11 +230,24 @@ private fun ReaderContent( isReaderMode = true if (readerContent == null) { isExtracting = true - val extracted = ReaderExtractor.extract(context, loaded.link) + val result = ReaderExtractor.extractResult(context, loaded.link) isExtracting = false - extracted?.content?.let { - readerContent = it - db.articleDao().setExtractedContent(articleId, it) + when (result) { + is ExtractionResult.Success -> { + val content = result.article.content + if (content != null) { + readerContent = content + db.articleDao().setExtractedContent(articleId, content) + } + } + is ExtractionResult.Failure -> { + // Previously this branch did not exist: a failure was + // dropped, leaving the raw feed content on screen with + // no indication anything had gone wrong. + Log.w("ReaderExtractor", "extract failed for ${loaded.link}: ${result.reason.diagnostic()}") + extractionError = result.reason.userMessage() + snackbarHostState.showSnackbar(result.reason.userMessage()) + } } } } @@ -245,12 +266,20 @@ private fun ReaderContent( if (newEnabled) { if (readerContent == null) { isExtracting = true - val extracted = ReaderExtractor.extract(context, a.link) + val result = ReaderExtractor.extractResult(context, a.link) isExtracting = false - val content = extracted?.content - if (content != null) { - readerContent = content - db.articleDao().setExtractedContent(articleId, content) + when (result) { + is ExtractionResult.Success -> { + val content = result.article.content + if (content != null) { + readerContent = content + db.articleDao().setExtractedContent(articleId, content) + } + } + is ExtractionResult.Failure -> { + Log.w("ReaderExtractor", "extract failed for ${a.link}: ${result.reason.diagnostic()}") + extractionError = result.reason.userMessage() + } } } isReaderMode = true @@ -397,7 +426,12 @@ private fun ReaderContent( val a = currentArticle ?: return@DropdownMenuItem scope.launch { isExtracting = true - val extracted = ReaderExtractor.extract(context, a.link) + val reResult = ReaderExtractor.extractResult(context, a.link) + val extracted = (reResult as? ExtractionResult.Success)?.article + if (reResult is ExtractionResult.Failure) { + Log.w("ReaderExtractor", "re-extract failed for ${a.link}: ${reResult.reason.diagnostic()}") + extractionError = reResult.reason.userMessage() + } isExtracting = false val content = extracted?.content if (content != null) { @@ -406,7 +440,7 @@ private fun ReaderContent( db.articleDao().setExtractedContent(a.id, content) snackbarHostState.showSnackbar("Re-extracted") } else { - snackbarHostState.showSnackbar("Re-extraction failed") + snackbarHostState.showSnackbar(extractionError ?: "Re-extraction failed") } } } |
