diff --git a/docs/requirements.md b/docs/requirements.md index f5fdb0960..edaac50ab 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -498,6 +498,7 @@ Internal architecture, components, and application logic. | DR-294 | A download plays with no network. Playing a downloaded item asked the server for its `PlaybackInfo` — only to read the media-source id that subtitle URLs are keyed by — and `HybridRepository::get_playback_info` went to the server alone, so offline the call retried for seven seconds, failed, and the file on disk was never opened. A completed download for the current user now answers playback info from its download row, first and regardless of reachability: the local path, direct play, and the item id as media-source id (a download names no source, so the server served its default, which carries the item's id). Next Up had the same shape — server-only — and the TV landing page loads it in one `Promise.all` with its other rows, so offline that single failure blanked the whole page with Continue Watching and Latest sitting in the cache; it now falls back to the cache when the server cannot answer. And a slow cache read is waited for, never discarded: the cache is one SQLite connection behind one mutex, so any write in progress (the catalog sync at every launch, a download finishing) pushes a read past the 100 ms fast path, and `get_items`, the library list, genres and playlist items discarded such a read, waited on the server, and offline returned its error over data on disk — "More info" on a downloaded show failed exactly so. They keep the read running (`cache_try`) and wait for it when the server fails (`settle`); the cache-only reads (search, favourites) simply await the cache | Repository | UR-002, UR-071 | Done | | DR-295 | A series page lists its episodes with one concurrent season fan-out. "More info" on Frasier took ~10 s: the page asked Rust for the episodes and for the current episode as two commands, each of which walked every season, and each walk fetched the eleven seasons one after another — so the wait was the sum of twenty-two listings, each a cache read slowed by whatever the database was writing (the catalog sync at launch measured it at ~4 s per walk). The seasons are now fetched together (`gather_season_episodes`, so the wait is the slowest season), and `repository_get_series_view` returns the episodes and the current episode from one walk, with Next Up and resume fetched alongside it | Repository | UR-062 | Done | | DR-296 | Returning from background audio resumes the item the native player is actually on, not the one the video page was mounted with. An episode that ends while backgrounded advances in the backend (`advance_to_next_episode_audio_only`), but `player_exit_background_audio` returned only a position, so the webview reloaded the *previous* episode at the new episode's timestamp. The command now returns `BackgroundAudioResume { itemId, positionSeconds }` (`PlayerController::background_audio_resume`); `planHandoffReturn` yields `other-item` when the id differs from the mounted one, and the player page navigates to that episode with `resumeAt=`, recording the outgoing episode as watched and suppressing the stale unmount stop report | Playback | UR-040, UR-023 | Done (pending device verification) | +| DR-297 | A background refresh never blanks a library detail page that is on screen, and every load that succeeds clears the page's error. Resuming the app after a few minutes in the background replaced the Frasier series page with "Failed to load item": the resume reload (reconnect / offline-filter change) is a refresh of content the cache had already answered, but any throw in it replaced the page with an error that no later successful reload of the same item cleared — and the catch logged nothing and turned backend errors (plain strings) into the generic text. A failed refresh now keeps the page and logs the thrown value; only failing to open an item shows an error, with the backend's own message | UI | UR-062 | Done | --- @@ -567,7 +568,7 @@ Internal architecture, components, and application logic. | UR-058 | - | DR-087, DR-142 | | UR-060 | - | DR-090, DR-091, DR-111 | | UR-061 | - | DR-092 | -| UR-062 | - | DR-101, DR-102, DR-103, DR-104, DR-107, DR-295 | +| UR-062 | - | DR-101, DR-102, DR-103, DR-104, DR-107, DR-295, DR-297 | | UR-063 | - | DR-105 | | UR-064 | - | DR-106 | | UR-065 | IR-030 | DR-108, DR-109, DR-110, DR-111 | @@ -859,6 +860,7 @@ Internal architecture, components, and application logic. | UT-264 | Ten seasons whose listings each take 100 ms are gathered in well under the 1 s a sequential walk takes, and a season that fails to load leaves the other nine seasons' episodes in the result | DR-295 | Done | | UT-265 | `planHandoffReturn` switches to the item the backend advanced to while backgrounded, and reloads in place when the backend is still on the mounted item or reports none | DR-296 | Done | | UT-266 | After a background-audio episode advance, the controller's resume point names the new episode and carries no base from the previous one | DR-296 | Done | +| UT-267 | A failed refresh of a detail page already on screen shows no error, a successful load clears any error, a failure to open an item shows its message, and a backend error's plain-string text is shown rather than a generic fallback | DR-297 | Done | ### Integration Tests | Test ID | Test Description | Traces To | Status | diff --git a/src/lib/components/library/detailLoadError.test.ts b/src/lib/components/library/detailLoadError.test.ts new file mode 100644 index 000000000..0e37f20af --- /dev/null +++ b/src/lib/components/library/detailLoadError.test.ts @@ -0,0 +1,43 @@ +import { describe, it, expect } from "vitest"; +import { errorAfterLoad, loadErrorMessage } from "./detailLoadError"; + +/** + * TRACES: UR-062 | DR-297 | UT-267 + * + * Resuming the app after a few minutes in the background replaced a series page + * that was on screen with "Failed to load item". The resume reload is a refresh + * of content the cache had already answered, yet any throw in it blanked the + * page — and no later successful reload of the same item ever cleared it. + */ +describe("errorAfterLoad", () => { + it("clears the error when a load succeeds", () => { + expect(errorAfterLoad({ ok: true }, { refreshing: true })).toBeNull(); + expect(errorAfterLoad({ ok: true }, { refreshing: false })).toBeNull(); + }); + + it("keeps a page on screen when a refresh of it fails", () => { + expect(errorAfterLoad({ ok: false, error: "boom" }, { refreshing: true })).toBeNull(); + }); + + it("reports a failure to open an item that is not on screen yet", () => { + expect(errorAfterLoad({ ok: false, error: new Error("Offline") }, { refreshing: false })).toBe( + "Offline", + ); + }); +}); + +describe("loadErrorMessage", () => { + it("shows the text of a backend error, which arrives as a plain string", () => { + expect(loadErrorMessage("Repository not found")).toBe("Repository not found"); + }); + + it("shows an Error's message", () => { + expect(loadErrorMessage(new TypeError("x is undefined"))).toBe("x is undefined"); + }); + + it("falls back to a generic message for anything else", () => { + expect(loadErrorMessage(undefined)).toBe("Failed to load item"); + expect(loadErrorMessage({})).toBe("Failed to load item"); + expect(loadErrorMessage("")).toBe("Failed to load item"); + }); +}); diff --git a/src/lib/components/library/detailLoadError.ts b/src/lib/components/library/detailLoadError.ts new file mode 100644 index 000000000..963671465 --- /dev/null +++ b/src/lib/components/library/detailLoadError.ts @@ -0,0 +1,35 @@ +// What the library detail page's error slot shows after a load. +// +// Extracted from `/library/[id]/+page.svelte` so it can be unit-tested. + +export type LoadOutcome = { ok: true } | { ok: false; error: unknown }; + +/** + * The text of whatever a load threw. Backend commands reject with a plain + * string, not an `Error`, so reading only `Error.message` replaced every + * backend error with the generic fallback and hid what actually failed. + */ +export function loadErrorMessage(error: unknown): string { + if (error instanceof Error && error.message) return error.message; + if (typeof error === "string" && error) return error; + return "Failed to load item"; +} + +/** + * The error to show once a load has ended. + * + * A success always clears it — a same-item reload used to leave a stale error + * up for good. A failed *refresh* of a page already on screen shows none: the + * page is still showing what the cache answered, and a background refresh + * (reconnect, resume, filter change) must not blank it. Only failing to open an + * item leaves the viewer with nothing to see. + * + * TRACES: UR-062 | DR-297 | UT-267 + */ +export function errorAfterLoad( + outcome: LoadOutcome, + options: { refreshing: boolean }, +): string | null { + if (outcome.ok || options.refreshing) return null; + return loadErrorMessage(outcome.error); +} diff --git a/src/routes/library/[id]/+page.svelte b/src/routes/library/[id]/+page.svelte index c4c6dcd33..24e53d8a0 100644 --- a/src/routes/library/[id]/+page.svelte +++ b/src/routes/library/[id]/+page.svelte @@ -1,4 +1,4 @@ - +