From 9dbe246d2043ed1563f78274b77792ec0ec10d08 Mon Sep 17 00:00:00 2001 From: Alicia Wilkerson Date: Mon, 31 Aug 2026 14:58:47 -0500 Subject: [PATCH] feat(storage): bound stalled IndexedDB transactions with an opt-in guard (DAB-1177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A stalled IndexedDB transaction fires no event at all — not complete, not error, not abort — so `requestToPromise` never settles and its caller waits forever with nothing dispatched and nothing to recover from. `open()` has been guarded against exactly this since 3.1.0; the transaction path never was. Add the same shape to `requestToPromise`, the single choke point every promise-returning operation flows through, including the get/getAll/count reads that carry no transaction promise to fall back on: - `slowTransactionTimeout` reports a long-running operation via `onSlowStorage` and leaves it completely alone, so consumers can measure what healthy storage costs before choosing a deadline. - `transactionTimeout` aborts and rejects with a `StorageTimeoutError`, the name @dabble/patches gives the same condition so a consumer watching both storage paths classifies them alike. Both default to 0 (disabled), so this release changes no existing behavior. Ported from the patches guard: a timer that wakes to find far more wall-clock elapsed than it asked for measured a suspended tab rather than the connection, so it re-arms once; and a connection-wide `lastSettleAt` lets sibling work vouch for a slow transaction, bounded at 30 windows so live traffic cannot postpone a genuine stall forever. Aborting a healthy-but-queued transaction is its own bug, and that defer window is what prevents it. The database name is kept out of the error message and carried as a property: names routinely embed a user id, which would give every user a private error group in aggregators and make the class uncountable. Co-Authored-By: Claude Opus 5 --- .gitignore | 2 + src/Browserbase.ts | 281 +++++++++++++++++++++++++++++++++++++- src/storageGuard.spec.ts | 282 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 558 insertions(+), 7 deletions(-) create mode 100644 .gitignore create mode 100644 src/storageGuard.spec.ts diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..b947077 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +node_modules/ +dist/ diff --git a/src/Browserbase.ts b/src/Browserbase.ts index b8e96ad..39cb334 100644 --- a/src/Browserbase.ts +++ b/src/Browserbase.ts @@ -55,6 +55,38 @@ export type ObjectStoreMap>> = interface ErrorDispatcher { dispatchError: (err: Error) => void; + /** + * Per-connection state the storage stall guard reads. Transaction clones share their parent's, + * so work on one connection can vouch for other work on the same connection. + */ + storageContext: StorageContext; +} + +/** + * Shared per-connection state for the storage stall guard. + */ +export interface StorageContext { + /** Database name. Reported alongside a stall, never embedded in the error message — see `storageTimeoutError`. */ + dbName: string; + /** When anything on this connection last settled, used to tell a busy connection from a wedged one. */ + lastSettleAt: number; +} + +/** + * Reported when an operation is still unsettled after `Browserbase.slowTransactionTimeout`. + */ +export interface SlowStorageDetail { + dbName: string; + /** Stores involved, best-effort — cosmetic, and empty when the handle can no longer say. */ + storeNames: string[]; + /** Wall-clock milliseconds since the operation started. */ + elapsedMs: number; + /** + * The timer fired far later than its budget, so the tab was suspended for most of `elapsedMs` + * and the figure is mostly frozen time rather than active work. Callers reporting durations + * should keep this alongside them, or the distribution is unreadable. + */ + lateFire: boolean; } interface BrowserbaseConstructor { @@ -125,6 +157,36 @@ export class Browserbase = {}> extends Typ */ static upgradeTimeout = 30000; + /** + * Milliseconds a transaction may go unsettled before `onSlowStorage` reports it. Observation + * only: the operation is left running and its promise is untouched, so this is safe to enable + * on its own to measure what healthy storage actually costs. + * + * 0 disables it, which is the default — a library release must not start reporting on its own. + */ + static slowTransactionTimeout = 0; + + /** + * Milliseconds a transaction may go unsettled before it is abandoned: the transaction is + * aborted and its promise rejects with a `StorageTimeoutError`. + * + * A stalled IndexedDB transaction fires no event at all — not `complete`, not `error`, not + * `abort` — so without a deadline its promise never settles and the caller waits forever. + * That is the wedge this bounds, mirroring the guard `open()` already has. + * + * 0 disables it, which is the default. Enable it only against a measured distribution of + * healthy durations: a threshold below what legitimate bulk work costs converts working + * writes into failures, and a queued-but-healthy transaction must never be aborted for + * merely waiting its turn. + */ + static transactionTimeout = 0; + + /** + * Where `slowTransactionTimeout` reports go. Replace to route them at telemetry; set to null + * to drop them. + */ + static onSlowStorage: ((detail: SlowStorageDetail) => void) | null = warnSlowStorage; + /** * Deletes a database by name. */ @@ -149,6 +211,17 @@ export class Browserbase = {}> extends Typ _channel: BroadcastChannel | null; _opening?: Promise; _closed?: boolean = true; + _storageContext?: StorageContext; + + /** + * Per-connection state for the storage stall guard. `start()` clones the Browserbase to scope a + * transaction, so a clone defers to its parent: every transaction on one connection shares one + * record, which is what lets a settling transaction vouch for a slow sibling. + */ + get storageContext(): StorageContext { + if (this._parent) return this._parent.storageContext; + return (this._storageContext ??= { dbName: this.name, lastSettleAt: Date.now() }); + } /** * Creates a new indexeddb database with the given name. @@ -466,6 +539,10 @@ export class ObjectStore extends T this.db.dispatchError(error); } + get storageContext(): StorageContext { + return this.db.storageContext; + } + /** * Get an object from the store by its primary key */ @@ -613,6 +690,10 @@ export class Where { this.store.dispatchError(error); } + get storageContext(): StorageContext { + return this.store.storageContext; + } + /** * Set greater than the value provided. */ @@ -880,29 +961,57 @@ function requestToPromise( errorDispatcher?: ErrorDispatcher ): Promise { return new Promise((resolve, reject) => { + const context: StorageContext = errorDispatcher?.storageContext ?? { dbName: '', lastSettleAt: Date.now() }; + let guard: { clear(): void } | null = null; + + // Every settle stamps the connection. It is the only evidence that tells a busy connection + // apart from a wedged one, and the defer window in `armStorageGuard` is built on it. + const settle = + (fn: (value: A) => void) => + (value: A) => { + context.lastSettleAt = Date.now(); + guard?.clear(); + fn(value); + }; + const settleResolve = settle(resolve); + const settleReject = settle(reject); + if (transaction) { let promise = transactionPromise.get(transaction); if (!promise) { promise = requestToPromise(transaction, null, errorDispatcher); } promise = promise.then( - () => resolve(request.result), + () => settleResolve(request.result), err => { let requestError; try { requestError = request.error; } catch (e) { } - reject(requestError || err); + settleReject(requestError || err); return Promise.reject(err); } ); transactionPromise.set(transaction, promise); - } else if (request.onsuccess === null) { - request.onsuccess = successHandler(resolve); + } else { + // Only the transaction-less shape owns a deadline. With a transaction this promise is + // chained onto that transaction's own promise, which was armed when the transaction was + // created — arming again here would put two deadlines on one stall. + guard = armStorageGuard(request, context, (budgetMs, elapsedMs, lateFire) => { + const error = storageTimeoutError(request, context, budgetMs, elapsedMs, lateFire); + abortTarget(request); + settleReject(error); + // Dispatch like any other storage failure. Without this a stall is completely silent — + // no event fires, so nothing watching the connection can react to it at all. + errorDispatcher?.dispatchError(error); + }); + if (request.onsuccess === null) { + request.onsuccess = successHandler(settleResolve); + } } - if (request.oncomplete === null) request.oncomplete = successHandler(resolve); - if (request.onerror === null) request.onerror = errorHandler(reject, errorDispatcher); - if (request.onabort === null) request.onabort = () => reject(new Error('Abort')); + if (request.oncomplete === null) request.oncomplete = successHandler(settleResolve); + if (request.onerror === null) request.onerror = errorHandler(settleReject, errorDispatcher); + if (request.onabort === null) request.onabort = () => settleReject(new Error('Abort')); }); } @@ -912,6 +1021,164 @@ function namedError(name: string, message: string) { return error; } +/** + * How much later than its budget a timer may fire before the delay is read as tab suspension + * rather than a stall. A backgrounded tab freezes timers, so a guard that wakes to find far more + * wall-clock elapsed than it asked for has learnt nothing about the connection: it re-arms once + * and gives the operation a fair window while the page is actually awake. + */ +const LATE_FIRE_FACTOR = 2; + +/** + * How many times a guard will defer to a demonstrably-alive connection before firing anyway. + * Bounded so a connection that keeps other work moving cannot postpone a genuine stall forever. + */ +const MAX_DEFER_WINDOWS = 30; + +function warnSlowStorage({ storeNames, elapsedMs }: SlowStorageDetail) { + const label = storeNames.length ? ` [${storeNames.join(', ')}]` : ''; + console.warn(`IndexedDB transaction${label} still unsettled after ${elapsedMs}ms`); +} + +/** + * Best-effort store names for a transaction or a request. Cosmetic only — a handle the browser + * has torn down can throw on property access, and a nameless report beats a thrown guard. + */ +function storeNamesOf(target: any): string[] { + try { + if (target?.objectStoreNames) return Array.from(target.objectStoreNames as DOMStringList); + const source = target?.source; + if (source?.objectStore?.name) return [source.objectStore.name]; + if (source?.name) return [source.name]; + if (target?.transaction?.objectStoreNames) return Array.from(target.transaction.objectStoreNames as DOMStringList); + } catch (e) { + // Fall through to no names. + } + return []; +} + +/** + * Abandon a stalled transaction. Without this the transaction stays live and may still commit + * long after its caller gave up and moved on, which is worse than the stall. + */ +function abortTarget(target: any) { + try { + const trans = typeof target?.abort === 'function' ? target : target?.transaction; + if (trans && typeof trans.abort === 'function') trans.abort(); + } catch (e) { + // Already finishing, or the handle is gone; either way there is nothing left to abort. + } +} + +/** + * The error a stalled operation rejects with. `StorageTimeoutError` is the name @dabble/patches + * gives the same condition, so a consumer watching both storage paths classifies them alike. + * + * The database name is deliberately NOT in the message: names routinely embed a user id, which + * would hand every user a private error group in Sentry and make the class uncountable. It rides + * on the error as a property instead. + */ +function storageTimeoutError( + target: any, + context: StorageContext, + budgetMs: number, + elapsedMs: number, + lateFire: boolean +) { + const storeNames = storeNamesOf(target); + const label = storeNames.length ? ` [${storeNames.join(', ')}]` : ''; + // `budgetMs` is the deadline this operation was armed with, not the current static: the static + // can be changed while an operation is in flight, and a message that reported the new value + // would describe a deadline this transaction was never held to. + const error = namedError( + 'StorageTimeoutError', + `IndexedDB transaction${label} did not settle within ${budgetMs}ms` + ); + return Object.assign(error, { dbName: context.dbName, storeNames, elapsedMs, lateFire }); +} + +/** + * Put a deadline on an operation that may never fire an event. Returns null when both timers are + * disabled, which is the default — see the statics on Browserbase. + * + * Unlike @dabble/patches, whose store wrappers report every individual request settle, there is no + * per-request progress signal available here: when a transaction is passed to `requestToPromise` + * the request's promise is chained onto the transaction's, so it resolves only once the whole + * transaction completes. The connection-wide `lastSettleAt` is therefore the only liveness + * evidence, and it is what the defer window reads. + */ +function armStorageGuard( + target: any, + context: StorageContext, + onTimeout: (budgetMs: number, elapsedMs: number, lateFire: boolean) => void +) { + const softBudget = Browserbase.slowTransactionTimeout; + const hardBudget = Browserbase.transactionTimeout; + if (softBudget <= 0 && hardBudget <= 0) return null; + + const started = Date.now(); + let softTimer: ReturnType | undefined; + let hardTimer: ReturnType | undefined; + let scheduledAt = started; + let budget = hardBudget; + let reArmedLate = false; + let deferWindows = 0; + + if (softBudget > 0) { + softTimer = setTimeout(() => { + softTimer = undefined; + const elapsedMs = Date.now() - started; + Browserbase.onSlowStorage?.({ + dbName: context.dbName, + storeNames: storeNamesOf(target), + elapsedMs, + lateFire: elapsedMs > softBudget * LATE_FIRE_FACTOR, + }); + }, softBudget); + } + + function scheduleHard(ms: number) { + scheduledAt = Date.now(); + budget = ms; + hardTimer = setTimeout(fireHard, ms); + } + + function fireHard() { + hardTimer = undefined; + const elapsedMs = Date.now() - started; + + // The timer itself came back late, so the tab was suspended and this window measured nothing. + // Once only: a genuinely stalled transaction on a tab that keeps sleeping must still resolve. + if (!reArmedLate && Date.now() - scheduledAt > budget * LATE_FIRE_FACTOR) { + reArmedLate = true; + scheduleHard(hardBudget); + return; + } + + // Something else on this connection settled recently, so the connection is alive and this + // transaction is queued behind work rather than wedged. Aborting a healthy-but-queued + // transaction is its own bug; wait out the remainder of a fresh window instead. + const silentFor = Date.now() - context.lastSettleAt; + if (silentFor < hardBudget && deferWindows < MAX_DEFER_WINDOWS) { + deferWindows++; + scheduleHard(hardBudget - silentFor); + return; + } + + onTimeout(hardBudget, elapsedMs, reArmedLate); + } + + if (hardBudget > 0) scheduleHard(hardBudget); + + return { + clear() { + if (softTimer !== undefined) clearTimeout(softTimer); + if (hardTimer !== undefined) clearTimeout(hardTimer); + softTimer = hardTimer = undefined; + }, + }; +} + function successHandler(resolve: (result: any) => void) { return (event: Event) => resolve((event.target as any).result); } diff --git a/src/storageGuard.spec.ts b/src/storageGuard.spec.ts new file mode 100644 index 0000000..81541ad --- /dev/null +++ b/src/storageGuard.spec.ts @@ -0,0 +1,282 @@ +import indexeddb, { IDBKeyRange, IDBTransaction } from 'fake-indexeddb'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Browserbase } from './Browserbase'; + +globalThis.indexedDB = indexeddb; +globalThis.IDBTransaction = IDBTransaction; +globalThis.IDBKeyRange = IDBKeyRange; + +describe('Browserbase storage stall guard', () => { + const defaults = { + slow: Browserbase.slowTransactionTimeout, + hard: Browserbase.transactionTimeout, + onSlow: Browserbase.onSlowStorage, + }; + + // A wall clock the tests can jump forward, standing in for a tab the browser suspended: the + // timer still fires on the real event loop, but Date.now() reports that a long time passed + // meanwhile. This is what the guard actually reads, so it needs no fake timers. + let clockOffset = 0; + const realNow = Date.now.bind(Date); + + beforeEach(() => { + clockOffset = 0; + vi.spyOn(Date, 'now').mockImplementation(() => realNow() + clockOffset); + }); + + afterEach(() => { + Browserbase.slowTransactionTimeout = defaults.slow; + Browserbase.transactionTimeout = defaults.hard; + Browserbase.onSlowStorage = defaults.onSlow; + vi.restoreAllMocks(); + }); + + const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + + const storeNameList = (names: string[]) => { + const list: any = [...names]; + list.contains = (name: string) => list.includes(name); + return list; + }; + + // A transaction that accepts handlers and then never fires one — the wedge itself. + function stalledTransaction(names = ['foo']) { + return { + objectStoreNames: storeNameList(names), + objectStore: (name: string) => ({ name, keyPath: 'key', createIndex() {}, deleteIndex() {} }), + oncomplete: null, + onerror: null, + onabort: null, + aborted: false, + abort() { + this.aborted = true; + }, + } as any; + } + + // Opens a Browserbase against a hand-driven connection whose transactions the test controls. + async function openWith(makeTransaction: () => any) { + const request: any = { + result: null, + transaction: null, + onsuccess: null, + onerror: null, + onblocked: null, + onupgradeneeded: null, + }; + vi.spyOn(indexedDB, 'open').mockReturnValue(request); + + const db = new Browserbase('stall' + (Math.random() + '').slice(2), { dontDispatch: true }); + db.version(1, { foo: 'key' }); + const opening = db.open(); + + request.result = { + closed: false, + objectStoreNames: storeNameList(['foo']), + transaction: makeTransaction, + close() { + this.closed = true; + }, + onerror: null, + onabort: null, + onversionchange: null, + onclose: null, + }; + request.onsuccess(); + await opening; + return db; + } + + it('leaves a never-settling transaction alone when both timers are disabled', async () => { + // The shipped default. A library release must not change how anything already behaves. + Browserbase.slowTransactionTimeout = 0; + Browserbase.transactionTimeout = 0; + const reports: any[] = []; + Browserbase.onSlowStorage = detail => reports.push(detail); + + const trans = stalledTransaction(); + const db = await openWith(() => trans); + + let state = 'pending'; + const scoped = db.start(['foo']); + void scoped.commit().then( + () => (state = 'resolved'), + () => (state = 'rejected') + ); + + await delay(60); + expect(state).to.equal('pending'); + expect(reports).to.have.length(0); + expect(trans.aborted).toBe(false); + }); + + it('reports a slow transaction without disturbing it', async () => { + Browserbase.slowTransactionTimeout = 20; + Browserbase.transactionTimeout = 0; + const reports: any[] = []; + Browserbase.onSlowStorage = detail => reports.push(detail); + + const trans = stalledTransaction(['foo']); + const db = await openWith(() => trans); + + let state = 'pending'; + const scoped = db.start(['foo']); + void scoped.commit().then( + () => (state = 'resolved'), + () => (state = 'rejected') + ); + + await delay(50); + expect(reports).to.have.length(1); + expect(reports[0].storeNames).to.deep.equal(['foo']); + expect(reports[0].elapsedMs).toBeGreaterThanOrEqual(20); + expect(reports[0].lateFire).toBe(false); + expect(reports[0].dbName).to.equal(db.name); + // Observation only: the operation itself is untouched. + expect(state).to.equal('pending'); + expect(trans.aborted).toBe(false); + }); + + it('flags a slow report as lateFire when the clock jumped while the tab slept', async () => { + Browserbase.slowTransactionTimeout = 20; + Browserbase.transactionTimeout = 0; + const reports: any[] = []; + Browserbase.onSlowStorage = detail => reports.push(detail); + + const db = await openWith(() => stalledTransaction()); + const scoped = db.start(['foo']); + void scoped.commit().catch(() => {}); + + clockOffset = 5000; // suspended before the timer got to run + await delay(50); + + expect(reports).to.have.length(1); + expect(reports[0].lateFire).toBe(true); + expect(reports[0].elapsedMs).toBeGreaterThanOrEqual(5000); + }); + + it('rejects, aborts and dispatches when a transaction outlasts the hard timeout', async () => { + Browserbase.slowTransactionTimeout = 0; + Browserbase.transactionTimeout = 20; + + const trans = stalledTransaction(['foo']); + const db = await openWith(() => trans); + const errors: Error[] = []; + db.addEventListener('error', (event: any) => errors.push(event.error)); + + const scoped = db.start(['foo']); + const error: any = await scoped.commit().catch(err => err); + + expect(error.name).to.equal('StorageTimeoutError'); + expect(error.message).toContain('[foo]'); + // The database name embeds a uid in real use, so it must stay out of the message or every + // user gets their own error group in Sentry. + expect(error.message).not.toContain(db.name); + expect(error.dbName).to.equal(db.name); + expect(error.storeNames).to.deep.equal(['foo']); + expect(error.elapsedMs).toBeGreaterThanOrEqual(20); + // A stalled transaction left live could still commit after its caller gave up. + expect(trans.aborted).toBe(true); + expect(errors.map(e => e.name)).toContain('StorageTimeoutError'); + }); + + it('does not fire once the transaction completes', async () => { + Browserbase.slowTransactionTimeout = 0; + Browserbase.transactionTimeout = 30; + + const trans = stalledTransaction(); + const db = await openWith(() => trans); + + const scoped = db.start(['foo']); + const settled = scoped.commit(); + trans.oncomplete({ target: trans }); + + await settled; + await delay(60); + expect(trans.aborted).toBe(false); + }); + + it('defers while other work on the same connection is still settling', async () => { + Browserbase.slowTransactionTimeout = 0; + Browserbase.transactionTimeout = 30; + + const trans = stalledTransaction(); + const db = await openWith(() => trans); + + const scoped = db.start(['foo']); + let state = 'pending'; + const settled = scoped.commit().then( + () => (state = 'resolved'), + () => (state = 'rejected') + ); + + // A sibling transaction keeps completing, so the connection is demonstrably alive and this + // one is queued behind work rather than wedged (DAB-834: never abort a healthy queued txn). + for (let i = 0; i < 4; i++) { + await delay(20); + db.storageContext.lastSettleAt = Date.now(); + } + expect(state).to.equal('pending'); + expect(trans.aborted).toBe(false); + + // Once the connection goes quiet, the deadline lands. + await delay(90); + await settled; + expect(state).to.equal('rejected'); + }); + + it('re-arms once when the timer itself came back late, then fires', async () => { + Browserbase.slowTransactionTimeout = 0; + Browserbase.transactionTimeout = 25; + + const db = await openWith(() => stalledTransaction()); + const scoped = db.start(['foo']); + let state = 'pending'; + const settled = scoped.commit().then( + () => (state = 'resolved'), + err => { + state = 'rejected'; + return err; + } + ); + + // The tab slept through the first window, so that window measured nothing. + clockOffset = 5000; + await delay(40); + expect(state).to.equal('pending'); + + // The second window is served awake, so this time it is a real verdict. + await delay(60); + const error: any = await settled; + expect(state).to.equal('rejected'); + expect(error.name).to.equal('StorageTimeoutError'); + expect(error.lateFire).toBe(true); + }); + + it('bounds a stalled read that has no transaction promise behind it', async () => { + // get/getAll/count settle on the request's own events with no transaction promise to chain + // onto, so they are the shape with nothing at all to fall back on. + Browserbase.slowTransactionTimeout = 0; + Browserbase.transactionTimeout = 20; + + const stalledRequest: any = { onsuccess: null, onerror: null, source: { name: 'foo' } }; + const trans = stalledTransaction(); + trans.objectStore = () => ({ getAll: () => stalledRequest }); + stalledRequest.transaction = trans; + + const db = await openWith(() => trans); + const error: any = await db.stores.foo.getAll().catch((err: Error) => err); + + expect(error.name).to.equal('StorageTimeoutError'); + expect(error.storeNames).to.deep.equal(['foo']); + expect(trans.aborted).toBe(true); + }); + + it('shares one storage context between a connection and its transaction clones', async () => { + const db = await openWith(() => stalledTransaction()); + const scoped = db.start(['foo']); + expect(scoped.storageContext).toBe(db.storageContext); + expect(db.storageContext.dbName).to.equal(db.name); + void scoped.commit().catch(() => {}); + }); +});