Add reproduction for platform-browser/BrowserKeyValueStore issue

.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.

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 = <A>(
       ))
   })
 
+const idbWriteRequest = <A>(
+  db: IDBDatabase,
+  failArgs: { method: string; message: string; key?: string },
+  evaluate: (store: IDBObjectStore) => IDBRequest<A>
+): Effect.Effect<A, KeyValueStore.KeyValueStoreError> =>
+  Effect.callback<A, KeyValueStore.KeyValueStoreError>((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)

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))
+  })
 })