diff --git a/.changeset/brave-keys-commit.md b/.changeset/brave-keys-commit.md new file mode 100644 index 00000000000..f9358594f9b --- /dev/null +++ b/.changeset/brave-keys-commit.md @@ -0,0 +1,5 @@ +--- +"@effect/platform-browser": patch +--- + +Fix IndexedDB-backed key-value writes to wait for transaction commit before reporting success. diff --git a/packages/platform-browser/src/BrowserKeyValueStore.ts b/packages/platform-browser/src/BrowserKeyValueStore.ts index 72840e3968c..dd63cbffe3b 100644 --- a/packages/platform-browser/src/BrowserKeyValueStore.ts +++ b/packages/platform-browser/src/BrowserKeyValueStore.ts @@ -74,8 +74,11 @@ export const layerIndexedDb = (options?: { return KeyValueStore.make({ clear: Effect.suspend(() => { - const store = getKvsEntriesStore(db, "readwrite") - return idbRequest({ method: "clear", message: "Failed to clear backing store" }, () => store.clear()) + return idbWriteRequest( + db, + { method: "clear", message: "Failed to clear backing store" }, + (store) => store.clear() + ) }), get: (key: string) => Effect.map( @@ -103,10 +106,10 @@ export const layerIndexedDb = (options?: { ), set: (key: string, value: string | Uint8Array) => Effect.asVoid(Effect.suspend(() => { - const store = getKvsEntriesStore(db, "readwrite") - return idbRequest( + return idbWriteRequest( + db, { method: "set", message: "Failed to set value in backing store", key }, - () => store.put({ key, value }) + (store) => store.put({ key, value }) ) })), size: Effect.suspend(() => { @@ -118,10 +121,10 @@ export const layerIndexedDb = (options?: { }), remove: (key: string) => Effect.asVoid(Effect.suspend(() => { - const store = getKvsEntriesStore(db, "readwrite") - return idbRequest( + return idbWriteRequest( + db, { method: "remove", message: "Failed to remove value from backing store", key }, - () => store.delete(key) + (store) => store.delete(key) ) })) }) @@ -171,6 +174,45 @@ const idbRequest = ( )) }) +const idbWriteRequest = ( + db: IDBDatabase, + failArgs: { method: string; message: string; key?: string }, + evaluate: (store: IDBObjectStore) => IDBRequest +): Effect.Effect => + Effect.callback((resume) => { + const transaction = db.transaction(entriesStoreName, "readwrite") + const request = evaluate(transaction.objectStore(entriesStoreName)) + let result: A + let done = false + + const fail = (cause: unknown) => { + if (done) return + done = true + resume(Effect.fail(new KeyValueStore.KeyValueStoreError({ ...failArgs, cause }))) + } + + if (request.readyState === "done") { + result = request.result + } else { + request.onsuccess = () => { + result = request.result + } + request.onerror = () => fail(request.error) + } + + transaction.oncomplete = () => { + if (done) return + done = true + resume(Effect.succeed(result!)) + } + transaction.onerror = () => fail(transaction.error) + transaction.onabort = () => fail(transaction.error) + + return Effect.sync(() => { + if (!done) transaction.abort() + }) + }) + const getKvsEntriesStore = (db: IDBDatabase, mode: IDBTransactionMode) => { const transaction = db.transaction(entriesStoreName, mode) return transaction.objectStore(entriesStoreName) diff --git a/packages/platform-browser/test/BrowserKeyValueStore.test.ts b/packages/platform-browser/test/BrowserKeyValueStore.test.ts index eabe5636765..f4ca6281fbc 100644 --- a/packages/platform-browser/test/BrowserKeyValueStore.test.ts +++ b/packages/platform-browser/test/BrowserKeyValueStore.test.ts @@ -1,8 +1,11 @@ import * as BrowserKeyValueStore from "@effect/platform-browser/BrowserKeyValueStore" import * as IndexedDb from "@effect/platform-browser/IndexedDb" -import { describe } from "@effect/vitest" +import { assert, describe, it } from "@effect/vitest" import { Layer } from "effect" import { testLayer } from "effect-test/unstable/persistence/KeyValueStore.test" +import * as Effect from "effect/Effect" +import * as Result from "effect/Result" +import * as KeyValueStore from "effect/unstable/persistence/KeyValueStore" import { IDBKeyRange, indexedDB } from "fake-indexeddb" describe("KeyValueStore / layerLocalStorage", () => testLayer(BrowserKeyValueStore.layerLocalStorage)) @@ -20,4 +23,69 @@ describe("KeyValueStore / layerIndexedDb", () => { Layer.provide(layerFakeIndexedDb) ) ) + + it.effect("does not report a write before its transaction commits", () => { + const db = { + objectStoreNames: { contains: () => true }, + close() {}, + transaction() { + const transaction = { + error: null as unknown, + onabort: null as null | (() => void), + objectStore() { + return { + put() { + const request = { + readyState: "pending", + result: undefined, + error: null, + onsuccess: null as null | (() => void), + onerror: null as null | (() => void) + } + queueMicrotask(() => { + request.readyState = "done" + request.onsuccess?.() + transaction.error = new DOMException("Commit failed", "AbortError") + transaction.onabort?.() + }) + return request + } + } + } + } + return transaction + } + } + const failingIndexedDb = { + open() { + const request = { + readyState: "pending", + result: undefined as unknown, + error: null, + onsuccess: null as null | (() => void), + onerror: null as null | (() => void), + onupgradeneeded: null as null | (() => void) + } + queueMicrotask(() => { + request.readyState = "done" + request.result = db + request.onsuccess?.() + }) + return request + } + } + const layer = BrowserKeyValueStore.layerIndexedDb({ database: "transaction_repro" }).pipe( + Layer.provide(Layer.succeed( + IndexedDb.IndexedDb, + IndexedDb.make({ indexedDB: failingIndexedDb as unknown as IDBFactory, IDBKeyRange }) + )) + ) + + return Effect.gen(function*() { + const store = yield* KeyValueStore.KeyValueStore + const result = yield* Effect.result(store.set("key", "value")) + yield* Effect.yieldNow + assert.isTrue(Result.isFailure(result), "the aborted transaction was reported as successful") + }).pipe(Effect.provide(layer)) + }) })