[web-console] Remember support bundles the user opened, in IndexedDB - #6971
[web-console] Remember support bundles the user opened, in IndexedDB#6971Karakatiza666 wants to merge 1 commit into
Conversation
A support bundle opened from disk is now remembered, so the user can reopen it without hunting for the file again. IndexedDB holds the history because neither of the two things an entry can carry survives JSON: both a FileSystemFileHandle and a File are structured-cloneable but not serializable, so localStorage can hold neither. Which one an entry carries follows the browser: Browser Entry holds Cost Re-read needs Chromium (file picker) handle a few hundred B a re-grant Firefox, Safari (input) file the whole archive nothing A handle is preferred wherever it exists, since it costs the same few hundred bytes whatever the size of the archive behind it. Browsers with no File System Access API hand out only a File, which cannot be re-opened from its path, so the history caches the archive itself. Everything specific to those copies lives in supportBundleCache, which the history module does not depend on. Copies are bounded twice: 256 MB per archive, 512 MB across all of them. A bundle past either limit still opens, it just leaves no history entry, which beats filling the origin's storage quota with one archive. Both modules cap the history at 30 entries. Entries are readable from any tab of this origin, which is what lets one tab link another to a bundle. Nothing consumes the history yet. Tests cover both modules against a real IndexedDB: the handle round trip, the identity tests that keep a re-picked file from duplicating its entry, both limits, and the permission queries. `cachedBundlesOverBudget` is exported so the budget can be exercised without storing half a gigabyte.
| /** | ||
| * Whether remembering a bundle means caching the archive, because this browser hands | ||
| * out no file handles. | ||
| */ | ||
| export const isBundleCacheRequired = () => !isBundlePickerSupported() |
There was a problem hiding this comment.
isBundlePickerSupported() folds two independent capabilities into one answer (showOpenFilePicker and indexedDB), and this negation then reads the IndexedDB half backwards.
I confirmed it in a browser test: with indexedDB deleted and no picker, isBundlePickerSupported() === false and isBundleCacheRequired() === true — so a browser with no IndexedDB is told to cache into IndexedDB, and every call below throws ReferenceError on indexedDB.open. Same for a Chromium with site data blocked, where indexedDB.open throws SecurityError.
Suggest splitting the check: isBundlePickerSupported() should ask only about the picker, and a separate isHistorySupported() should ask about IndexedDB, with isBundleCacheRequired() = isHistorySupported() && !isBundlePickerSupported().
| const finished = new Promise<void>((resolve, reject) => { | ||
| transaction.oncomplete = () => resolve() | ||
| transaction.onerror = () => reject(transaction.error ?? new Error('IndexedDB write failed')) | ||
| transaction.onabort = () => reject(transaction.error ?? new Error('IndexedDB write aborted')) | ||
| }) | ||
| const result = await use(transaction.objectStore(STORE_NAME)) | ||
| await finished | ||
| return result |
There was a problem hiding this comment.
When use rejects, await use(...) throws and await finished is never reached — but finished already has a reject wired to onerror/onabort, so it rejects with nothing attached. That is an unhandled promise rejection on top of the real error.
Verified with a browser probe that mirrors this shape (a failing request that also aborts the transaction): the caller's promise rejects and window.onunhandledrejection fires with Error: write failed.
Reachable here: a store.put of a cached archive hitting QuotaExceededError errors the request and aborts the transaction. Attaching a no-op catch — finished.catch(() => {}) right after construction, or await Promise.all([use(...), finished]) — closes it.
| const existing = await findSameEntry(handle) | ||
| const stored = await putSupportBundle(existing?.id, { | ||
| name: handle.name, |
There was a problem hiding this comment.
Read-then-write across two transactions, so the dedup is a TOCTOU. The module header advertises cross-tab use ("readable from any tab of this origin"), which is exactly the case where the two callers interleave.
Confirmed in a browser test:
await Promise.all([
rememberSupportBundle(fakeHandle('same.zip')),
rememberSupportBundle(fakeHandle('same.zip'))
])
// → ["same.zip", "same.zip"]Both reads see an empty history, both put with no id, the key generator hands out two ids. rememberSupportBundleFile has the same shape. One readwrite transaction that does the getAll and the put (with findSameEntry's comparison inlined) would hold; if isSameEntry's await makes that impossible, a serializing promise chain in the module would at least cover the single-tab case. Either way, no test covers concurrent calls today.
| export const rememberSupportBundleFile = async ( | ||
| file: File | ||
| ): Promise<StoredSupportBundle | null> => { | ||
| if (file.size > maxCachedBundleBytes) { | ||
| return null | ||
| } |
There was a problem hiding this comment.
The contract is "too big to cache → null, the bundle still opens", but the browser's own quota is a third limit that is not handled: a 256 MB archive that overruns the origin quota makes putSupportBundle reject, so the caller in the next PR loses the bundle rather than just the history entry. Wrapping the store in try/catch and returning null on QuotaExceededError would make the two limits behave the same way. Worth a test too — no test drives a failing write.
| /** | ||
| * The cached copies that exceed the byte budget, oldest first. Entries holding a | ||
| * handle weigh nothing, so they are passed over. |
There was a problem hiding this comment.
Nit: "oldest first" is the reverse of what comes back. The loop walks mostRecentFirst, so excess is newest-first too. Deletion does not care, but the test named "drops the oldest copies once they outgrow the budget" only passes because its expectation has one element — it would not catch the order the doc claims.
| export const resolveStoredBundle = async ( | ||
| id: number | undefined | ||
| ): Promise<{ bundle: StoredSupportBundle; needsPermission: boolean }> => { | ||
| const bundle = id ? await getSupportBundle(id) : undefined |
There was a problem hiding this comment.
Nit: id ? treats 0 as absent. IndexedDB's key generator starts at 1, so nothing produces it today, but id !== undefined says what is meant and does not depend on that.
|
Nice module: the two-kinds-of-entry split is well chosen, the comments explain why rather than what, and the commit message is a model of the form. Ran locally in Three findings I reproduced in the browser (details inline): Gates:
Uncovered cases beyond the inline ones: |
A support bundle opened from disk is now remembered, so the user can reopen it without selecting the file in the file picker again. There are two modes of history, depending on the API, available in the browser: either a reference (handle) to file is stored, or the file itself is cached. The former is more beneficial because it consumes very little browser storage capacity (IndexedDB), but not all browsers provide this handle API.
For caching the entire support bundle, reasonable limits of 256 MB per support bundle and 512 MB total are applied - when the bundles no longer fit or are too large they are removed from history.
Describe Manual Test Plan
Automated only, and sufficient here: this PR adds no UI, so there is nothing to drive by hand.
bun run test-unit— 76 files, 681 tests pass.bun run check— 0 errors.cachedBundlesOverBudgetis exported so the budget can be exercised without storing half a gigabyte.Checklist
Breaking Changes?
Mark if you think the answer is yes for any of these components:
Describe Incompatible Changes
None. New browser-local storage; no existing data is read or migrated.