mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-02 21:03:34 +03:00
fix(frontend): guard IndexedDB cursor walks against a dead transaction
On iOS WebKit, migrateFilesStore ran an openCursor() walk inside onupgradeneeded for every oldVersion below 9 - including a brand-new empty database at oldVersion 0. WebKit throws "Attempt to open a cursor in database without an in-progress transaction" once the versionchange transaction goes inactive, and the throw escaped the handler uncaught, so the stirling-pdf-files database never opened and file persistence stopped for that session. Skip the walk when the files store was just created (nothing to migrate), and wrap openCursor(), update(), and continue() so a dead transaction aborts the upgrade cleanly instead of throwing into the void. Apply the same guard to the two cursor walks in fileStorage so they reject their promise rather than escape. Generated-By: PostHog Desktop Task-Id: 4ca1aa52-9e02-4641-8e7e-2f906e36b78b
This commit is contained in:
@@ -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)));
|
||||
}
|
||||
};
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<IDBCursorWithValue | null>;
|
||||
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)
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user