diff --git a/frontend/editor/src/core/services/fileStorage.ts b/frontend/editor/src/core/services/fileStorage.ts index b0cede48cb..b2a42c46ed 100644 --- a/frontend/editor/src/core/services/fileStorage.ts +++ b/frontend/editor/src/core/services/fileStorage.ts @@ -724,66 +724,72 @@ class FileStorageService { request.onerror = () => reject(request.error); request.onsuccess = (event) => { - const cursor = (event.target as IDBRequest).result; - if (cursor) { - const record = cursor.value as StoredStirlingFileRecord; - if (record && record.name && typeof record.size === "number") { - const fresh = this.isThumbnailFresh(record); - if ( - record.thumbnail && - maintenanceMayRewrite(record, this.blobValuesSupported) - ) { - if (fresh) tobump.push(record.id); - else toexpire.push(record.id); + try { + const cursor = (event.target as IDBRequest).result; + if (cursor) { + const record = cursor.value as StoredStirlingFileRecord; + if (record && record.name && typeof record.size === "number") { + const fresh = this.isThumbnailFresh(record); + if ( + record.thumbnail && + maintenanceMayRewrite(record, this.blobValuesSupported) + ) { + if (fresh) tobump.push(record.id); + else toexpire.push(record.id); + } + this.reportIfUnreadable(record); + stubs.push({ + id: record.id, + dataUnavailable: + this.unreadableRecords.has(record.id) || undefined, + name: record.name, + type: record.type, + size: record.size, + lastModified: record.lastModified, + quickKey: record.quickKey, + thumbnailUrl: fresh ? record.thumbnail : undefined, + isLeaf: record.isLeaf, + remoteStorageId: record.remoteStorageId, + remoteStorageUpdatedAt: record.remoteStorageUpdatedAt, + remoteOwnerUsername: record.remoteOwnerUsername, + remoteOwnedByCurrentUser: record.remoteOwnedByCurrentUser, + remoteAccessRole: record.remoteAccessRole, + remoteSharedViaLink: record.remoteSharedViaLink, + remoteHasShareLinks: record.remoteHasShareLinks, + remoteShareToken: record.remoteShareToken, + versionNumber: record.versionNumber || 1, + originalFileId: record.originalFileId || record.id, + parentFileId: record.parentFileId, + toolHistory: record.toolHistory || [], + derivedFromTool: + record.derivedFromTool ?? legacyDerivedFromTool(record), + sourceFileIds: record.sourceFileIds, + folderId: record.folderId ?? null, + createdAt: record.createdAt || Date.now(), + classificationLabels: record.classificationLabels, + classificationConfidence: record.classificationConfidence, + }); } - this.reportIfUnreadable(record); - stubs.push({ - id: record.id, - dataUnavailable: - this.unreadableRecords.has(record.id) || undefined, - name: record.name, - type: record.type, - size: record.size, - lastModified: record.lastModified, - quickKey: record.quickKey, - thumbnailUrl: fresh ? record.thumbnail : undefined, - isLeaf: record.isLeaf, - remoteStorageId: record.remoteStorageId, - remoteStorageUpdatedAt: record.remoteStorageUpdatedAt, - remoteOwnerUsername: record.remoteOwnerUsername, - remoteOwnedByCurrentUser: record.remoteOwnedByCurrentUser, - remoteAccessRole: record.remoteAccessRole, - remoteSharedViaLink: record.remoteSharedViaLink, - remoteHasShareLinks: record.remoteHasShareLinks, - remoteShareToken: record.remoteShareToken, - versionNumber: record.versionNumber || 1, - originalFileId: record.originalFileId || record.id, - parentFileId: record.parentFileId, - toolHistory: record.toolHistory || [], - derivedFromTool: - record.derivedFromTool ?? legacyDerivedFromTool(record), - sourceFileIds: record.sourceFileIds, - folderId: record.folderId ?? null, - createdAt: record.createdAt || Date.now(), - classificationLabels: record.classificationLabels, - classificationConfidence: record.classificationConfidence, - }); + cursor.continue(); + } else { + // Only open the writeback transaction when there's something to do - + // previously fired two empty transactions per refresh. + if (tobump.length > 0) { + void this.bumpThumbnailTTL(tobump).catch((e) => + console.warn("[fileStorage] thumbnail TTL bump failed", e), + ); + } + if (toexpire.length > 0) { + void this.bumpThumbnailTTL(toexpire, true).catch((e) => + console.warn("[fileStorage] thumbnail expire failed", e), + ); + } + resolve(stubs); } - cursor.continue(); - } else { - // Only open the writeback transaction when there's something to do - - // previously fired two empty transactions per refresh. - if (tobump.length > 0) { - void this.bumpThumbnailTTL(tobump).catch((e) => - console.warn("[fileStorage] thumbnail TTL bump failed", e), - ); - } - if (toexpire.length > 0) { - void this.bumpThumbnailTTL(toexpire, true).catch((e) => - console.warn("[fileStorage] thumbnail expire failed", e), - ); - } - resolve(stubs); + } catch (error) { + // iOS WebKit throws from openCursor()/continue() once the transaction + // has gone inactive; reject instead of letting it escape uncaught. + reject(error instanceof Error ? error : new Error(String(error))); } }; }); @@ -818,70 +824,76 @@ class FileStorageService { request.onerror = () => reject(request.error); request.onsuccess = (event) => { - const cursor = (event.target as IDBRequest).result; - if (cursor) { - const record = cursor.value as StoredStirlingFileRecord; - // Only include leaf files (default to true if undefined) - if ( - record && - record.name && - typeof record.size === "number" && - record.isLeaf !== false - ) { - const fresh = this.isThumbnailFresh(record); + try { + const cursor = (event.target as IDBRequest).result; + if (cursor) { + const record = cursor.value as StoredStirlingFileRecord; + // Only include leaf files (default to true if undefined) if ( - record.thumbnail && - maintenanceMayRewrite(record, this.blobValuesSupported) + record && + record.name && + typeof record.size === "number" && + record.isLeaf !== false ) { - if (fresh) tobump.push(record.id); - else toexpire.push(record.id); + const fresh = this.isThumbnailFresh(record); + if ( + record.thumbnail && + maintenanceMayRewrite(record, this.blobValuesSupported) + ) { + if (fresh) tobump.push(record.id); + else toexpire.push(record.id); + } + this.reportIfUnreadable(record); + leafStubs.push({ + id: record.id, + dataUnavailable: + this.unreadableRecords.has(record.id) || undefined, + name: record.name, + type: record.type, + size: record.size, + lastModified: record.lastModified, + quickKey: record.quickKey, + thumbnailUrl: fresh ? record.thumbnail : undefined, + isLeaf: record.isLeaf, + remoteStorageId: record.remoteStorageId, + remoteStorageUpdatedAt: record.remoteStorageUpdatedAt, + remoteOwnerUsername: record.remoteOwnerUsername, + remoteOwnedByCurrentUser: record.remoteOwnedByCurrentUser, + remoteAccessRole: record.remoteAccessRole, + remoteSharedViaLink: record.remoteSharedViaLink, + remoteHasShareLinks: record.remoteHasShareLinks, + remoteShareToken: record.remoteShareToken, + versionNumber: record.versionNumber || 1, + originalFileId: record.originalFileId || record.id, + parentFileId: record.parentFileId, + toolHistory: record.toolHistory || [], + derivedFromTool: + record.derivedFromTool ?? legacyDerivedFromTool(record), + sourceFileIds: record.sourceFileIds, + folderId: record.folderId ?? null, + createdAt: record.createdAt || Date.now(), + classificationLabels: record.classificationLabels, + classificationConfidence: record.classificationConfidence, + }); } - this.reportIfUnreadable(record); - leafStubs.push({ - id: record.id, - dataUnavailable: - this.unreadableRecords.has(record.id) || undefined, - name: record.name, - type: record.type, - size: record.size, - lastModified: record.lastModified, - quickKey: record.quickKey, - thumbnailUrl: fresh ? record.thumbnail : undefined, - isLeaf: record.isLeaf, - remoteStorageId: record.remoteStorageId, - remoteStorageUpdatedAt: record.remoteStorageUpdatedAt, - remoteOwnerUsername: record.remoteOwnerUsername, - remoteOwnedByCurrentUser: record.remoteOwnedByCurrentUser, - remoteAccessRole: record.remoteAccessRole, - remoteSharedViaLink: record.remoteSharedViaLink, - remoteHasShareLinks: record.remoteHasShareLinks, - remoteShareToken: record.remoteShareToken, - versionNumber: record.versionNumber || 1, - originalFileId: record.originalFileId || record.id, - parentFileId: record.parentFileId, - toolHistory: record.toolHistory || [], - derivedFromTool: - record.derivedFromTool ?? legacyDerivedFromTool(record), - sourceFileIds: record.sourceFileIds, - folderId: record.folderId ?? null, - createdAt: record.createdAt || Date.now(), - classificationLabels: record.classificationLabels, - classificationConfidence: record.classificationConfidence, - }); + cursor.continue(); + } else { + if (tobump.length > 0) { + void this.bumpThumbnailTTL(tobump).catch((e) => + console.warn("[fileStorage] thumbnail TTL bump failed", e), + ); + } + if (toexpire.length > 0) { + void this.bumpThumbnailTTL(toexpire, true).catch((e) => + console.warn("[fileStorage] thumbnail expire failed", e), + ); + } + resolve(leafStubs); } - cursor.continue(); - } else { - if (tobump.length > 0) { - void this.bumpThumbnailTTL(tobump).catch((e) => - console.warn("[fileStorage] thumbnail TTL bump failed", e), - ); - } - if (toexpire.length > 0) { - void this.bumpThumbnailTTL(toexpire, true).catch((e) => - console.warn("[fileStorage] thumbnail expire failed", e), - ); - } - resolve(leafStubs); + } catch (error) { + // iOS WebKit throws from openCursor()/continue() once the transaction + // has gone inactive; reject instead of letting it escape uncaught. + reject(error instanceof Error ? error : new Error(String(error))); } }; }); diff --git a/frontend/editor/src/core/services/indexedDBManager.migration.test.ts b/frontend/editor/src/core/services/indexedDBManager.migration.test.ts index 0a5a4b9827..b4eaad9d0e 100644 --- a/frontend/editor/src/core/services/indexedDBManager.migration.test.ts +++ b/frontend/editor/src/core/services/indexedDBManager.migration.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test, beforeEach } from "vitest"; +import { describe, expect, test, beforeEach, vi } from "vitest"; import "fake-indexeddb/auto"; import { expectConsole } from "@app/tests/failOnConsole"; @@ -286,6 +286,25 @@ describe("IndexedDB migration (FILES store)", () => { indexedDBManager.closeDatabase(DB_NAME); }); + test("fresh install skips the files-store migration walk", async () => { + // The reported iOS WebKit crash was migrateFilesStore running openCursor() + // on the just-created empty store at oldVersion 0. A new store has nothing + // to migrate, so the walk must not run at all. + const walk = vi.spyOn( + Object.getPrototypeOf(indexedDBManager) as { + migrateFilesStore: (...args: unknown[]) => void; + }, + "migrateFilesStore", + ); + try { + await indexedDBManager.openDatabase(DATABASE_CONFIGS.FILES); + indexedDBManager.closeDatabase(DB_NAME); + expect(walk).not.toHaveBeenCalled(); + } finally { + walk.mockRestore(); + } + }); + test("SaaS v8 -> latest backfills folderId, preserves files, drops orphan stores", async () => { await seedSaasDatabase(8, ["saas-file-a", "saas-file-b"]); await indexedDBManager.openDatabase(DATABASE_CONFIGS.FILES); diff --git a/frontend/editor/src/core/services/indexedDBManager.ts b/frontend/editor/src/core/services/indexedDBManager.ts index dd823a2bd9..4d2217ba75 100644 --- a/frontend/editor/src/core/services/indexedDBManager.ts +++ b/frontend/editor/src/core/services/indexedDBManager.ts @@ -173,9 +173,11 @@ class IndexedDBManager { // Create or update object stores config.stores.forEach((storeConfig) => { let store: IDBObjectStore | undefined; + let storeExisted = false; if (db.objectStoreNames.contains(storeConfig.name)) { // Store exists - get reference for migration + storeExisted = true; console.log(`Object store '${storeConfig.name}' already exists`); store = transaction?.objectStore(storeConfig.name); @@ -218,11 +220,14 @@ class IndexedDBManager { } } - // Perform data migration for files database + // Perform data migration for files database. A store we just + // created is empty, so there is nothing to walk - skipping also + // avoids the openCursor() throw on a brand-new database (oldVersion 0). if ( config.name === "stirling-pdf-files" && storeConfig.name === "files" && - store + store && + storeExisted ) { this.migrateFilesStore(store, oldVersion); } @@ -271,7 +276,17 @@ class IndexedDBManager { private migrateFilesStore(store: IDBObjectStore, oldVersion: number): void { if (oldVersion >= 9) return; // nothing to migrate at the current schema - const cursor = store.openCursor(); + let cursor: IDBRequest; + try { + cursor = store.openCursor(); + } catch (error) { + // iOS WebKit throws "Attempt to open a cursor ... without an in-progress + // transaction" once the versionchange transaction has gone inactive. + // Abort so the open rejects through request.onerror instead of the throw + // escaping onupgradeneeded uncaught. + this.abortMigration(store, "open cursor", error); + return; + } let migrated = 0; cursor.onsuccess = (event) => { @@ -319,35 +334,48 @@ class IndexedDBManager { needsUpdate = true; } - if (needsUpdate) { - try { + try { + if (needsUpdate) { result.update(record); migrated += 1; - } catch (error) { - // Aborting the upgrade transaction here forces IndexedDB to roll back - // the schema version bump too - the user retries on next page load - // instead of silently losing folderId / isLeaf / etc on partial rows. - console.error("Failed to migrate record:", record.id, error); - store.transaction.abort(); - return; } + result.continue(); + } catch (error) { + // update()/continue() throw the same dead-transaction error as + // openCursor above. Aborting rolls back the schema version bump too, so + // the user retries on next page load instead of silently losing + // folderId / isLeaf / etc on partial rows. + this.abortMigration(store, "write record", error); } - result.continue(); }; cursor.onerror = (event) => { - // Same reasoning as the per-record catch above: abort the upgrade so the - // schema doesn't get marked as v9 with rows still on the older shape. - const err = (event.target as IDBRequest).error; - console.error("Files-store migration cursor failed:", err); - try { - store.transaction.abort(); - } catch { - // Already aborted - ignore. - } + this.abortMigration( + store, + "walk cursor", + (event.target as IDBRequest).error, + ); }; } + /** + * Roll back a files-store migration by aborting its upgrade transaction, so + * the schema is not marked at the new version with rows still on the old shape. + * Safe to call after the transaction has already aborted. + */ + private abortMigration( + store: IDBObjectStore, + phase: string, + error: unknown, + ): void { + console.error(`Files-store migration failed to ${phase}:`, error); + try { + store.transaction.abort(); + } catch { + // Already aborted - ignore. + } + } + /** * Get database connection (must be already opened) */