From 24d4e9ce9657713fa152cfecf399f87dd479f5c0 Mon Sep 17 00:00:00 2001 From: Jiho Lee Date: Sun, 26 Jul 2026 23:30:26 +0900 Subject: [PATCH] fix(editor): show an error instead of an endless skeleton when metadata fails getRecordingMetaByPath returns Result and fails whenever recording-meta.json is missing or unparseable. The editor's status derivation only looked at the query's data: const meta = rawMetaQuery.data; if (!meta) return "loading" as const; A failed query has no data either, so a failure was indistinguishable from a query still in flight. The Switch had no arm for the error case, so it fell through to the spinner and stayed there -- which is #1812: the editor opens on the loading skeleton and never leaves it. EditorErrorScreen already existed for this, complete with a recovery path; it was simply unreachable on a metadata failure. It is now wired to the new error state. The error also has to outrank the lockedToImporting latch. That latch exists so the UI doesn't flicker while metadata is re-read, but if the metadata stops being readable mid-import there is nothing to wait for, and it would strand the import screen the same way. Extracted both derivations into import-status.ts so the state machine is testable without a running editor, and added 8 vitest cases. Confirmed they fail without the fix: reverting just the two error branches gives 3 failed | 5 passed. --- apps/desktop/src/routes/editor/Editor.tsx | 33 ++++++------ .../src/routes/editor/import-status.test.ts | 54 +++++++++++++++++++ .../src/routes/editor/import-status.ts | 48 +++++++++++++++++ 3 files changed, 117 insertions(+), 18 deletions(-) create mode 100644 apps/desktop/src/routes/editor/import-status.test.ts create mode 100644 apps/desktop/src/routes/editor/import-status.ts diff --git a/apps/desktop/src/routes/editor/Editor.tsx b/apps/desktop/src/routes/editor/Editor.tsx index f53a293e62..a863721047 100644 --- a/apps/desktop/src/routes/editor/Editor.tsx +++ b/apps/desktop/src/routes/editor/Editor.tsx @@ -54,6 +54,7 @@ import { import { EditorErrorScreen } from "./EditorErrorScreen"; import { Header } from "./Header"; import { ImportProgress } from "./ImportProgress"; +import { deriveImportStatus, deriveRawImportStatus } from "./import-status"; import { PlayerContent } from "./Player"; import { Timeline } from "./Timeline"; import { Dialog, DialogContent, EditorButton, Input, Subfield } from "./ui"; @@ -148,20 +149,12 @@ export function Editor() { refetchOnReconnect: false, })); - const rawImportStatus = createMemo(() => { - const meta = rawMetaQuery.data; - if (!meta) return "loading" as const; - if ( - "status" in meta && - meta.status && - typeof meta.status === "object" && - "status" in meta.status && - meta.status.status === "InProgress" - ) { - return "importing" as const; - } - return "ready" as const; - }); + const rawImportStatus = createMemo(() => + deriveRawImportStatus({ + data: rawMetaQuery.data, + isError: rawMetaQuery.isError, + }), + ); const [lockedToImporting, setLockedToImporting] = createSignal(false); @@ -171,10 +164,8 @@ export function Editor() { } }); - const importStatus = () => { - if (lockedToImporting()) return "importing" as const; - return rawImportStatus(); - }; + const importStatus = () => + deriveImportStatus(rawImportStatus(), lockedToImporting()); const [importAborted, setImportAborted] = createSignal(false); @@ -211,6 +202,12 @@ export function Editor() { } > + + + diff --git a/apps/desktop/src/routes/editor/import-status.test.ts b/apps/desktop/src/routes/editor/import-status.test.ts new file mode 100644 index 0000000000..82599f509f --- /dev/null +++ b/apps/desktop/src/routes/editor/import-status.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; + +import { deriveImportStatus, deriveRawImportStatus } from "./import-status"; + +describe("editor import status", () => { + it("reports loading while the metadata query is still in flight", () => { + expect(deriveRawImportStatus({ data: undefined, isError: false })).toBe( + "loading", + ); + }); + + it("reports ready once metadata arrives", () => { + expect( + deriveRawImportStatus({ data: { status: null }, isError: false }), + ).toBe("ready"); + }); + + it("reports importing while an import is in progress", () => { + expect( + deriveRawImportStatus({ + data: { status: { status: "InProgress" } }, + isError: false, + }), + ).toBe("importing"); + }); + + // #1812: a failed query also has no data. Before the fix this returned + // "loading", so an unreadable recording-meta.json left the editor on the + // skeleton with no error and no way forward. + it("reports error when the metadata query fails", () => { + expect(deriveRawImportStatus({ data: undefined, isError: true })).toBe( + "error", + ); + }); + + it("still reports error when a stale success payload is present", () => { + expect( + deriveRawImportStatus({ data: { status: null }, isError: true }), + ).toBe("error"); + }); + + it("keeps showing the import screen while the lock is held", () => { + expect(deriveImportStatus("loading", true)).toBe("importing"); + }); + + it("lets an error override the importing lock", () => { + expect(deriveImportStatus("error", true)).toBe("error"); + }); + + it("passes through the raw status when nothing is latched", () => { + expect(deriveImportStatus("ready", false)).toBe("ready"); + expect(deriveImportStatus("loading", false)).toBe("loading"); + }); +}); diff --git a/apps/desktop/src/routes/editor/import-status.ts b/apps/desktop/src/routes/editor/import-status.ts new file mode 100644 index 0000000000..272d44bec0 --- /dev/null +++ b/apps/desktop/src/routes/editor/import-status.ts @@ -0,0 +1,48 @@ +/** + * Which screen the editor should show while a recording's metadata loads. + * + * Extracted from `Editor.tsx` so the state machine can be tested without a + * running editor. The case that matters is `error`: `getRecordingMetaByPath` + * returns `Result` and fails whenever + * `recording-meta.json` is missing or unparseable. A failed query has no + * `data`, so treating "no data" as "still loading" leaves the editor on the + * loading skeleton indefinitely (#1812). + */ +export type EditorImportStatus = "loading" | "importing" | "ready" | "error"; + +export function deriveRawImportStatus(query: { + data: unknown; + isError: boolean; +}): EditorImportStatus { + if (query.isError) return "error"; + if (!query.data) return "loading"; + + const meta = query.data as { status?: unknown }; + if ( + "status" in meta && + meta.status && + typeof meta.status === "object" && + "status" in meta.status && + (meta.status as { status?: unknown }).status === "InProgress" + ) { + return "importing"; + } + + return "ready"; +} + +/** + * `lockedToImporting` latches once an import starts, so the UI doesn't flicker + * between screens as the metadata is re-read. An error has to outrank that + * latch: if the metadata stops being readable mid-import there is nothing left + * to wait for, and without this the editor would stay on the import screen for + * the same reason it used to stay on the skeleton. + */ +export function deriveImportStatus( + rawStatus: EditorImportStatus, + lockedToImporting: boolean, +): EditorImportStatus { + if (rawStatus === "error") return "error"; + if (lockedToImporting) return "importing"; + return rawStatus; +}