Update error handling in Onyx to prevent unnecessary data eviction

API-INTERNAL.md

@@ -109,10 +109,13 @@ 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:</p>
+<ul>
+<li>Storage capacity errors: evicts data and retries the operation</li>
+<li>Invalid data errors: logs an alert and throws an error</li>
+<li>Other errors: retries the operation</li>
+</ul>
 </dd>
 <dt><a href="#broadcastUpdate">broadcastUpdate()</a></dt>
 <dd><p>Notifies subscribers and writes current value to cache</p>
@@ -147,14 +150,31 @@ It will also mark deep nested objects that need to be entirely replaced during t
 <dt><a href="#unsubscribeFromKey">unsubscribeFromKey(subscriptionID)</a></dt>
 <dd><p>Disconnects and removes the listener from the Onyx key.</p>
 </dd>
-<dt><a href="#mergeCollectionWithPatches">mergeCollectionWithPatches(collectionKey, collection, mergeReplaceNullPatches)</a></dt>
+<dt><a href="#setWithRetry">setWithRetry(params, retryAttempt)</a></dt>
+<dd><p>Writes a value to our store with the given key.
+Serves as core implementation for <code>Onyx.set()</code> public function, the difference being
+that this internal function allows passing an additional <code>retryAttempt</code> parameter to retry on failure.</p>
+</dd>
+<dt><a href="#multiSetWithRetry">multiSetWithRetry(data, retryAttempt)</a></dt>
+<dd><p>Sets multiple keys and values.
+Serves as core implementation for <code>Onyx.multiSet()</code> public function, the difference being
+that this internal function allows passing an additional <code>retryAttempt</code> parameter to retry on failure.</p>
+</dd>
+<dt><a href="#setCollectionWithRetry">setCollectionWithRetry(params, retryAttempt)</a></dt>
+<dd><p>Sets a collection by replacing all existing collection members with new values.
+Any existing collection members not included in the new data will be removed.
+Serves as core implementation for <code>Onyx.setCollection()</code> public function, the difference being
+that this internal function allows passing an additional <code>retryAttempt</code> parameter to retry on failure.</p>
+</dd>
+<dt><a href="#mergeCollectionWithPatches">mergeCollectionWithPatches(params, retryAttempt)</a></dt>
 <dd><p>Merges a collection based on their keys.
 Serves as core implementation for <code>Onyx.mergeCollection()</code> public function, the difference being
-that this internal function allows passing an additional <code>mergeReplaceNullPatches</code> parameter.</p>
+that this internal function allows passing an additional <code>mergeReplaceNullPatches</code> parameter and retries on failure.</p>
 </dd>
-<dt><a href="#partialSetCollection">partialSetCollection(collectionKey, collection)</a></dt>
+<dt><a href="#partialSetCollection">partialSetCollection(params, retryAttempt)</a></dt>
 <dd><p>Sets keys in a collection by replacing all targeted collection members with new values.
-Any existing collection members not included in the new data will not be removed.</p>
+Any existing collection members not included in the new data will not be removed.
+Retries on failure.</p>
 </dd>
 <dt><a href="#clearOnyxUtilsInternals">clearOnyxUtilsInternals()</a></dt>
 <dd><p>Clear internal variables used in this file, useful in test environments.</p>
@@ -406,12 +426,13 @@ subscriber callbacks receive the data in a different format than they normally e
 Remove a key from Onyx and update the subscribers
 
 **Kind**: global function  
-<a name="evictStorageAndRetry"></a>
+<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>
@@ -507,33 +528,87 @@ Disconnects and removes the listener from the Onyx key.
 | --- | --- |
 | subscriptionID | Subscription ID returned by calling `OnyxUtils.subscribeToKey()`. |
 
+<a name="setWithRetry"></a>
+
+## setWithRetry(params, retryAttempt)
+Writes a value to our store with the given key.
+Serves as core implementation for `Onyx.set()` public function, the difference being
+that this internal function allows passing an additional `retryAttempt` parameter to retry on failure.
+
+**Kind**: global function  
+
+| Param | Description |
+| --- | --- |
+| params | set parameters |
+| params.key | ONYXKEY to set |
+| params.value | value to store |
+| params.options | optional configuration object |
+| retryAttempt | retry attempt |
+
+<a name="multiSetWithRetry"></a>
+
+## multiSetWithRetry(data, retryAttempt)
+Sets multiple keys and values.
+Serves as core implementation for `Onyx.multiSet()` public function, the difference being
+that this internal function allows passing an additional `retryAttempt` parameter to retry on failure.
+
+**Kind**: global function  
+
+| Param | Description |
+| --- | --- |
+| data | object keyed by ONYXKEYS and the values to set |
+| retryAttempt | retry attempt |
+
+<a name="setCollectionWithRetry"></a>
+
+## setCollectionWithRetry(params, retryAttempt)
+Sets a collection by replacing all existing collection members with new values.
+Any existing collection members not included in the new data will be removed.
+Serves as core implementation for `Onyx.setCollection()` public function, the difference being
+that this internal function allows passing an additional `retryAttempt` parameter to retry on failure.
+
+**Kind**: global function  
+
+| Param | Description |
+| --- | --- |
+| params | collection parameters |
+| params.collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
+| params.collection | Object collection keyed by individual collection member keys and values |
+| retryAttempt | retry attempt |
+
 <a name="mergeCollectionWithPatches"></a>
 
-## mergeCollectionWithPatches(collectionKey, collection, mergeReplaceNullPatches)
+## mergeCollectionWithPatches(params, retryAttempt)
 Merges a collection based on their keys.
 Serves as core implementation for `Onyx.mergeCollection()` public function, the difference being
-that this internal function allows passing an additional `mergeReplaceNullPatches` parameter.
+that this internal function allows passing an additional `mergeReplaceNullPatches` parameter and retries on failure.
 
 **Kind**: global function  
 
 | Param | Description |
 | --- | --- |
-| collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
-| collection | Object collection keyed by individual collection member keys and values |
-| mergeReplaceNullPatches | Record where the key is a collection member key and the value is a list of tuples that we'll use to replace the nested objects of that collection member record with something else. |
+| params | mergeCollection parameters |
+| params.collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
+| params.collection | Object collection keyed by individual collection member keys and values |
+| params.mergeReplaceNullPatches | Record where the key is a collection member key and the value is a list of tuples that we'll use to replace the nested objects of that collection member record with something else. |
+| params.isProcessingCollectionUpdate | whether this is part of a collection update operation. |
+| retryAttempt | retry attempt |
 
 <a name="partialSetCollection"></a>
 
-## partialSetCollection(collectionKey, collection)
+## partialSetCollection(params, retryAttempt)
 Sets keys in a collection by replacing all targeted collection members with new values.
 Any existing collection members not included in the new data will not be removed.
+Retries on failure.
 
 **Kind**: global function  
 
 | Param | Description |
 | --- | --- |
-| collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
-| collection | Object collection keyed by individual collection member keys and values |
+| params | collection parameters |
+| params.collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
+| params.collection | Object collection keyed by individual collection member keys and values |
+| retryAttempt | retry attempt |
 
 <a name="clearOnyxUtilsInternals"></a>
 

lib/Onyx.ts

