fix: show readable error messages when feed subscription fails

[mlkgrnt]

PR: fix: show readable error messages when feed subscription fails

分支 fix/readable-subscribe-errors · 单 commit 5ced2138 · 基于 upstream main (d2b979cc)
8 files changed, 276 insertions(+), 5 deletions(-)
作者:mlkgrnt(提交前已把占位身份换成 GitHub 账号)

---

Problem

When a feed cannot be imported in the subscribe dialog, ReadYou surfaces the raw exception message to the user. Real-world failures therefore look like:

• SSLHandshakeException: connection closed — e.g. status.deepseek.com from a mainland-China network whose exit route cannot complete the TLS handshake
• Unable to detect RSS feed URL — no hint of the HTTP status behind the failure
• Failed to connect to xxx.com/... — indistinguishable network/HTTP/parse causes

These messages are meaningless to end users and give no clue whether the problem is the network, the server, or the URL itself. This matches long-standing user complaints about "cannot import feed with no clear reason" (e.g. #394 and similar).

Root cause of the investigated case (status.deepseek.com/feed.rss)

Investigated end-to-end on an emulator with the real app:

1. status.deepseek.com is hosted on Atlassian Statuspage; its /feed.rss always 302-redirects to /history.atom (a standard Atom 1.0 document, application/atom+xml; charset=utf-8). Reproduced with githubstatus.com as a control.
2. ReadYou's pipeline — SubscribeViewModel.searchFeed() → RssHelper.searchFeed() → OkHttp (followRedirects = true) → ROME SyndFeedInput — handles this shape correctly: verified by driving the real searchFeed() against a local mock statuspage server (redirect → Atom) in instrumentation tests, all green.
3. The real failure for this URL is SSLHandshakeException: connection closed, reproduced on-device. Follow-up on a physical device confirmed this is network-reachability dependent: the domain (hosted overseas on Atlassian Statuspage) is reachable when the exit route is "clean" but the TLS handshake is terminated on some mainland-China exit paths (direct connection through certain ISPs, or through shared/flagged proxy nodes). The feed itself is healthy — it imports fine once a working route is used.

Conclusion: ReadYou has no parsing/redirect bug for statuspage-style feeds — the pipeline handles the RSS→Atom redirect shape end to end. The user-visible defect is that reachability failures of this kind are reported as a raw, unreadable exception string (SSLHandshakeException: connection closed) with no actionable hint, which is what this PR fixes.

Fix

Classify subscription failures and present a short, localized message instead of the raw exception.

RssHelper.kt

• New semantic exception types replacing bare IOException:
  • FeedHttpException(message, statusCode) — the server answered with a non-success HTTP code (both for the direct feed response and the HTML-discovery response).
  • FeedNotFoundException — the URL is not a feed and no feed could be discovered on the page.

SubscribeErrors.kt (new)

• Pure function Throwable.toSubscribeError(): SubscribeError:
  • FeedHttpException → localized "server rejected this URL (HTTP %1$d)"
  • FeedNotFoundException → localized "no RSS/Atom feed found at this URL"
  • SSLException / UnknownHostException / ConnectException / SocketTimeoutException → localized "couldn't connect to the server"
  • SAXParseException / ROME FeedException → localized "content could not be parsed"
  • anything else → raw message, falling back to a generic hint when blank

SubscribeViewModel.kt

• .onFailure now maps the throwable through toSubscribeError() and resolves the string resource (with format args) via the existing androidStringsHelper, instead of copying it.message.

Resources

• values/strings.xml + values-zh-rCN/strings.xml: 5 new strings (subscribe_error_network/http/feed_not_found/parse/fallback).

Tests

• SubscribeErrorsTest (JVM): covers every branch of toSubscribeError().
• StatuspageAtomParsingTest (JVM): proves ROME parses an Atom document in the exact shape statuspage hosts serve (RSS URL → Atom redirect, application/atom+xml; charset=utf-8); fixture uses a fictional domain.
• Verified locally (not part of this PR, as they need a local mock HTTP server): instrumentation E2E driving the real RssHelper.searchFeed() through a 302→Atom mock, an HTML-discovery page, and a direct Atom URL — all green on the emulator.
• CI-equivalent: ./gradlew :app:testGithubReleaseUnitTest → BUILD SUCCESSFUL.

Notes for maintainers

• Behavior change: the subscribe dialog now shows category messages instead of raw exception text. Debugging detail is still available in logcat; if you prefer keeping the raw message visible for power users, the SubscribeError.Raw fallback already preserves it for unexpected error types.
• New strings are English (default) + Simplified Chinese. Other locales are managed via Weblate and will pick the keys up automatically.
• The commit author identity is a placeholder; update to your own before pushing if you take over this branch.

[ibrahim-iqbal]

Nice structure — the sealed SubscribeError with a Text / Raw split makes the fallback path explicit without swallowing the original message, and the Throwable.toSubscribeError() extension keeps the ViewModel readable.

A few things I noticed while reading through:

toSubscribeError() mapping

• The SSLException branch catches SSLHandshakeException, SSLPeerUnverifiedException, and SSLProtocolException (all subclasses), which is good for the ICP/geo-blocking case called out in the description.
• ConnectException + SocketTimeoutException cover the common connectivity failures. NoRouteToHostException (a different SocketException subclass) will not match and will fall through to SubscribeError.Raw. Not necessarily a bug — the raw message from that one is usually already legible — but worth a note in case it's meant to be part of the "network" bucket.

FeedHttpException / FeedNotFoundException

• Both extend IOException, so any caller further up that was already catching IOException keeps working. Good.
• FeedHttpException carries the status code, which makes it easy to specialize the string later (e.g. subscribe_error_http_not_found for 404s), if you want that as a follow-up.

Strings

• subscribe_error_fallback is only referenced from SubscribeError.Raw when message is blank — the resource is present in both values/strings.xml and values-zh-rCN/strings.xml, so the fallback path renders in both locales.

The test additions (both the parser fixture and the mapper table) exercise the meaningful branches. LGTM overall.

[mlkgrnt]

Thanks for the careful read, @ibrahim-iqbal — and you were right about NoRouteToHostException. It is a SocketException subclass that would have fallen through to SubscribeError.Raw, and it does show up in the geo-blocking/ICP scenario the PR description mentions (some hosts answer with "no route to host" rather than a refused connection). Commit 8174b24a now maps it to the network bucket and adds it to the unreachable-host test table so it renders the localized network message.

On the FeedHttpException status-code specialization (e.g. a dedicated 404 string): agreed it is a good follow-up, but I have kept this PR scoped to readable fallback messages. Happy to pick that up in a separate PR if the maintainers want it.

[ibrahim-iqbal]

Nice — 8174b24a slots NoRouteToHostException in cleanly next to the other unreachable-host cases, and the test covers it. Agreed on keeping status-code specialisation out of scope; happy to see it land as a follow-up whenever.

[ibrahim-iqbal]

Confirmed. NoRouteToHostException in the network bucket in SubscribeErrors.kt plus the new row in unreachable host maps to network message covers the geo-blocking / ICP scenario from the PR description — that was the last edge case I was worried about.

Agreed on keeping the per-status-code specialization out of this PR. FeedHttpException already carries the code and subscribe_error_http (HTTP %1$d) surfaces it, which is enough for a readable-fallback first cut. Happy to see a dedicated 404 (and 401/403) string land in a follow-up.

LGTM.