Skip to content

[Bug]: a failed timeline save is invisible — the edit stays on screen, the document is not written #282

Description

@EtienneLescot

The bug

A failed save during a timeline edit is invisible to the user. Delete a clip, drop a
region, duplicate a clip — if the write fails, the UI still shows the edit applied, the
document on disk is unchanged, and nothing tells the user. They find out on reload,
having lost the work.

Found while auditing detached promises in #281, which fixed the sites where a rejection
could actually escape. These twelve were deliberately left alone, because the right fix
is a product decision rather than an audit cleanup — see "Why not just add .catch()"
below.

The chain

void tl.removeClip(c.id)                            ← V4Timeline.tsx:1636, no catch
  └─ await saveDocument(removeClipInDocument(...))   ← useTimeline.ts, 0 catch, 0 toast
       └─ throw new Error(result.error)              ← projectStore.ts, when the IPC save fails

useTimeline and useSequentialTimelineOps contain zero catch and zero toast, and
projectStore.saveDocument throws on a failed write. Every caller below detaches the
promise with void, so the throw goes nowhere — it lands as an unhandled rejection in the
renderer console and the user sees nothing.

Realistic triggers: disk full, the project file locked by another process, a main-process
IPC handler failing, a path that vanished under the app.

The twelve sites

As of perf/test-suite-environment-split:

File:line Call
src/components/ai-edition/NewEditorShell.tsx:383 state.saveDocument(next)
src/components/ai-edition/NewEditorShell.tsx:394 state.saveDocument(next)
src/components/ai-edition/NewEditorShell.tsx:417 tl.insertClipAt(...)
src/components/ai-edition/NewEditorShell.tsx:559 applyTimelineOp({...})
src/components/ai-edition/NewEditorShell.tsx:573 applyTimelineOp({...})
src/components/ai-edition/NewEditorShell.tsx:884 tl.removeRegions(tl.multiSelection)
src/components/ai-edition/NewEditorShell.tsx:888 tl.removeRegion(...)
src/components/ai-edition/NewEditorShell.tsx:920 handleCopyRegion().then(() => tl.removeRegion(...))
src/components/ai-edition/NewEditorShell.tsx:932 tl.duplicateClip(...)
src/components/ai-edition/v4/FloatingInspector.tsx:466 tl.removeRegion(...)
src/components/ai-edition/v4/FloatingInspector.tsx:979 tl.removeRegion("trim", ...)
src/components/ai-edition/v4/V4Timeline.tsx:1636 tl.removeClip(c.id)

Re-derive with:

grep -rnE "void [^;]*(saveDocument|applyTimelineOp|tl\.(remove|insert|move|duplicate|split))" --include="*.tsx" --include="*.ts" src | grep -v "\.test\." | grep -v "\.catch"

Why not just add .catch() to each

Twelve scattered .catch(err => console.warn(...)) would silence the linting concern and
make the real problem worse: it converts silent data loss into a console line nobody
reads, and it looks fixed. That is why #281 stopped here rather than papering over it.

All twelve route through two functions. The guard belongs there, once, not at twelve
call sites.

Suggested fix

Handle it inside useTimeline and useSequentialTimelineOps, so every current and future
caller is covered without ceremony. The pattern already exists next door — handleSave in
NewEditorShell.tsx does exactly this:

try {
  await saveDocument(doc);
  toast.success("Project saved");
} catch (err) {
  toast.error("Save failed", {
    description: err instanceof Error ? err.message : String(err),
  });
}

Two options for what the user sees, and this is the decision the issue exists to make:

A — toast on every failed mutation. Loud and honest, matches handleSave, smallest
diff. Downside: a repeated failure (locked file) toasts on every edit.

B — a persistent "unsaved changes" indicator in the top bar, set when a mutation fails
and cleared on the next success. Quieter and arguably more correct for an editor, but more
code and new state to own.

A is the lazy one and is probably enough; B is worth it only if we expect repeated
failures rather than one-offs.

Whichever is chosen, the UI must not keep showing an edit as applied when the document was
not written — that inconsistency is the actual bug, the missing toast is only the symptom.

Acceptance criteria

  • A failed timeline mutation is surfaced to the user (option A or B), not just logged.
  • The handling lives in useTimeline / useSequentialTimelineOps, not duplicated across the twelve call sites.
  • No unhandled rejection escapes: process.on("unhandledRejection") stays quiet when saveDocument is forced to throw.
  • A test drives one mutation with a rejecting saveDocument and asserts both the surfaced error and that nothing escapes. Ablate it against the unfixed code to prove it has teeth.
  • The grep above returns nothing (or only sites with a documented reason).

Notes

useTimeline's background duration probe was already fixed in #281 (same shape, but
genuinely opportunistic — losing it costs a placeholder length, not user work), so it is
not in the list above and is not part of this issue.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions