Skip to content

Commit 87568d7

Browse files
authored
Merge pull request #694 from callstack-internal/VickyStash/bugfix/update-eviction-rules
Update error handling in Onyx to prevent unnecessary data eviction
2 parents cc0f68e + 447f9e3 commit 87568d7

File tree

7 files changed

+479
-232
lines changed

7 files changed

+479
-232
lines changed

API-INTERNAL.md

Lines changed: 96 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,13 @@ subscriber callbacks receive the data in a different format than they normally e
109109
<dt><a href="#remove">remove()</a></dt>
110110
<dd><p>Remove a key from Onyx and update the subscribers</p>
111111
</dd>
112-
<dt><a href="#evictStorageAndRetry">evictStorageAndRetry()</a></dt>
113-
<dd><p>If we fail to set or merge we must handle this by
114-
evicting some data from Onyx and then retrying to do
115-
whatever it is we attempted to do.</p>
112+
<dt><a href="#retryOperation">retryOperation()</a></dt>
113+
<dd><p>Handles storage operation failures based on the error type:</p>
114+
<ul>
115+
<li>Storage capacity errors: evicts data and retries the operation</li>
116+
<li>Invalid data errors: logs an alert and throws an error</li>
117+
<li>Other errors: retries the operation</li>
118+
</ul>
116119
</dd>
117120
<dt><a href="#broadcastUpdate">broadcastUpdate()</a></dt>
118121
<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
147150
<dt><a href="#unsubscribeFromKey">unsubscribeFromKey(subscriptionID)</a></dt>
148151
<dd><p>Disconnects and removes the listener from the Onyx key.</p>
149152
</dd>
150-
<dt><a href="#mergeCollectionWithPatches">mergeCollectionWithPatches(collectionKey, collection, mergeReplaceNullPatches)</a></dt>
153+
<dt><a href="#setWithRetry">setWithRetry(params, retryAttempt)</a></dt>
154+
<dd><p>Writes a value to our store with the given key.
155+
Serves as core implementation for <code>Onyx.set()</code> public function, the difference being
156+
that this internal function allows passing an additional <code>retryAttempt</code> parameter to retry on failure.</p>
157+
</dd>
158+
<dt><a href="#multiSetWithRetry">multiSetWithRetry(data, retryAttempt)</a></dt>
159+
<dd><p>Sets multiple keys and values.
160+
Serves as core implementation for <code>Onyx.multiSet()</code> public function, the difference being
161+
that this internal function allows passing an additional <code>retryAttempt</code> parameter to retry on failure.</p>
162+
</dd>
163+
<dt><a href="#setCollectionWithRetry">setCollectionWithRetry(params, retryAttempt)</a></dt>
164+
<dd><p>Sets a collection by replacing all existing collection members with new values.
165+
Any existing collection members not included in the new data will be removed.
166+
Serves as core implementation for <code>Onyx.setCollection()</code> public function, the difference being
167+
that this internal function allows passing an additional <code>retryAttempt</code> parameter to retry on failure.</p>
168+
</dd>
169+
<dt><a href="#mergeCollectionWithPatches">mergeCollectionWithPatches(params, retryAttempt)</a></dt>
151170
<dd><p>Merges a collection based on their keys.
152171
Serves as core implementation for <code>Onyx.mergeCollection()</code> public function, the difference being
153-
that this internal function allows passing an additional <code>mergeReplaceNullPatches</code> parameter.</p>
172+
that this internal function allows passing an additional <code>mergeReplaceNullPatches</code> parameter and retries on failure.</p>
154173
</dd>
155-
<dt><a href="#partialSetCollection">partialSetCollection(collectionKey, collection)</a></dt>
174+
<dt><a href="#partialSetCollection">partialSetCollection(params, retryAttempt)</a></dt>
156175
<dd><p>Sets keys in a collection by replacing all targeted collection members with new values.
157-
Any existing collection members not included in the new data will not be removed.</p>
176+
Any existing collection members not included in the new data will not be removed.
177+
Retries on failure.</p>
158178
</dd>
159179
<dt><a href="#clearOnyxUtilsInternals">clearOnyxUtilsInternals()</a></dt>
160180
<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
406426
Remove a key from Onyx and update the subscribers
407427

408428
**Kind**: global function
409-
<a name="evictStorageAndRetry"></a>
429+
<a name="retryOperation"></a>
410430

411-
## evictStorageAndRetry()
412-
If we fail to set or merge we must handle this by
413-
evicting some data from Onyx and then retrying to do
414-
whatever it is we attempted to do.
431+
## retryOperation()
432+
Handles storage operation failures based on the error type:
433+
- Storage capacity errors: evicts data and retries the operation
434+
- Invalid data errors: logs an alert and throws an error
435+
- Other errors: retries the operation
415436

