Merge pull request Expensify#796 from callstack-internal/fix/handle-aborted-transactions

Unify how merge and set handles AbortErrors

Author: NicolasBonet

lib/storage/providers/IDBKeyValProvider/index.ts

@@ -9,6 +9,18 @@ import type {StorageKeyValuePair} from '../types';
 const DB_NAME = 'OnyxDB';
 const STORE_NAME = 'keyvaluepairs';
 
+/**
+ * Awaits an IndexedDB write transaction. idb-keyval's promisifyRequest rejects with
+ * `transaction.error`, which is `null` for an abort not caused by its own request
+ * (connection close / versionchange / a sibling transaction aborting). Normalize that
+ * `null` into a tagged AbortError.
+ */
+function promisifyWriteTransaction(transaction: IDBTransaction): Promise<void> {
+    return IDB.promisifyRequest(transaction).catch((error) => {
+        throw error ?? new DOMException('IDB write transaction aborted without an error', 'AbortError');
+    });
+}
+
 const provider: StorageProvider<UseStore | undefined> = {
     // We don't want to initialize the store while the JS bundle loads as idb-keyval will try to use global.indexedDB
     // which might not be available in certain environments that load the bundle (e.g. electron main process).
@@ -38,7 +50,13 @@ const provider: StorageProvider<UseStore | undefined> = {
             return provider.removeItem(key);
         }
 
-        return IDB.set(key, value, provider.store);
+        // Drive the write through the manual store transaction so promisifyWriteTransaction can
+        // normalize a null abort error — idb-keyval's IDB.set() awaits the raw transaction and
+        // would propagate the unclassifiable "Error: null".
+        return provider.store('readwrite', (store) => {
+            store.put(value, key);
+            return promisifyWriteTransaction(store.transaction);
+        });
     },
     multiGet(keysParam) {
         if (!provider.store) {
@@ -71,7 +89,7 @@ const provider: StorageProvider<UseStore | undefined> = {
                     }
                 }
 
-                return IDB.promisifyRequest(store.transaction);
+                return promisifyWriteTransaction(store.transaction);
             });
         });
     },
@@ -93,7 +111,7 @@ const provider: StorageProvider<UseStore | undefined> = {
                 }
             }
 
-            return IDB.promisifyRequest(store.transaction);
+            return promisifyWriteTransaction(store.transaction);
         });
     },
     clear() {
@@ -133,14 +151,22 @@ const provider: StorageProvider<UseStore | undefined> = {
             throw new Error('Store not initialized!');
         }
 
-        return IDB.del(key, provider.store);
+        return provider.store('readwrite', (store) => {
+            store.delete(key);
+            return promisifyWriteTransaction(store.transaction);
+        });
     },
     removeItems(keysParam) {
         if (!provider.store) {
             throw new Error('Store not initialized!');
         }
 
-        return IDB.delMany(keysParam, provider.store);
+        return provider.store('readwrite', (store) => {
+            for (const key of keysParam) {
+                store.delete(key);
+            }
+            return promisifyWriteTransaction(store.transaction);
+        });
     },
     getDatabaseSize() {
         if (!provider.store) {

tests/unit/storage/providers/IDBKeyvalProviderTest.ts

@@ -174,6 +174,77 @@ describe('IDBKeyValProvider', () => {
         });
     });
 
+    describe('write-error normalization (aborted transactions)', () => {
+        // A write transaction aborted by something other than its own request (connection close,
+        // versionchange, a sibling transaction) leaves `transaction.error === null`, which idb-keyval
+        // rejects with as-is. Every write path must instead reject with a real Error so the failure
+        // can be classified and retried.
+        function abortTransactionOnPut() {
+            const originalPut = IDBObjectStore.prototype.put;
+            jest.spyOn(IDBObjectStore.prototype, 'put').mockImplementation(function put(this: IDBObjectStore, ...args: Parameters<IDBObjectStore['put']>) {
+                const request = originalPut.apply(this, args);
+                this.transaction.abort();
+                return request;
+            });
+        }
+
+        function abortTransactionOnDelete() {
+            const originalDelete = IDBObjectStore.prototype.delete;
+            jest.spyOn(IDBObjectStore.prototype, 'delete').mockImplementation(function del(this: IDBObjectStore, ...args: Parameters<IDBObjectStore['delete']>) {
+                const request = originalDelete.apply(this, args);
+                this.transaction.abort();
+                return request;
+            });
+        }
+
+        function expectAbortError(error: unknown) {
+            expect(error).not.toBeNull();
+            expect(error).toBeInstanceOf(DOMException);
+            expect((error as DOMException).name).toBe('AbortError');
+            expect((error as DOMException).message.length).toBeGreaterThan(0);
+        }
+
+        afterEach(() => {
+            jest.restoreAllMocks();
+        });
+
+        it('should reject setItem with a tagged AbortError, never null', async () => {
+            abortTransactionOnPut();
+            const error = await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, 'value').catch((e: unknown) => e);
+            expectAbortError(error);
+        });
+
+        it('should reject multiSet with a tagged AbortError, never null', async () => {
+            abortTransactionOnPut();
+            const error = await IDBKeyValProvider.multiSet([[ONYXKEYS.TEST_KEY, 'value']]).catch((e: unknown) => e);
+            expectAbortError(error);
+        });
+
+        it('should reject multiMerge with a tagged AbortError, never null', async () => {
+            abortTransactionOnPut();
+            const error = await IDBKeyValProvider.multiMerge([[ONYXKEYS.TEST_KEY, 'value']]).catch((e: unknown) => e);
+            expectAbortError(error);
+        });
+
+        it('should reject setItem(null) with a tagged AbortError, never null', async () => {
+            abortTransactionOnDelete();
+            const error = await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, null).catch((e: unknown) => e);
+            expectAbortError(error);
+        });
+
+        it('should reject removeItem with a tagged AbortError, never null', async () => {
+            abortTransactionOnDelete();
+            const error = await IDBKeyValProvider.removeItem(ONYXKEYS.TEST_KEY).catch((e: unknown) => e);
+            expectAbortError(error);
+        });
+
+        it('should reject removeItems with a tagged AbortError, never null', async () => {
+            abortTransactionOnDelete();
+            const error = await IDBKeyValProvider.removeItems([ONYXKEYS.TEST_KEY]).catch((e: unknown) => e);
+            expectAbortError(error);
+        });
+    });
+
     describe('mergeItem', () => {
         it('should merge all the supported kinds of data correctly', async () => {
             await IDB.set(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store);