fix(library): resuming the app no longer blanks the page on screen
Coming back to the app after a few minutes in the background replaced the Frasier series page with "Failed to load item". Android cuts a backgrounded app's network, the app declares the server offline, and on resume the reconnect and offline-filter reloads refresh the page. Every backend call in that refresh answered from the cache, yet something in it threw, and: - any throw replaced the whole page with an error, although it was a refresh of content already on screen; - no later successful reload of the same item cleared that error, so it stayed until the viewer navigated away; - the catch logged nothing and turned every non-Error value (backend errors arrive as plain strings) into the generic text, so neither logcat nor the screen said what failed. A failed refresh now keeps the page and logs the value actually thrown; every successful load clears the error; failing to open an item still shows one, with the backend's own message. The decision lives in detailLoadError.ts so it is unit-tested. The throw itself is not yet identified - the next occurrence names itself in the log. DR-297, UT-267.
This commit is contained in:
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
<!-- TRACES: UR-035, UR-038, UR-048, UR-058, UR-062 | DR-043, DR-062, DR-102, DR-103, DR-142 -->
|
||||
<!-- TRACES: UR-035, UR-038, UR-048, UR-058, UR-062 | DR-043, DR-062, DR-102, DR-103, DR-142, DR-297 -->
|
||||
<script lang="ts">
|
||||
import { untrack } from "svelte";
|
||||
import { formatDuration } from "$lib/utils/duration";
|
||||
@@ -52,6 +52,7 @@
|
||||
} from "$lib/components/library/seriesNavigation";
|
||||
import { createLogger } from "$lib/utils/logger";
|
||||
import { createCoalescedLoader } from "$lib/utils/coalescedLoader";
|
||||
import { errorAfterLoad } from "$lib/components/library/detailLoadError";
|
||||
|
||||
const log = createLogger("LibraryDetail");
|
||||
|
||||
@@ -250,8 +251,12 @@
|
||||
}
|
||||
}
|
||||
}
|
||||
error = errorAfterLoad({ ok: true }, { refreshing: !isNewItem });
|
||||
} catch (e) {
|
||||
error = e instanceof Error ? e.message : "Failed to load item";
|
||||
// A failed refresh keeps the page up (DR-297), so this log is the only
|
||||
// trace it leaves — it must carry the value actually thrown.
|
||||
log.error(`Failed to ${isNewItem ? "load" : "refresh"} item ${itemId}:`, e);
|
||||
error = errorAfterLoad({ ok: false, error: e }, { refreshing: !isNewItem });
|
||||
} finally {
|
||||
loading = false;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user