416437
**Kind**: global function
417438
<a name="broadcastUpdate"></a>
@@ -507,33 +528,87 @@ Disconnects and removes the listener from the Onyx key.
507528
| --- | --- |
508529
| subscriptionID | Subscription ID returned by calling `OnyxUtils.subscribeToKey()`. |
509530

531+
<a name="setWithRetry"></a>
532+
533+
## setWithRetry(params, retryAttempt)
534+
Writes a value to our store with the given key.
535+
Serves as core implementation for `Onyx.set()` public function, the difference being
536+
that this internal function allows passing an additional `retryAttempt` parameter to retry on failure.
537+
538+
**Kind**: global function
539+
540+
| Param | Description |
541+
| --- | --- |
542+
| params | set parameters |
543+
| params.key | ONYXKEY to set |
544+
| params.value | value to store |
545+
| params.options | optional configuration object |
546+
| retryAttempt | retry attempt |
547+
548+
<a name="multiSetWithRetry"></a>
549+
550+
## multiSetWithRetry(data, retryAttempt)
551+
Sets multiple keys and values.
552+
Serves as core implementation for `Onyx.multiSet()` public function, the difference being
553+
that this internal function allows passing an additional `retryAttempt` parameter to retry on failure.
554+
555+
**Kind**: global function
556+
557+
| Param | Description |
558+
| --- | --- |
559+
| data | object keyed by ONYXKEYS and the values to set |
560+
| retryAttempt | retry attempt |
561+
562+
<a name="setCollectionWithRetry"></a>
563+
564+
## setCollectionWithRetry(params, retryAttempt)
565+
Sets a collection by replacing all existing collection members with new values.
566+
Any existing collection members not included in the new data will be removed.
567+
Serves as core implementation for `Onyx.setCollection()` public function, the difference being
568+
that this internal function allows passing an additional `retryAttempt` parameter to retry on failure.
569+
570+
**Kind**: global function
571+
572+
| Param | Description |
573+
| --- | --- |
574+
| params | collection parameters |
575+
| params.collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
576+
| params.collection | Object collection keyed by individual collection member keys and values |
577+
| retryAttempt | retry attempt |
578+
510579
<a name="mergeCollectionWithPatches"></a>
511580

512-
## mergeCollectionWithPatches(collectionKey, collection, mergeReplaceNullPatches)
581+
## mergeCollectionWithPatches(params, retryAttempt)
513582
Merges a collection based on their keys.
514583
Serves as core implementation for `Onyx.mergeCollection()` public function, the difference being
515-
that this internal function allows passing an additional `mergeReplaceNullPatches` parameter.
584+
that this internal function allows passing an additional `mergeReplaceNullPatches` parameter and retries on failure.
516585

517586
**Kind**: global function
518587

519588
| Param | Description |
520589
| --- | --- |
521-
| collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
522-
| collection | Object collection keyed by individual collection member keys and values |
523-
| 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. |
590+
| params | mergeCollection parameters |
591+
| params.collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
592+
| params.collection | Object collection keyed by individual collection member keys and values |
593+
| 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. |
594+
| params.isProcessingCollectionUpdate | whether this is part of a collection update operation. |
595+
| retryAttempt | retry attempt |
524596

525597
<a name="partialSetCollection"></a>
526598

527-
## partialSetCollection(collectionKey, collection)
599+
## partialSetCollection(params, retryAttempt)
528600
Sets keys in a collection by replacing all targeted collection members with new values.
529601
Any existing collection members not included in the new data will not be removed.
602+
Retries on failure.
530603

531604
**Kind**: global function
532605

