Skip to content

Update error handling in Onyx to prevent unnecessary data eviction #694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
+474 −231
Open

Update error handling in Onyx to prevent unnecessary data eviction #694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
df729c3
Update evictStorageAndRetry to not evict the data if error isn't stor…
VickyStash Oct 28, 2025
6fc70ed
Minor improvement and test fix
VickyStash Oct 28, 2025
c4d29dc
Applied feedback
VickyStash Oct 28, 2025
67bd9d4
Add comments
VickyStash Oct 29, 2025
a8fbd85
Add a simple limitation of operations retries
VickyStash Oct 29, 2025
42f44fb
Merge branch 'main' into VickyStash/bugfix/update-eviction-rules
VickyStash Oct 29, 2025
3bfb0e5
re-run checks
VickyStash Oct 29, 2025
9db02f2
Add internal functions for retries
VickyStash Oct 30, 2025
c146abf
Update mergeCollectionWithPatches
VickyStash Oct 30, 2025
5a6d696
Update partialSetCollection
VickyStash Oct 30, 2025
23e996e
Rename const
VickyStash Oct 30, 2025
1bd718a
Minor clean-up
VickyStash Oct 30, 2025
b57b22a
Rename const
VickyStash Oct 30, 2025
5e0c4ad
Move setWithRetry, multiSetWithRetry, setCollectionWithRetry to OnyxU…
VickyStash Oct 30, 2025
7f0bf74
Minor code improvements
VickyStash Oct 31, 2025
ca326b9
Add unit tests
VickyStash Oct 31, 2025
c55cc64
Update the docs
VickyStash Oct 31, 2025
7da26ae
Adjust params passed to the retry function
VickyStash Nov 3, 2025
f5e8b4b
Apply feedback
VickyStash Nov 3, 2025
fa98ffb
Merge branch 'main' into VickyStash/bugfix/update-eviction-rules
VickyStash Nov 3, 2025
3a6e61d
Add changes from main
VickyStash Nov 3, 2025
8ebfc39
Add logging when max retries are reached
VickyStash Nov 3, 2025
fddfe2d
Re-run checks
VickyStash Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 96 additions & 21 deletions API-INTERNAL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you regenerate these docs after your recent changes? I don't think retryOperation() is exposed anywhere publicly, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgolen I've re-generated it one more time.
Yeah, it's not public! This file is also API-INTERNAL (describes internal methods)

<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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>

Expand Down
180 changes: 10 additions & 170 deletions lib/Onyx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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});
}

/**
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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});
}

/**
Expand Down Expand Up @@ -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>}));
}
});

Expand Down Expand Up @@ -656,63 +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);

// Preserve references for unchanged items in setCollection
const preservedCollection = OnyxUtils.preserveCollectionReferences(keyValuePairs);

const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, 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 = {
Expand Down
Loading
Loading