Update error handling in Onyx to prevent unnecessary data eviction

API-INTERNAL.md

@@ -109,10 +109,11 @@ subscriber callbacks receive the data in a different format than they normally e
 <dt><a href="#remove">remove()</a></dt>
 <dd><p>Remove a key from Onyx and update the subscribers</p>
 </dd>
-<dt><a href="#evictStorageAndRetry">evictStorageAndRetry()</a></dt>
-<dd><p>If we fail to set or merge we must handle this by
-evicting some data from Onyx and then retrying to do
-whatever it is we attempted to do.</p>
+<dt><a href="#retryOperation">retryOperation()</a></dt>
+<dd><p>Handles storage operation failures based on the error type:
+- Storage capacity errors: evicts data and retries the operation
+- Invalid data errors: logs an alert and throws an error
+- Other errors: retries the operation</p>
 </dd>
 <dt><a href="#broadcastUpdate">broadcastUpdate()</a></dt>
 <dd><p>Notifies subscribers and writes current value to cache</p>
@@ -405,13 +406,14 @@ subscriber callbacks receive the data in a different format than they normally e
 ## remove()
 Remove a key from Onyx and update the subscribers
 
-**Kind**: global function  
-<a name="evictStorageAndRetry"></a>
+**Kind**: global function
+<a name="retryOperation"></a>
 
-## evictStorageAndRetry()
-If we fail to set or merge we must handle this by
-evicting some data from Onyx and then retrying to do
-whatever it is we attempted to do.
+## retryOperation()
+Handles storage operation failures based on the error type:
+- Storage capacity errors: evicts data and retries the operation
+- Invalid data errors: logs an alert and throws an error
+- Other errors: retries the operation
 
 **Kind**: global function  
 <a name="broadcastUpdate"></a>

lib/Onyx.ts

@@ -208,7 +208,7 @@ function set<TKey extends OnyxKey>(key: TKey, value: OnyxSetInput<TKey>, options
     }
 
     return Storage.setItem(key, valueWithoutNestedNullValues)
-        .catch((error) => OnyxUtils.evictStorageAndRetry(error, set, key, valueWithoutNestedNullValues))
+        .catch((error) => OnyxUtils.retryOperation(error, set, key, valueWithoutNestedNullValues))
         .then(() => {
             OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNestedNullValues);
             return updatePromise;
@@ -258,7 +258,7 @@ function multiSet(data: OnyxMultiSetInput): Promise<void> {
     });
 
     return Storage.multiSet(keyValuePairsToSet)