533606
| Param | Description |
534607
| --- | --- |
535-
| collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
536-
| collection | Object collection keyed by individual collection member keys and values |
608+
| params | collection parameters |
609+
| params.collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` |
610+
| params.collection | Object collection keyed by individual collection member keys and values |
611+
| retryAttempt | retry attempt |
537612

538613
<a name="clearOnyxUtilsInternals"></a>
539614

lib/Onyx.ts

Lines changed: 10 additions & 170 deletions
Original file line numberDiff line numberDiff line change
@@ -150,71 +150,7 @@ function disconnect(connection: Connection): void {
150150
* @param options optional configuration object
151151
*/
152152
function set<TKey extends OnyxKey>(key: TKey, value: OnyxSetInput<TKey>, options?: SetOptions): Promise<void> {
153-
// When we use Onyx.set to set a key we want to clear the current delta changes from Onyx.merge that were queued
154-
// before the value was set. If Onyx.merge is currently reading the old value from storage, it will then not apply the changes.
155-
if (OnyxUtils.hasPendingMergeForKey(key)) {
156-
delete OnyxUtils.getMergeQueue()[key];
157-
}
158-
159-
const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs();
160-
if (skippableCollectionMemberIDs.size) {
161-
try {
162-
const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
163-
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
164-
// The key is a skippable one, so we set the new value to null.
165-
// eslint-disable-next-line no-param-reassign
166-
value = null;
167-
}
168-
} catch (e) {
169-
// The key is not a collection one or something went wrong during split, so we proceed with the function's logic.
170-
}
171-
}
172-
173-
// Onyx.set will ignore `undefined` values as inputs, therefore we can return early.
174-
if (value === undefined) {
175-
return Promise.resolve();
176-
}
177-
178-
const existingValue = cache.get(key, false);
179-
// If the existing value as well as the new value are null, we can return early.
180-
if (existingValue === undefined && value === null) {
181-
return Promise.resolve();
182-
}
183-
184-
// Check if the value is compatible with the existing value in the storage
185-
const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(value, existingValue);
186-
if (!isCompatible) {
187-
Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'set', existingValueType, newValueType));
188-
return Promise.resolve();
189-
}
190-
191-
// If the change is null, we can just delete the key.
192-
// Therefore, we don't need to further broadcast and update the value so we can return early.
193-
if (value === null) {
194-
OnyxUtils.remove(key);
195-
OnyxUtils.logKeyRemoved(OnyxUtils.METHOD.SET, key);
196-
return Promise.resolve();
197-
}
198-
199-
const valueWithoutNestedNullValues = utils.removeNestedNullValues(value) as OnyxValue<TKey>;
200-
const hasChanged = options?.skipCacheCheck ? true : cache.hasValueChanged(key, valueWithoutNestedNullValues);
201-
202-
OnyxUtils.logKeyChanged(OnyxUtils.METHOD.SET, key, value, hasChanged);
203-
204-
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
205-
const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNestedNullValues, hasChanged);
206-
207-
// 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.
208-
if (!hasChanged) {
209-
return updatePromise;
210-
}
211-
212-
return Storage.setItem(key, valueWithoutNestedNullValues)
213-
.catch((error) => OnyxUtils.evictStorageAndRetry(error, set, key, valueWithoutNestedNullValues))
214-
.then(() => {
215-
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNestedNullValues);
216-
return updatePromise;
217-
});
153+
return OnyxUtils.setWithRetry({key, value, options});
218154
}
219155

220156
/**
@@ -225,47 +161,7 @@ function set<TKey extends OnyxKey>(key: TKey, value: OnyxSetInput<TKey>, options
225161
* @param data object keyed by ONYXKEYS and the values to set
226162
*/
227163
function multiSet(data: OnyxMultiSetInput): Promise<void> {
228-
let newData = data;
229-
230-
const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs();
231-
if (skippableCollectionMemberIDs.size) {
232-
newData = Object.keys(newData).reduce((result: OnyxMultiSetInput, key) => {
233-
try {
234-
const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
235-
// If the collection member key is a skippable one we set its value to null.
236-
// eslint-disable-next-line no-param-reassign
237-
result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? newData[key] : null;
238-
} catch {
239-
// The key is not a collection one or something went wrong during split, so we assign the data to result anyway.
240-
// eslint-disable-next-line no-param-reassign
241-
result[key] = newData[key];
242-
}
243-
244-
return result;
245-
}, {});
246-
}
247-
248-
const keyValuePairsToSet = OnyxUtils.prepareKeyValuePairsForStorage(newData, true);
249-
250-
const updatePromises = keyValuePairsToSet.map(([key, value]) => {
251-
// When we use multiSet to set a key we want to clear the current delta changes from Onyx.merge that were queued
252-
// before the value was set. If Onyx.merge is currently reading the old value from storage, it will then not apply the changes.
253-
if (OnyxUtils.hasPendingMergeForKey(key)) {
254-
delete OnyxUtils.getMergeQueue()[key];
255-
}
256-
257-
// Update cache and optimistically inform subscribers on the next tick
258-
cache.set(key, value);
259-
return OnyxUtils.scheduleSubscriberUpdate(key, value);
260-
});
261-
262-
return Storage.multiSet(keyValuePairsToSet)
263-
.catch((error) => OnyxUtils.evictStorageAndRetry(error, multiSet, newData))
264-
.then(() => {
265-
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MULTI_SET, undefined, newData);
266-
return Promise.all(updatePromises);
267-
})
268-
.then(() => undefined);
164+
return OnyxUtils.multiSetWithRetry(data);
269165
}
270166

271167
/**
@@ -374,7 +270,7 @@ function merge<TKey extends OnyxKey>(key: TKey, changes: OnyxMergeInput<TKey>):
374270
* @param collection Object collection keyed by individual collection member keys and values
375271
*/
376272
function mergeCollection<TKey extends CollectionKeyBase>(collectionKey: TKey, collection: OnyxMergeCollectionInput<TKey>): Promise<void> {
377-
return OnyxUtils.mergeCollectionWithPatches(collectionKey, collection, undefined, true);
273+
return OnyxUtils.mergeCollectionWithPatches({collectionKey, collection, isProcessingCollectionUpdate: true});
378274
}
379275

380276
/**
@@ -608,16 +504,16 @@ function update(data: OnyxUpdate[]): Promise<void> {
608504

609505
if (!utils.isEmptyObject(batchedCollectionUpdates.merge)) {
610506
promises.push(() =>
611-
OnyxUtils.mergeCollectionWithPatches(
507+
OnyxUtils.mergeCollectionWithPatches({
612508
collectionKey,
613-
batchedCollectionUpdates.merge as OnyxMergeCollectionInput<OnyxKey>,
614-
batchedCollectionUpdates.mergeReplaceNullPatches,
615-
true,
616-
),
509+
collection: batchedCollectionUpdates.merge as OnyxMergeCollectionInput<OnyxKey>,
510+
mergeReplaceNullPatches: batchedCollectionUpdates.mergeReplaceNullPatches,
511+
isProcessingCollectionUpdate: true,
512+
}),
617513
);
618514
}
619515
if (!utils.isEmptyObject(batchedCollectionUpdates.set)) {
620-
promises.push(() => OnyxUtils.partialSetCollection(collectionKey, batchedCollectionUpdates.set as OnyxSetCollectionInput<OnyxKey>));
516+
promises.push(() => OnyxUtils.partialSetCollection({collectionKey, collection: batchedCollectionUpdates.set as OnyxSetCollectionInput<OnyxKey>}));
621517
}
622518
});
623519

@@ -655,63 +551,7 @@ function update(data: OnyxUpdate[]): Promise<void> {
655551
* @param collection Object collection keyed by individual collection member keys and values
656552
*/
657553
function setCollection<TKey extends CollectionKeyBase>(collectionKey: TKey, collection: OnyxSetCollectionInput<TKey>): Promise<void> {
658-
let resultCollection: OnyxInputKeyValueMapping = collection;
659-
let resultCollectionKeys = Object.keys(resultCollection);
660-
661-
// Confirm all the collection keys belong to the same parent
662-
if (!OnyxUtils.doAllCollectionItemsBelongToSameParent(collectionKey, resultCollectionKeys)) {
663-
Logger.logAlert(`setCollection called with keys that do not belong to the same parent ${collectionKey}. Skipping this update.`);
664-
return Promise.resolve();
665-
}
666-
667-
const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs();
668-
if (skippableCollectionMemberIDs.size) {
669-
resultCollection = resultCollectionKeys.reduce((result: OnyxInputKeyValueMapping, key) => {
670-
try {
671-
const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key, collectionKey);
672-
// If the collection member key is a skippable one we set its value to null.
673-
// eslint-disable-next-line no-param-reassign
674-
result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? resultCollection[key] : null;
675-
} catch {
676-
// Something went wrong during split, so we assign the data to result anyway.
677-
// eslint-disable-next-line no-param-reassign
678-
result[key] = resultCollection[key];
679-
}
680-
681-
return result;
682-
}, {});
683-
}
684-
resultCollectionKeys = Object.keys(resultCollection);
685-
686-
return OnyxUtils.getAllKeys().then((persistedKeys) => {
687-
const mutableCollection: OnyxInputKeyValueMapping = {...resultCollection};
688-
689-
persistedKeys.forEach((key) => {
690-
if (!key.startsWith(collectionKey)) {
691-
return;
692-
}
693-
if (resultCollectionKeys.includes(key)) {
694-
return;
695-
}
696-
697-
mutableCollection[key] = null;
698-
});
699-
700-
const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true);
701-
const previousCollection = OnyxUtils.getCachedCollection(collectionKey);
702-
703-
// Preserve references for unchanged items in setCollection
704-
const preservedCollection = OnyxUtils.preserveCollectionReferences(keyValuePairs);
705-
706-
const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, previousCollection);
707-
708-
return Storage.multiSet(keyValuePairs)
709-
.catch((error) => OnyxUtils.evictStorageAndRetry(error, setCollection, collectionKey, collection))
710-
.then(() => {
711-
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection);
712-
return updatePromise;
713-
});
714-
});
554+
return OnyxUtils.setCollectionWithRetry({collectionKey, collection});
715555
}
716556

717557
const Onyx = {

0 commit comments

Comments
 (0)