From 7b8e5fbfbee1f733426e741e0e8dd5730c5c0332 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Thu, 13 Aug 2026 12:51:44 +0300 Subject: [PATCH] fix(app): harden document recovery lifecycle - Serialize recovery cleanup with active snapshot writes - Preserve version ordering across autosave, restore, and cleanup - Fall back to memory when IndexedDB operations fail --- packages/vue/src/i18n/locales/fr/dialogs.json | 2 +- src/app/document/io/source.ts | 4 +- src/app/document/io/write.ts | 6 +- src/app/document/recovery/controller.ts | 20 ++++-- src/app/document/recovery/store.ts | 48 +++++++++++-- src/app/tabs/index.ts | 13 +++- src/components/recovery/RecoveryDialog.vue | 2 + .../app/document/recovery/controller.test.ts | 69 +++++++++++++++++++ tests/engine/tauri/document-io.test.ts | 17 +++++ 9 files changed, 164 insertions(+), 17 deletions(-) diff --git a/packages/vue/src/i18n/locales/fr/dialogs.json b/packages/vue/src/i18n/locales/fr/dialogs.json index 4e64f91c9..345f8e5ff 100644 --- a/packages/vue/src/i18n/locales/fr/dialogs.json +++ b/packages/vue/src/i18n/locales/fr/dialogs.json @@ -5,7 +5,7 @@ "recoverUnsavedWork": "Récupérer le travail non enregistré", "recoverUnsavedWorkDescription": "OpenPencil a trouvé des documents d’une session précédente.", "restore": "Restaurer", - "discard": "Ignorer", + "discard": "Supprimer", "recoveryFailed": "Impossible de restaurer ce document.", "rename": "Renommer", "renameLayers": "Renommer {count} calques", diff --git a/src/app/document/io/source.ts b/src/app/document/io/source.ts index 94001bb8e..18d2b8d66 100644 --- a/src/app/document/io/source.ts +++ b/src/app/document/io/source.ts @@ -87,7 +87,9 @@ export function createDocumentSourceActions({ getSavedVersion, hasWritableSource: () => !!getFileHandle() || !!getFilePath() || !!getStorageBinding(), saveCurrentDocument: async () => { - await writeFile(await buildFigFile()) + const version = state.sceneVersion + const data = await buildFigFile() + await writeFile(data, version) } }) diff --git a/src/app/document/io/write.ts b/src/app/document/io/write.ts index 3a6eb709b..e80ad15cb 100644 --- a/src/app/document/io/write.ts +++ b/src/app/document/io/write.ts @@ -27,7 +27,11 @@ export function createDocumentWriter({ }: DocumentWriterOptions) { async function finishWrite(version: number): Promise { setSavedVersion(version) - await onWriteSuccess?.(version) + try { + await onWriteSuccess?.(version) + } catch (error) { + console.warn('[Recovery] Cleanup after document write failed:', error) + } return true } diff --git a/src/app/document/recovery/controller.ts b/src/app/document/recovery/controller.ts index 547f57876..3b9004901 100644 --- a/src/app/document/recovery/controller.ts +++ b/src/app/document/recovery/controller.ts @@ -35,6 +35,7 @@ export function createDocumentRecovery({ }: DocumentRecoveryOptions): DocumentRecoveryController { let id = recoveryId let protectedVersion = state.sceneVersion + let persistedVersion: number | null = null let requestedVersion = protectedVersion let lifecycleGeneration = 0 let writing: Promise | null = null @@ -52,6 +53,7 @@ export function createDocumentRecovery({ sceneVersion: version, figBytes: bytes }) + persistedVersion = version if (generation !== lifecycleGeneration) return protectedVersion = version if (requestedVersion !== version) await runWrites(generation) @@ -80,25 +82,35 @@ export function createDocumentRecovery({ { debounce: 3000, maxWait: 10000 } ) + async function invalidateActiveWrite(): Promise { + lifecycleGeneration++ + await writing + } + return { getRecoveryId: () => id, adoptRecoverySnapshot(nextId, sceneVersion) { lifecycleGeneration++ id = nextId protectedVersion = sceneVersion + persistedVersion = sceneVersion requestedVersion = sceneVersion disposed = false }, persistNow, async markProtectedVersion(version) { - lifecycleGeneration++ + await invalidateActiveWrite() protectedVersion = version - requestedVersion = version - await store.remove(id) + requestedVersion = state.sceneVersion + if (persistedVersion == null || persistedVersion <= version) { + await store.remove(id) + persistedVersion = null + } }, async discardRecovery() { - lifecycleGeneration++ + await invalidateActiveWrite() protectedVersion = state.sceneVersion + persistedVersion = null requestedVersion = state.sceneVersion await store.remove(id) }, diff --git a/src/app/document/recovery/store.ts b/src/app/document/recovery/store.ts index 40f379e6a..956252325 100644 --- a/src/app/document/recovery/store.ts +++ b/src/app/document/recovery/store.ts @@ -1,20 +1,54 @@ import { createIdbRecoveryStore } from '@/app/document/recovery/idb' import { createMemoryRecoveryStore } from '@/app/document/recovery/memory' -import type { RecoveryStore } from '@/app/document/recovery/types' +import type { + RecoverySnapshot, + RecoverySnapshotInput, + RecoverySnapshotMeta, + RecoveryStore +} from '@/app/document/recovery/types' let singleton: RecoveryStore | null = null let memoryFallback = false +function warnMemoryFallback(error?: unknown): void { + if (memoryFallback) return + console.warn('[Recovery] IndexedDB unavailable; crash recovery is limited to this session', error) + memoryFallback = true +} + +function createResilientRecoveryStore(primary: RecoveryStore): RecoveryStore { + let current = primary + + async function run(operation: (store: RecoveryStore) => Promise): Promise { + try { + return await operation(current) + } catch (error) { + if (current !== primary) throw error + warnMemoryFallback(error) + current = createMemoryRecoveryStore() + return operation(current) + } + } + + return { + list: () => run((store) => store.list()), + read: (id: string): Promise => run((store) => store.read(id)), + write: (input: RecoverySnapshotInput): Promise => + run((store) => store.write(input)), + remove: (id: string): Promise => run((store) => store.remove(id)), + clear: (): Promise => run((store) => store.clear()) + } +} + export function getRecoveryStore(): RecoveryStore { if (singleton) return singleton - if (typeof indexedDB !== 'undefined') { - singleton = createIdbRecoveryStore() - memoryFallback = false - } else { - console.warn('[Recovery] IndexedDB unavailable; crash recovery is limited to this session') + if (typeof indexedDB === 'undefined') { + warnMemoryFallback() singleton = createMemoryRecoveryStore() - memoryFallback = true + return singleton } + memoryFallback = false + singleton = createResilientRecoveryStore(createIdbRecoveryStore()) return singleton } diff --git a/src/app/tabs/index.ts b/src/app/tabs/index.ts index 9bfa1d40b..59e51b2ff 100644 --- a/src/app/tabs/index.ts +++ b/src/app/tabs/index.ts @@ -36,6 +36,7 @@ function generateTabId(): string { } const tabsRef = shallowRef([]) +const pendingRecoveryDeletions = new Set>() const activeTabId = shallowRef('') export const activeTab = computed(() => tabsRef.value.find((t) => t.id === activeTabId.value)) @@ -97,8 +98,11 @@ export function closeTab(tabId: string) { const closingTab = tabsRef.value[idx] const wasActive = activeTabId.value === tabId + const deletion = closingTab.store.discardRecovery().finally(() => { + pendingRecoveryDeletions.delete(deletion) + }) + pendingRecoveryDeletions.add(deletion) closingTab.store.dispose() - void closingTab.store.discardRecovery() tabsRef.value = tabsRef.value.filter((t) => t.id !== tabId) if (tabsRef.value.length === 0) { @@ -292,7 +296,7 @@ export async function restoreRecoverySnapshot(id: string): Promise { store.replaceGraph(imported) store.undo.clear() store.state.documentName = snapshot.documentName - store.adoptRecoverySnapshot(id, store.state.sceneVersion) + store.adoptRecoverySnapshot(id, snapshot.sceneVersion) store.clearSelection() const pageId = store.graph.getPages()[0]?.id ?? store.graph.rootId await store.switchPage(pageId) @@ -300,7 +304,10 @@ export async function restoreRecoverySnapshot(id: string): Promise { } export async function prepareForReload(): Promise { - await Promise.all(tabsRef.value.map((tab) => tab.store.persistRecoveryNow())) + await Promise.all([ + ...pendingRecoveryDeletions, + ...tabsRef.value.map((tab) => tab.store.persistRecoveryNow()) + ]) } export function tabCount(): number { diff --git a/src/components/recovery/RecoveryDialog.vue b/src/components/recovery/RecoveryDialog.vue index 94d2a24e9..413ecafaf 100644 --- a/src/components/recovery/RecoveryDialog.vue +++ b/src/components/recovery/RecoveryDialog.vue @@ -28,6 +28,7 @@ async function restore(snapshot: RecoverySnapshotMeta): Promise { try { await restoreRecoverySnapshot(snapshot.id) snapshots.value = snapshots.value.filter((candidate) => candidate.id !== snapshot.id) + if (snapshots.value.length === 0) open.value = false } catch (error) { toast.error(error instanceof Error ? error.message : dialogs.value.recoveryFailed) } finally { @@ -40,6 +41,7 @@ async function discard(snapshot: RecoverySnapshotMeta): Promise { try { await discardRecoverySnapshot(snapshot.id) snapshots.value = snapshots.value.filter((candidate) => candidate.id !== snapshot.id) + if (snapshots.value.length === 0) open.value = false } finally { busyId.value = null } diff --git a/tests/engine/app/document/recovery/controller.test.ts b/tests/engine/app/document/recovery/controller.test.ts index 0e61814ac..c3f71f971 100644 --- a/tests/engine/app/document/recovery/controller.test.ts +++ b/tests/engine/app/document/recovery/controller.test.ts @@ -6,6 +6,22 @@ import { createDefaultEditorState } from '@open-pencil/core/editor' import { createDocumentRecovery } from '@/app/document/recovery/controller' import { createMemoryRecoveryStore } from '@/app/document/recovery/memory' +import type { RecoverySnapshotInput, RecoveryStore } from '@/app/document/recovery/types' + +function deferredWriteStore() { + const memory = createMemoryRecoveryStore() + let release: (() => void) | null = null + const store: RecoveryStore = { + ...memory, + async write(input: RecoverySnapshotInput) { + await new Promise((resolve) => { + release = resolve + }) + return memory.write(input) + } + } + return { store, release: () => release?.() } +} function setup(buildFigFile = async () => new Uint8Array([1, 2, 3])) { const state = reactive({ ...createDefaultEditorState('page-1'), documentName: 'Agent draft' }) @@ -80,4 +96,57 @@ describe('document recovery controller', () => { expect(await store.list()).toEqual([]) recovery.disposeRecovery() }) + + test('save waits for an active write before deleting its snapshot', async () => { + const deferred = deferredWriteStore() + const state = reactive({ ...createDefaultEditorState('page-1'), documentName: 'Draft' }) + const recovery = createDocumentRecovery({ + state, + store: deferred.store, + recoveryId: 'recovery-1', + hasWritableSource: () => false, + buildFigFile: () => new Uint8Array([1]) + }) + state.sceneVersion = 1 + const write = recovery.persistNow() + await Promise.resolve() + const cleanup = recovery.markProtectedVersion(1) + deferred.release() + await Promise.all([write, cleanup]) + + expect(await deferred.store.list()).toEqual([]) + recovery.disposeRecovery() + }) + + test('discard waits for an active write before deleting its snapshot', async () => { + const deferred = deferredWriteStore() + const state = reactive({ ...createDefaultEditorState('page-1'), documentName: 'Draft' }) + const recovery = createDocumentRecovery({ + state, + store: deferred.store, + recoveryId: 'recovery-1', + hasWritableSource: () => false, + buildFigFile: () => new Uint8Array([1]) + }) + state.sceneVersion = 1 + const write = recovery.persistNow() + await Promise.resolve() + const discard = recovery.discardRecovery() + deferred.release() + await Promise.all([write, discard]) + + expect(await deferred.store.list()).toEqual([]) + recovery.disposeRecovery() + }) + + test('preserves a snapshot newer than the saved version', async () => { + const { state, store, recovery } = setup() + state.sceneVersion = 2 + await recovery.persistNow() + + await recovery.markProtectedVersion(1) + + expect((await store.read('recovery-1'))?.sceneVersion).toBe(2) + recovery.disposeRecovery() + }) }) diff --git a/tests/engine/tauri/document-io.test.ts b/tests/engine/tauri/document-io.test.ts index 1b4f19ceb..f58e3f503 100644 --- a/tests/engine/tauri/document-io.test.ts +++ b/tests/engine/tauri/document-io.test.ts @@ -56,6 +56,23 @@ describe('Tauri document IO helpers', () => { expect(savedVersions).toEqual([42]) }) + test('keeps a completed write successful when recovery cleanup fails', async () => { + await mockTauriIPC(() => null) + const savedVersions: number[] = [] + const write = createDocumentWriter({ + state: { sceneVersion: 42 } as Parameters[0]['state'], + getFilePath: () => '/tmp/document.fig', + getFileHandle: () => null, + getStorageBinding: () => null, + setSavedVersion: (version) => savedVersions.push(version), + setLastWriteTime: () => undefined, + onWriteSuccess: () => Promise.reject(new Error('IndexedDB blocked')) + }) + + await expect(write(new Uint8Array([1]), 17)).resolves.toBe(true) + expect(savedVersions).toEqual([17]) + }) + test('chooses a Tauri save path through plugin-dialog', async () => { await mockTauriIPC((cmd, args) => { expect(cmd).toBe('plugin:dialog|save')