-        .catch((error) => OnyxUtils.evictStorageAndRetry(error, multiSet, newData))
+        .catch((error) => OnyxUtils.retryOperation(error, multiSet, newData))
         .then(() => {
             OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MULTI_SET, undefined, newData);
             return Promise.all(updatePromises);
@@ -702,7 +702,7 @@ function setCollection<TKey extends CollectionKeyBase, TMap>(collectionKey: TKey
         const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection);
 
         return Storage.multiSet(keyValuePairs)
-            .catch((error) => OnyxUtils.evictStorageAndRetry(error, setCollection, collectionKey, collection))
+            .catch((error) => OnyxUtils.retryOperation(error, setCollection, collectionKey, collection))
             .then(() => {
                 OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection);
                 return updatePromise;

lib/OnyxUtils.ts

@@ -49,6 +49,10 @@ const METHOD = {
     CLEAR: 'clear',
 } as const;
 
+const IDB_STORAGE_ERRORS = ['quotaexceedederror'] as const;
+const SQL_STORAGE_ERRORS = ['database or disk is full', 'disk I/O error', 'out of memory'] as const;
+const STORAGE_ERRORS = [...IDB_STORAGE_ERRORS, ...SQL_STORAGE_ERRORS];
+
 type OnyxMethod = ValueOf<typeof METHOD>;
 
 // Key/value store of Onyx key and arrays of values to merge
@@ -858,11 +862,12 @@ function reportStorageQuota(): Promise<void> {
 }
 
 /**
- * If we fail to set or merge we must handle this by
- * evicting some data from Onyx and then retrying to do
- * whatever it is we attempted to do.
+ * Handles storage operation failures based on the error type:
+ * - Storage capacity errors: evicts data and retries the operation
+ * - Invalid data errors: logs an alert and throws an error
+ * - Other errors: retries the operation
  */
-function evictStorageAndRetry<TMethod extends typeof Onyx.set | typeof Onyx.multiSet | typeof Onyx.mergeCollection | typeof Onyx.setCollection>(
+function retryOperation<TMethod extends typeof Onyx.set | typeof Onyx.multiSet | typeof Onyx.mergeCollection | typeof Onyx.setCollection>(
     error: Error,
     onyxMethod: TMethod,
     ...args: Parameters<TMethod>
@@ -874,6 +879,14 @@ function evictStorageAndRetry<TMethod extends typeof Onyx.set | typeof Onyx.mult
         throw error;
     }
 
+    const errorMessage = error?.message?.toLowerCase?.();
+    const isStorageCapacityError = STORAGE_ERRORS.some((storageError) => storageError === error?.name?.toLowerCase() || errorMessage?.includes(storageError));
+
+    if (!isStorageCapacityError) {
+        // @ts-expect-error No overload matches this call.
+        return onyxMethod(...args);
+    }
+
     // Find the first key that we can remove that has no subscribers in our blocklist
     const keyForRemoval = cache.getKeyForEviction();
     if (!keyForRemoval) {
@@ -1341,7 +1354,7 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase, TMap>(
             });
 
             return Promise.all(promises)
-                .catch((error) => evictStorageAndRetry(error, mergeCollectionWithPatches, collectionKey, resultCollection))
+                .catch((error) => retryOperation(error, mergeCollectionWithPatches, collectionKey, resultCollection))
                 .then(() => {
                     sendActionToDevTools(METHOD.MERGE_COLLECTION, undefined, resultCollection);
                     return promiseUpdate;
@@ -1396,7 +1409,7 @@ function partialSetCollection<TKey extends CollectionKeyBase, TMap>(collectionKe
         const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection);
 
         return Storage.multiSet(keyValuePairs)
-            .catch((error) => evictStorageAndRetry(error, partialSetCollection, collectionKey, collection))
+            .catch((error) => retryOperation(error, partialSetCollection, collectionKey, collection))
             .then(() => {
                 sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection);
                 return updatePromise;
@@ -1452,7 +1465,7 @@ const OnyxUtils = {
     scheduleNotifyCollectionSubscribers,
     remove,
     reportStorageQuota,
-    evictStorageAndRetry,
+    retryOperation,
     broadcastUpdate,
     hasPendingMergeForKey,
     prepareKeyValuePairsForStorage,
@@ -1512,7 +1525,7 @@ GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => {
     // @ts-expect-error Reassign
     reportStorageQuota = decorateWithMetrics(reportStorageQuota, 'OnyxUtils.reportStorageQuota');
     // @ts-expect-error Complex type signature
-    evictStorageAndRetry = decorateWithMetrics(evictStorageAndRetry, 'OnyxUtils.evictStorageAndRetry');
+    retryOperation = decorateWithMetrics(retryOperation, 'OnyxUtils.retryOperation');
     // @ts-expect-error Reassign
     broadcastUpdate = decorateWithMetrics(broadcastUpdate, 'OnyxUtils.broadcastUpdate');
     // @ts-expect-error Reassign

tests/perf-test/OnyxUtils.perf-test.ts

@@ -502,12 +502,12 @@ describe('OnyxUtils', () => {
         });
     });
 
-    describe('evictStorageAndRetry', () => {
+    describe('retryOperation', () => {
         test('one call', async () => {
             const error = new Error();
             const onyxMethod = jest.fn() as typeof Onyx.set;
 
-            await measureAsyncFunction(() => OnyxUtils.evictStorageAndRetry(error, onyxMethod, '', null), {
+            await measureAsyncFunction(() => OnyxUtils.retryOperation(error, onyxMethod, '', null), {
                 beforeEach: async () => {
                     mockedReportActionsKeys.forEach((key) => OnyxCache.addLastAccessedKey(key, false));
                     OnyxCache.addLastAccessedKey(`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}1`, false);

tests/unit/cacheEvictionTest.ts

@@ -45,7 +45,7 @@ test('Cache eviction', () => {
             const setItemMock = jest.fn(originalSetItem).mockImplementationOnce(
                 () =>
                     new Promise((_resolve, reject) => {
-                        reject();
+                        reject(new Error('out of memory'));
                     }),
             );
             StorageMock.setItem = setItemMock;