@@ -151,71 +151,7 @@ function disconnect(connection: Connection): void {
  * @param options optional configuration object
  */
 function set<TKey extends OnyxKey>(key: TKey, value: OnyxSetInput<TKey>, options?: SetOptions): Promise<void> {
-    // When we use Onyx.set to set a key we want to clear the current delta changes from Onyx.merge that were queued
-    // before the value was set. If Onyx.merge is currently reading the old value from storage, it will then not apply the changes.
-    if (OnyxUtils.hasPendingMergeForKey(key)) {
-        delete OnyxUtils.getMergeQueue()[key];
-    }
-
-    const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs();
-    if (skippableCollectionMemberIDs.size) {
-        try {
-            const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
-            if (skippableCollectionMemberIDs.has(collectionMemberID)) {
-                // The key is a skippable one, so we set the new value to null.
-                // eslint-disable-next-line no-param-reassign
-                value = null;
-            }
-        } catch (e) {
-            // The key is not a collection one or something went wrong during split, so we proceed with the function's logic.
-        }
-    }
-
-    // Onyx.set will ignore `undefined` values as inputs, therefore we can return early.
-    if (value === undefined) {
-        return Promise.resolve();
-    }
-
-    const existingValue = cache.get(key, false);
-    // If the existing value as well as the new value are null, we can return early.
-    if (existingValue === undefined && value === null) {
-        return Promise.resolve();
-    }
-
-    // Check if the value is compatible with the existing value in the storage
-    const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(value, existingValue);
-    if (!isCompatible) {
-        Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'set', existingValueType, newValueType));
-        return Promise.resolve();
-    }
-
-    // If the change is null, we can just delete the key.
-    // Therefore, we don't need to further broadcast and update the value so we can return early.
-    if (value === null) {
-        OnyxUtils.remove(key);
-        OnyxUtils.logKeyRemoved(OnyxUtils.METHOD.SET, key);
-        return Promise.resolve();
-    }
-
-    const valueWithoutNestedNullValues = utils.removeNestedNullValues(value) as OnyxValue<TKey>;
-    const hasChanged = options?.skipCacheCheck ? true : cache.hasValueChanged(key, valueWithoutNestedNullValues);
-
-    OnyxUtils.logKeyChanged(OnyxUtils.METHOD.SET, key, value, hasChanged);
-
-    // This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
-    const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNestedNullValues, hasChanged);
-
-    // If the value has not changed or the key got removed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
-    if (!hasChanged) {
-        return updatePromise;
-    }
-
-    return Storage.setItem(key, valueWithoutNestedNullValues)
-        .catch((error) => OnyxUtils.evictStorageAndRetry(error, set, key, valueWithoutNestedNullValues))
-        .then(() => {
-            OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNestedNullValues);
-            return updatePromise;
-        });
+    return OnyxUtils.setWithRetry({key, value, options});
 }
 
 /**
@@ -226,47 +162,7 @@ function set<TKey extends OnyxKey>(key: TKey, value: OnyxSetInput<TKey>, options
  * @param data object keyed by ONYXKEYS and the values to set
  */
 function multiSet(data: OnyxMultiSetInput): Promise<void> {
-    let newData = data;
-
-    const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs();
-    if (skippableCollectionMemberIDs.size) {
-        newData = Object.keys(newData).reduce((result: OnyxMultiSetInput, key) => {
-            try {
-                const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
-                // If the collection member key is a skippable one we set its value to null.
-                // eslint-disable-next-line no-param-reassign
-                result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? newData[key] : null;
-            } catch {
-                // The key is not a collection one or something went wrong during split, so we assign the data to result anyway.
-                // eslint-disable-next-line no-param-reassign
-                result[key] = newData[key];
-            }
-
-            return result;
-        }, {});
-    }
-
-    const keyValuePairsToSet = OnyxUtils.prepareKeyValuePairsForStorage(newData, true);
-
-    const updatePromises = keyValuePairsToSet.map(([key, value]) => {
-        // When we use multiSet to set a key we want to clear the current delta changes from Onyx.merge that were queued
-        // before the value was set. If Onyx.merge is currently reading the old value from storage, it will then not apply the changes.
-        if (OnyxUtils.hasPendingMergeForKey(key)) {
-            delete OnyxUtils.getMergeQueue()[key];
-        }
-
-        // Update cache and optimistically inform subscribers on the next tick
-        cache.set(key, value);
-        return OnyxUtils.scheduleSubscriberUpdate(key, value);
-    });
-
-    return Storage.multiSet(keyValuePairsToSet)
-        .catch((error) => OnyxUtils.evictStorageAndRetry(error, multiSet, newData))
-        .then(() => {
-            OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MULTI_SET, undefined, newData);
-            return Promise.all(updatePromises);
-        })
-        .then(() => undefined);
+    return OnyxUtils.multiSetWithRetry(data);
 }
 
 /**
@@ -375,7 +271,7 @@ function merge<TKey extends OnyxKey>(key: TKey, changes: OnyxMergeInput<TKey>):
  * @param collection Object collection keyed by individual collection member keys and values
  */
 function mergeCollection<TKey extends CollectionKeyBase, TMap>(collectionKey: TKey, collection: OnyxMergeCollectionInput<TKey, TMap>): Promise<void> {
-    return OnyxUtils.mergeCollectionWithPatches(collectionKey, collection, undefined, true);
+    return OnyxUtils.mergeCollectionWithPatches({collectionKey, collection, isProcessingCollectionUpdate: true});
 }
 
 /**
@@ -609,16 +505,16 @@ function update(data: OnyxUpdate[]): Promise<void> {
 
         if (!utils.isEmptyObject(batchedCollectionUpdates.merge)) {
             promises.push(() =>
-                OnyxUtils.mergeCollectionWithPatches(
+                OnyxUtils.mergeCollectionWithPatches({
                     collectionKey,
-                    batchedCollectionUpdates.merge as Collection<CollectionKey, unknown, unknown>,
-                    batchedCollectionUpdates.mergeReplaceNullPatches,
-                    true,
-                ),
+                    collection: batchedCollectionUpdates.merge as Collection<CollectionKey, unknown, unknown>,
+                    mergeReplaceNullPatches: batchedCollectionUpdates.mergeReplaceNullPatches,
+                    isProcessingCollectionUpdate: true,
+                }),
             );
         }
         if (!utils.isEmptyObject(batchedCollectionUpdates.set)) {
-            promises.push(() => OnyxUtils.partialSetCollection(collectionKey, batchedCollectionUpdates.set as Collection<CollectionKey, unknown, unknown>));
+            promises.push(() => OnyxUtils.partialSetCollection({collectionKey, collection: batchedCollectionUpdates.set as Collection<CollectionKey, unknown, unknown>}));
         }
     });
 
@@ -656,62 +552,7 @@ function update(data: OnyxUpdate[]): Promise<void> {
  * @param collection Object collection keyed by individual collection member keys and values
  */
 function setCollection<TKey extends CollectionKeyBase, TMap>(collectionKey: TKey, collection: OnyxMergeCollectionInput<TKey, TMap>): Promise<void> {
-    let resultCollection: OnyxInputKeyValueMapping = collection;
-    let resultCollectionKeys = Object.keys(resultCollection);
-
-    // Confirm all the collection keys belong to the same parent
-    if (!OnyxUtils.doAllCollectionItemsBelongToSameParent(collectionKey, resultCollectionKeys)) {
-        Logger.logAlert(`setCollection called with keys that do not belong to the same parent ${collectionKey}. Skipping this update.`);
-        return Promise.resolve();
-    }
-
-    const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs();
-    if (skippableCollectionMemberIDs.size) {
-        resultCollection = resultCollectionKeys.reduce((result: OnyxInputKeyValueMapping, key) => {
-            try {
-                const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key, collectionKey);
-                // If the collection member key is a skippable one we set its value to null.
-                // eslint-disable-next-line no-param-reassign
-                result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? resultCollection[key] : null;
-            } catch {
-                // Something went wrong during split, so we assign the data to result anyway.
-                // eslint-disable-next-line no-param-reassign
-                result[key] = resultCollection[key];
-            }
-
-            return result;
-        }, {});
-    }
-    resultCollectionKeys = Object.keys(resultCollection);
-
-    return OnyxUtils.getAllKeys().then((persistedKeys) => {
-        const mutableCollection: OnyxInputKeyValueMapping = {...resultCollection};
-
-        persistedKeys.forEach((key) => {
-            if (!key.startsWith(collectionKey)) {
-                return;
-            }
-            if (resultCollectionKeys.includes(key)) {
-                return;
-            }
-
-            mutableCollection[key] = null;
-        });
-
-        const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true);
-        const previousCollection = OnyxUtils.getCachedCollection(collectionKey);
-
-        keyValuePairs.forEach(([key, value]) => cache.set(key, value));
-
-        const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection);
-
-        return Storage.multiSet(keyValuePairs)
-            .catch((error) => OnyxUtils.evictStorageAndRetry(error, setCollection, collectionKey, collection))
-            .then(() => {
-                OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection);
-                return updatePromise;
-            });
-    });
+    return OnyxUtils.setCollectionWithRetry({collectionKey, collection});
 }
 
 const Onyx = {