Reject aggregator error placeholders and bound the details meta fetch - #1
Merged
Merged
Conversation
Aggregator addons answer a failed upstream lookup with a well-formed meta object rather than an error: the name field carries the failing sub-addon tagged with a cross mark, and the description carries the error text. It parses cleanly, so the parser accepted it as real metadata. The value then travelled well past the request that produced it. On a user's install a series ended up stored as "[X] Anime Kitsu" with "Request for meta for Anime Kitsu timed out after 30000ms" as its synopsis, in both watch progress and the Continue Watching enrichment cache, and surfaced later as the title in the player overlay -- on a different device from the one where the lookup had failed. Presence checks cannot catch this: the placeholder has an id, a type and a name, so it satisfies looksLikeMetaObject. Reject it explicitly so the caller falls through to the next addon, which is what should have happened when the lookup failed. Tests cover the verbatim placeholder payload and confirm that legitimate titles containing brackets, such as "[Dub] Naruto Shippuuden", still parse.
load() walked its meta manifests with no timeout at all, while fetch() -- the prefetch path reaching the same tryFetchMeta -- has wrapped each attempt in withTimeoutOrNull(FETCH_TIMEOUT_MS) all along. The asymmetry only shows when an addon accepts the connection and then stalls: the desktop HTTP client allows 60s to connect and 60s to read, so a details screen could sit on MetaDetailsUiState(isLoading = true) for a minute per addon in turn, with no meta to render behind it. That reads as a permanently black screen, and the in-flight guard means tapping again does nothing but wait. Use a separate 20s budget rather than reusing FETCH_TIMEOUT_MS: this is the interactive path and some payloads are genuinely large -- One Piece is about 1.3 MB across 1239 videos -- so 5s would give up on responses that were going to arrive. The point is an upper bound, not a tight one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two defects in the same meta-resolution path, both surfaced by one report
("clicking Naruto or One Piece loads forever on a black screen"):
MetaDetailsParser.parseaccepted an aggregator addon's error placeholder asreal metadata, so it was stored and synced as a series title.
MetaDetailsRepository.load— the details screen path — had no timeout at all,while
fetch()reaching the sametryFetchMetahas always had one.PR type
Why
Defect 1 — error placeholder stored as metadata.
A series appeared in Continue Watching titled
[X] Anime Kitsu, withRequest for meta for Anime Kitsu timed out after 30000mswhere its synopsisbelongs. Not a render glitch — it was persisted on disk:
Written on a webOS install, it surfaced on desktop as the title in the player
overlay. The aggregator addon answers a failed upstream lookup with a
well-formed meta object rather than an error:
{"meta": {"id":"tt0988824", "type":"series", "name":"[X] Anime Kitsu", "description":"Request for meta for Anime Kitsu timed out after 30000ms"}}looksLikeMetaObjectasks only whetherid,typeandnameexist. Theplaceholder has all three, so presence validation cannot catch it. The 30000ms
is the aggregator's own internal budget for its sub-addon; no client knows that
name. Rejecting the placeholder lets the caller fall through to the next addon,
which is what should have happened when the lookup failed.
Defect 2 — unbounded fetch on the details screen.
load()walked its manifests with no timeout. With a 60s connect + 60s read HTTPclient, an addon that accepts the connection and then stalls leaves the screen on
MetaDetailsUiState(isLoading = true)with no meta to render behind it — a blackscreen — for up to a minute per addon in turn. The in-flight guard means tapping
again does nothing but wait.
Desktop scope
Windows (where this was diagnosed), and desktop shared code. Both files are in
commonMain, so the parser guard also benefits the other targets — the corruptedrecord here was in fact produced by the webOS client and reached desktop via sync.
The HTTP timeouts that make defect 2 visible are the desktop client's
(
AddonPlatform.desktop.kt: 60s connect, 60s read).Issue or approval
Fixes #2 — filed from this diagnosis, with the persisted corrupted records
quoted verbatim as evidence.
UI / behavior impact
No UI code changed. The visible effect is that a stored title stops being an
addon's error string, and that the details screen stops waiting without bound.
Policy check
CONTRIBUTING.md.Caveat on "limited to one problem": this is strictly two defects, in two
commits, and I would rather flag that than quietly tick the box. They share one
file pair, one code path and one user report, and the timeout is what turns a
failed lookup into an unbounded wait. Happy to split into two PRs if preferred.
Scope boundaries
Not changed, deliberately:
tryFetchMetaByAlternateIdis also unbounded, but it does not exist onDev—it is unmerged work on another branch. Left alone here.
10.5 MB with the same title fetched up to 5 times. That is performance, not this bug.
install were corrected by hand; the guard only prevents recurrence.
MetaDetailsParser's handling of any other malformed payload.Testing
MetaDetailsParserTest— 11/11 passing:Two new cases: the verbatim placeholder payload is rejected, and
[Dub] Naruto Shippuudenstill parses so legitimate bracketed titles areunaffected.
Not covered, stated plainly: the black screen was never captured under
instrumentation. Thread dumps every 15s across the session caught only one
transient 30s block that resolved itself. The timeout makes an unbounded wait
impossible, but it bounds the symptom rather than proving the cause — and the
placeholder cannot explain the One Piece case, whose
tt0388629is served bycinemeta and never touches that addon. Defect 1 is proven; defect 2 is a real
unbounded path that would produce this symptom.
Environment: Windows 11, desktop build from
Dev, JDK 21.0.11.Note:
composeApp/src/commonTest/.../PlayerExitOrderingTest.ktdoes not compile ona clean
Devcheckout. Pre-existing and unrelated — verified by stashing thesechanges and rebuilding on untouched
Dev, which reproduces the same single error.Screenshots / Video
Not a UI change.
Breaking changes
None. A meta response that previously parsed into a placeholder-titled entry now
fails parsing, so the caller advances to the next addon — the path already taken
for any other unusable response.
Linked issues
Fixes #2