From 78e2d702de32ef2bb68d4e15172e61db87246307 Mon Sep 17 00:00:00 2001 From: bymyself Date: Thu, 15 Jan 2026 22:49:37 -0800 Subject: [PATCH] fix: add LRU eviction for session storage to prevent QuotaExceededError Implement LRU-based session storage service for workflow data to prevent QuotaExceededError that was breaking workspace/teams features on mobile. Changes: - Add sessionStorageLruService with try-evict-retry logic - Store workflow data with embedded accessedAt timestamps for LRU tracking - Evict oldest entries when quota is exceeded (legacy entries first) - Graceful degradation: log warnings instead of throwing on persistent failures - Backward compatibility: legacy unwrapped data treated as oldest Fixes: https://comfy-org.sentry.io/issues/6955016837/ Amp-Thread-ID: https://ampcode.com/threads/T-019bc566-eb91-7545-a462-658c2b33083b Co-authored-by: Amp --- .../composables/useWorkflowPersistence.ts | 87 ++++-- .../services/sessionStorageLruService.test.ts | 238 +++++++++++++++ .../services/sessionStorageLruService.ts | 280 ++++++++++++++++++ 3 files changed, 575 insertions(+), 30 deletions(-) create mode 100644 src/platform/workflow/persistence/services/sessionStorageLruService.test.ts create mode 100644 src/platform/workflow/persistence/services/sessionStorageLruService.ts diff --git a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts index 09ffd5895..732af9ad0 100644 --- a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts +++ b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts @@ -11,11 +11,19 @@ import { useSettingStore } from '@/platform/settings/settingStore' import { useWorkflowService } from '@/platform/workflow/core/services/workflowService' import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' import { useTemplateUrlLoader } from '@/platform/workflow/templates/composables/useTemplateUrlLoader' +import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' import { api } from '@/scripts/api' import { app as comfyApp } from '@/scripts/app' import { getStorageValue, setStorageValue } from '@/scripts/utils' +import { + getStorageStats, + getWithLruTracking, + setWithLruEviction +} from '@/platform/workflow/persistence/services/sessionStorageLruService' import { useCommandStore } from '@/stores/commandStore' +const WORKFLOW_KEY_PATTERN = /^workflow:/ + export function useWorkflowPersistence() { const workflowStore = useWorkflowStore() const settingStore = useSettingStore() @@ -44,39 +52,45 @@ export function useWorkflowPersistence() { const persistCurrentWorkflow = () => { if (!workflowPersistenceEnabled.value) return - const workflow = JSON.stringify(comfyApp.rootGraph.serialize()) + const workflowData = comfyApp.rootGraph.serialize() + const workflowJson = JSON.stringify(workflowData) try { - localStorage.setItem('workflow', workflow) - if (api.clientId) { - sessionStorage.setItem(`workflow:${api.clientId}`, workflow) - } + localStorage.setItem('workflow', workflowJson) } catch (error) { - // Only log our own keys and aggregate stats - const ourKeys = Object.keys(sessionStorage).filter( - (key) => key.startsWith('workflow:') || key === 'workflow' + console.warn( + '[WorkflowPersistence] Failed to save to localStorage:', + error ) - console.error('QuotaExceededError details:', { - workflowSizeKB: Math.round(workflow.length / 1024), - totalStorageItems: Object.keys(sessionStorage).length, - ourWorkflowKeys: ourKeys.length, - ourWorkflowSizes: ourKeys.map((key) => ({ - key, - sizeKB: Math.round(sessionStorage[key].length / 1024) - })), - error: error instanceof Error ? error.message : String(error) - }) - throw error + } + + if (api.clientId) { + const key = `workflow:${api.clientId}` + const success = setWithLruEviction( + key, + workflowData, + WORKFLOW_KEY_PATTERN + ) + + if (!success) { + const stats = getStorageStats(WORKFLOW_KEY_PATTERN) + console.warn( + '[WorkflowPersistence] Failed to persist workflow after LRU eviction', + { + workflowSizeKB: Math.round(workflowJson.length / 1024), + ...stats + } + ) + } } } - const loadWorkflowFromStorage = async ( - json: string | null, + const loadWorkflowFromData = async ( + workflowData: ComfyWorkflowJSON | null, workflowName: string | null ) => { - if (!json) return false - const workflow = JSON.parse(json) - await comfyApp.loadGraphData(workflow, true, true, workflowName) + if (!workflowData) return false + await comfyApp.loadGraphData(workflowData, true, true, workflowName) return true } @@ -84,17 +98,30 @@ export function useWorkflowPersistence() { const workflowName = getStorageValue('Comfy.PreviousWorkflow') const clientId = api.initialClientId ?? api.clientId - // Try loading from session storage first + // Try loading from session storage first (uses LRU tracking) if (clientId) { - const sessionWorkflow = sessionStorage.getItem(`workflow:${clientId}`) - if (await loadWorkflowFromStorage(sessionWorkflow, workflowName)) { + const sessionWorkflow = getWithLruTracking( + `workflow:${clientId}` + ) + if (await loadWorkflowFromData(sessionWorkflow, workflowName)) { return true } } - // Fall back to local storage - const localWorkflow = localStorage.getItem('workflow') - return await loadWorkflowFromStorage(localWorkflow, workflowName) + // Fall back to local storage (raw JSON, no LRU wrapper) + const localWorkflowJson = localStorage.getItem('workflow') + if (localWorkflowJson) { + try { + const localWorkflow = JSON.parse(localWorkflowJson) as ComfyWorkflowJSON + return await loadWorkflowFromData(localWorkflow, workflowName) + } catch { + console.warn( + '[WorkflowPersistence] Failed to parse localStorage workflow' + ) + } + } + + return false } const loadDefaultWorkflow = async () => { diff --git a/src/platform/workflow/persistence/services/sessionStorageLruService.test.ts b/src/platform/workflow/persistence/services/sessionStorageLruService.test.ts new file mode 100644 index 000000000..e74b182cf --- /dev/null +++ b/src/platform/workflow/persistence/services/sessionStorageLruService.test.ts @@ -0,0 +1,238 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { + getStorageStats, + getWithLruTracking, + removeFromStorage, + setWithLruEviction +} from './sessionStorageLruService' + +describe('sessionStorageLruService', () => { + beforeEach(() => { + sessionStorage.clear() + vi.clearAllMocks() + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + describe('setWithLruEviction', () => { + it('stores data with LRU metadata wrapper', () => { + const data = { nodes: [], links: [] } + const result = setWithLruEviction('workflow:test', data) + + expect(result).toBe(true) + + const stored = sessionStorage.getItem('workflow:test') + expect(stored).toBeTruthy() + + const parsed = JSON.parse(stored!) + expect(parsed).toHaveProperty('accessedAt') + expect(parsed).toHaveProperty('data') + expect(parsed.data).toEqual(data) + expect(typeof parsed.accessedAt).toBe('number') + }) + + it('returns true on successful storage', () => { + const result = setWithLruEviction('key', { test: 'data' }) + expect(result).toBe(true) + }) + + it('evicts LRU entries when quota is exceeded', () => { + const oldEntry = { accessedAt: 1000, data: { old: 'data' } } + const newEntry = { accessedAt: 2000, data: { new: 'data' } } + + sessionStorage.setItem('workflow:old', JSON.stringify(oldEntry)) + sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) + + let callCount = 0 + const originalSetItem = sessionStorage.setItem.bind(sessionStorage) + vi.spyOn(sessionStorage, 'setItem').mockImplementation((key, value) => { + callCount++ + if (key === 'workflow:current' && callCount === 1) { + const error = new DOMException('Quota exceeded', 'QuotaExceededError') + throw error + } + return originalSetItem(key, value) + }) + + const result = setWithLruEviction( + 'workflow:current', + { current: 'data' }, + /^workflow:/ + ) + + expect(result).toBe(true) + expect(sessionStorage.getItem('workflow:old')).toBeNull() + expect(sessionStorage.getItem('workflow:new')).toBeTruthy() + }) + + it('returns false after max eviction attempts', () => { + const spy = vi.spyOn(sessionStorage, 'setItem').mockImplementation(() => { + throw new DOMException('Quota exceeded', 'QuotaExceededError') + }) + + const result = setWithLruEviction('key', { test: 'data' }) + expect(result).toBe(false) + + spy.mockRestore() + }) + }) + + describe('getWithLruTracking', () => { + it('returns null for non-existent key', () => { + const result = getWithLruTracking('nonexistent') + expect(result).toBeNull() + }) + + it('returns unwrapped data for new format entries', () => { + const entry = { accessedAt: Date.now(), data: { nodes: [], links: [] } } + sessionStorage.setItem('workflow:test', JSON.stringify(entry)) + + const result = getWithLruTracking('workflow:test') + expect(result).toEqual({ nodes: [], links: [] }) + }) + + it('handles legacy format (unwrapped data) with accessedAt: 0', () => { + const legacyData = { nodes: [1, 2, 3], links: [] } + sessionStorage.setItem('workflow:legacy', JSON.stringify(legacyData)) + + const result = getWithLruTracking('workflow:legacy') + expect(result).toEqual(legacyData) + }) + + it('updates accessedAt when reading', () => { + const oldTime = 1000 + const entry = { accessedAt: oldTime, data: { test: 'data' } } + sessionStorage.setItem('workflow:test', JSON.stringify(entry)) + + getWithLruTracking('workflow:test', true) + + const stored = JSON.parse(sessionStorage.getItem('workflow:test')!) + expect(stored.accessedAt).toBeGreaterThan(oldTime) + }) + + it('does not update accessedAt when updateAccessTime is false', () => { + const oldTime = 1000 + const entry = { accessedAt: oldTime, data: { test: 'data' } } + sessionStorage.setItem('workflow:test', JSON.stringify(entry)) + + getWithLruTracking('workflow:test', false) + + const stored = JSON.parse(sessionStorage.getItem('workflow:test')!) + expect(stored.accessedAt).toBe(oldTime) + }) + }) + + describe('removeFromStorage', () => { + it('removes item from session storage', () => { + sessionStorage.setItem('key', 'value') + expect(sessionStorage.getItem('key')).toBe('value') + + removeFromStorage('key') + expect(sessionStorage.getItem('key')).toBeNull() + }) + }) + + describe('getStorageStats', () => { + it('returns stats for all entries', () => { + const entry1 = { accessedAt: 1000, data: { a: 1 } } + const entry2 = { accessedAt: 2000, data: { b: 2 } } + sessionStorage.setItem('workflow:a', JSON.stringify(entry1)) + sessionStorage.setItem('workflow:b', JSON.stringify(entry2)) + sessionStorage.setItem('other:c', 'value') + + const stats = getStorageStats() + + expect(stats.totalItems).toBe(3) + expect(stats.matchingItems).toBe(3) + }) + + it('filters entries by pattern', () => { + const entry1 = { accessedAt: 1000, data: { a: 1 } } + const entry2 = { accessedAt: 2000, data: { b: 2 } } + sessionStorage.setItem('workflow:a', JSON.stringify(entry1)) + sessionStorage.setItem('workflow:b', JSON.stringify(entry2)) + sessionStorage.setItem('other:c', 'value') + + const stats = getStorageStats(/^workflow:/) + + expect(stats.totalItems).toBe(3) + expect(stats.matchingItems).toBe(2) + expect(stats.entries).toHaveLength(2) + }) + + it('sorts entries by accessedAt (oldest first)', () => { + const entry1 = { accessedAt: 2000, data: { newer: true } } + const entry2 = { accessedAt: 1000, data: { older: true } } + sessionStorage.setItem('workflow:newer', JSON.stringify(entry1)) + sessionStorage.setItem('workflow:older', JSON.stringify(entry2)) + + const stats = getStorageStats(/^workflow:/) + + expect(stats.entries[0].key).toBe('workflow:older') + expect(stats.entries[1].key).toBe('workflow:newer') + }) + + it('treats legacy entries as accessedAt: 0', () => { + const legacyData = { nodes: [], links: [] } + const newEntry = { accessedAt: 1000, data: { test: true } } + sessionStorage.setItem('workflow:legacy', JSON.stringify(legacyData)) + sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) + + const stats = getStorageStats(/^workflow:/) + + expect(stats.entries[0].key).toBe('workflow:legacy') + expect(stats.entries[0].accessedAt).toBe(0) + }) + }) + + describe('LRU eviction order', () => { + it('evicts oldest entries first (legacy before new)', () => { + const legacyData = { nodes: [], links: [] } + const newEntry = { accessedAt: Date.now(), data: { new: true } } + + sessionStorage.setItem('workflow:legacy', JSON.stringify(legacyData)) + sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) + + let callCount = 0 + const originalSetItem = sessionStorage.setItem.bind(sessionStorage) + vi.spyOn(sessionStorage, 'setItem').mockImplementation((key, value) => { + callCount++ + if (key === 'workflow:current' && callCount === 1) { + throw new DOMException('Quota exceeded', 'QuotaExceededError') + } + return originalSetItem(key, value) + }) + + setWithLruEviction('workflow:current', { current: true }, /^workflow:/) + + expect(sessionStorage.getItem('workflow:legacy')).toBeNull() + expect(sessionStorage.getItem('workflow:new')).toBeTruthy() + }) + + it('evicts oldest new-format entries when no legacy entries exist', () => { + const oldEntry = { accessedAt: 1000, data: { old: true } } + const newEntry = { accessedAt: 2000, data: { new: true } } + + sessionStorage.setItem('workflow:old', JSON.stringify(oldEntry)) + sessionStorage.setItem('workflow:new', JSON.stringify(newEntry)) + + let callCount = 0 + const originalSetItem = sessionStorage.setItem.bind(sessionStorage) + vi.spyOn(sessionStorage, 'setItem').mockImplementation((key, value) => { + callCount++ + if (key === 'workflow:current' && callCount === 1) { + throw new DOMException('Quota exceeded', 'QuotaExceededError') + } + return originalSetItem(key, value) + }) + + setWithLruEviction('workflow:current', { current: true }, /^workflow:/) + + expect(sessionStorage.getItem('workflow:old')).toBeNull() + expect(sessionStorage.getItem('workflow:new')).toBeTruthy() + }) + }) +}) diff --git a/src/platform/workflow/persistence/services/sessionStorageLruService.ts b/src/platform/workflow/persistence/services/sessionStorageLruService.ts new file mode 100644 index 000000000..aae6e3f0f --- /dev/null +++ b/src/platform/workflow/persistence/services/sessionStorageLruService.ts @@ -0,0 +1,280 @@ +/** + * @fileoverview LRU-based session storage service with automatic eviction + * @module services/sessionStorageLruService + * + * Provides session storage operations with: + * - LRU tracking via embedded timestamps + * - Automatic eviction on quota exceeded errors + * - Graceful degradation when storage is unavailable + * - Backward compatibility with legacy (unwrapped) data + * + * @deprecated-notice Legacy format support (unwrapped data) can be removed after 2026-07-15 + */ + +interface StorageEntry { + accessedAt: number + data: T +} + +interface EvictableEntry { + key: string + accessedAt: number + size: number +} + +const MAX_EVICTION_ATTEMPTS = 3 +const PROTECTED_KEY_PREFIXES = ['workspace.', 'Workspace.'] + +function isProtectedKey(key: string): boolean { + return PROTECTED_KEY_PREFIXES.some((prefix) => key.includes(prefix)) +} + +/** + * Checks if parsed data is in legacy format (raw data without wrapper) + * Legacy format: raw workflow JSON with nodes/links at top level + * New format: { accessedAt: number, data: T } + * + * @deprecated Remove after 2026-07-15 + */ +function isLegacyFormat(parsed: unknown): boolean { + if (typeof parsed !== 'object' || parsed === null) return true + const obj = parsed as Record + return !('accessedAt' in obj && 'data' in obj) +} + +/** + * Wraps data with LRU metadata for storage + */ +function wrapForStorage(data: T): string { + const entry: StorageEntry = { + accessedAt: Date.now(), + data + } + return JSON.stringify(entry) +} + +/** + * Unwraps stored data, handling both legacy and new formats + * Legacy entries are assigned accessedAt: 0 to prioritize them for eviction + * + * @deprecated Legacy handling can be removed after 2026-07-15 + */ +function unwrapFromStorage(raw: string): StorageEntry { + const parsed = JSON.parse(raw) + + if (isLegacyFormat(parsed)) { + return { + accessedAt: 0, + data: parsed as T + } + } + + return parsed as StorageEntry +} + +/** + * Gets all evictable entries from session storage matching a key pattern + */ +function getEvictableEntries(keyPattern: RegExp): EvictableEntry[] { + const entries: EvictableEntry[] = [] + + for (let i = 0; i < sessionStorage.length; i++) { + const key = sessionStorage.key(i) + if (!key || !keyPattern.test(key) || isProtectedKey(key)) continue + + const raw = sessionStorage.getItem(key) + if (!raw) continue + + try { + const entry = unwrapFromStorage(raw) + entries.push({ + key, + accessedAt: entry.accessedAt, + size: raw.length + }) + } catch { + entries.push({ + key, + accessedAt: 0, + size: raw.length + }) + } + } + + return entries +} + +/** + * Evicts the least recently used entries matching a pattern + * @returns Number of bytes freed + */ +function evictLruEntries( + keyPattern: RegExp, + excludeKey?: string, + maxToEvict: number = 1 +): number { + const entries = getEvictableEntries(keyPattern) + .filter((e) => e.key !== excludeKey) + .sort((a, b) => a.accessedAt - b.accessedAt) + + let freedBytes = 0 + const toEvict = entries.slice(0, maxToEvict) + + for (const entry of toEvict) { + sessionStorage.removeItem(entry.key) + freedBytes += entry.size + } + + if (toEvict.length > 0) { + console.warn( + `[SessionStorageLRU] Evicted ${toEvict.length} entries, freed ~${Math.round(freedBytes / 1024)}KB`, + toEvict.map((e) => e.key) + ) + } + + return freedBytes +} + +/** + * Attempts to set a session storage item with LRU eviction on quota exceeded + * + * @param key - Storage key + * @param data - Data to store (will be wrapped with LRU metadata) + * @param evictionPattern - Regex pattern for keys eligible for eviction + * @returns true if stored successfully, false if storage failed after retries + */ +export function setWithLruEviction( + key: string, + data: T, + evictionPattern: RegExp = /^workflow:/ +): boolean { + const wrapped = wrapForStorage(data) + + for (let attempt = 0; attempt <= MAX_EVICTION_ATTEMPTS; attempt++) { + try { + sessionStorage.setItem(key, wrapped) + return true + } catch (error) { + if ( + !(error instanceof DOMException) || + error.name !== 'QuotaExceededError' + ) { + console.error('[SessionStorageLRU] Unexpected storage error:', error) + return false + } + + if (attempt === MAX_EVICTION_ATTEMPTS) { + console.warn( + `[SessionStorageLRU] Failed to store ${key} after ${MAX_EVICTION_ATTEMPTS} eviction attempts` + ) + return false + } + + const entriesToEvict = Math.min(attempt + 1, 3) + const freedBytes = evictLruEntries(evictionPattern, key, entriesToEvict) + + if (freedBytes === 0) { + console.warn( + '[SessionStorageLRU] No entries available for eviction, giving up' + ) + return false + } + } + } + + return false +} + +/** + * Gets data from session storage, updating access time for LRU tracking + * + * @param key - Storage key + * @param updateAccessTime - Whether to update the access timestamp (default: true) + * @returns The stored data, or null if not found + */ +export function getWithLruTracking( + key: string, + updateAccessTime: boolean = true +): T | null { + const raw = sessionStorage.getItem(key) + if (!raw) return null + + try { + const entry = unwrapFromStorage(raw) + + if (updateAccessTime && entry.accessedAt !== Date.now()) { + try { + sessionStorage.setItem(key, wrapForStorage(entry.data)) + } catch { + // Ignore quota errors when updating access time + } + } + + return entry.data + } catch (error) { + console.warn(`[SessionStorageLRU] Failed to parse ${key}:`, error) + return null + } +} + +/** + * Removes an item from session storage + */ +export function removeFromStorage(key: string): void { + sessionStorage.removeItem(key) +} + +/** + * Gets storage statistics for debugging + */ +export function getStorageStats(keyPattern?: RegExp): { + totalItems: number + matchingItems: number + totalSizeKB: number + matchingSizeKB: number + entries: Array<{ key: string; sizeKB: number; accessedAt: number }> +} { + let totalSize = 0 + let matchingSize = 0 + let matchingItems = 0 + const entries: Array<{ key: string; sizeKB: number; accessedAt: number }> = [] + + for (let i = 0; i < sessionStorage.length; i++) { + const key = sessionStorage.key(i) + if (!key) continue + + const raw = sessionStorage.getItem(key) + if (!raw) continue + + const size = raw.length * 2 + totalSize += size + + if (!keyPattern || keyPattern.test(key)) { + matchingItems++ + matchingSize += size + + try { + const entry = unwrapFromStorage(raw) + entries.push({ + key, + sizeKB: Math.round(size / 1024), + accessedAt: entry.accessedAt + }) + } catch { + entries.push({ + key, + sizeKB: Math.round(size / 1024), + accessedAt: 0 + }) + } + } + } + + return { + totalItems: sessionStorage.length, + matchingItems, + totalSizeKB: Math.round(totalSize / 1024), + matchingSizeKB: Math.round(matchingSize / 1024), + entries: entries.sort((a, b) => a.accessedAt - b.accessedAt) + } +}