fix: preserve cache-first merge when multiGet pre-warm rejects

The new hybrid pre-warm in mergeCollectionWithPatches replaced
`Promise.all(existingKeys.map(get))` with `multiGet(existingKeys)` on
the slow path. The two have different failure semantics:

- get() catches storage read errors per-key and resolves with undefined
  (see Logger.logInfo catch at the bottom of get()).
- multiGet() has no .catch, so Storage.multiGet rejections propagate
  all the way up.

In the cold-key path this meant a transient IndexedDB read error
rejected before cache.merge() and keysChanged() ran — subscribers
missed the in-memory merge and the outer Onyx.mergeCollection / Onyx.update
promise rejected, regressing the cache-first invariant established
in PR #787 (#787).

Adds a .catch at the call site that swallows the rejection (logging
via Logger.logInfo for visibility) so cache.merge + keysChanged still
fire when pre-warm reads fail. Doesn't modify multiGet itself —
other callers may legitimately depend on rejection visibility.

Adds a regression test that mocks Storage.multiGet to reject on the
pre-warm read and asserts:
1. Onyx.mergeCollection resolves (doesn't reject up to caller).
2. The waitForCollectionCallback subscriber sees the merged delta.

Confirmed: test fails on pre-fix code with "Received: [Error:
Transient IndexedDB read error]" on `expect(outerRejected).toBeNull()`.

Addresses chatgpt-codex-connector review on PR #793
(#793 (comment)).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: elirangoshen

lib/OnyxUtils.ts

@@ -1611,7 +1611,14 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
             // missing keys into a single Storage.multiGet call (vs. N parallel get() invocations) and
             // writes the storage values back to cache before resolving.
             const hasColdExistingKey = existingKeys.some((key) => !cache.hasCacheForKey(key));
-            const prewarmPromise = hasColdExistingKey ? multiGet(existingKeys) : Promise.resolve();
+            // Swallow pre-warm read failures the same way the previous get()-based pre-warm did
+            // (see get() catch at the bottom of its definition). Without this, a transient
+            // Storage.multiGet rejection would skip cache.merge() + keysChanged() below and
+            // regress the cache-first invariant established in #787 — subscribers would miss
+            // the merge and Onyx.mergeCollection would reject up to the caller.
+            const prewarmPromise = hasColdExistingKey
+                ? multiGet(existingKeys).catch((err) => Logger.logInfo(`mergeCollectionWithPatches pre-warm failed; proceeding with cache-only merge. Error: ${err}`))
+                : Promise.resolve();
             return prewarmPromise.then(() => {
                 // Snapshot previous values from the (now-warm) cache for keysChanged's diff, then update
                 // cache and notify subscribers synchronously BEFORE issuing storage writes. This matches

tests/unit/onyxUtilsTest.ts

@@ -1089,6 +1089,62 @@ describe('OnyxUtils', () => {
             expect(warmResult).toEqual(coldResult);
             expect(coldResult).toEqual({value: 'after', extra: 'kept'});
         });
+        it('preserves cache-first invariant when Storage.multiGet rejects on the slow path', async () => {
+            // Regression for codex review #3302454987 / PR #793. Before the .catch() at the
+            // pre-warm call site, a Storage.multiGet rejection would propagate up and skip
+            // cache.merge() + keysChanged() entirely — subscribers would miss the merge and
+            // Onyx.mergeCollection would reject. The previous get()-based pre-warm swallowed
+            // these errors per-key inside get()'s own .catch(), so the cache-first invariant
+            // from #787 held even on a flaky read.
+            const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY;
+            const coldMemberKey = `${collectionKey}1`;
+            const newMemberKey = `${collectionKey}2`;
+
+            // Seed an existing member, then evict it from cache so it's "tracked but unloaded" —
+            // the slow path will try to multiGet it.
+            await Onyx.set(coldMemberKey, {value: 'persisted'});
+            evictFromCache(coldMemberKey);
+
+            // Connect the subscriber and flush its initial data load FIRST, before installing
+            // the rejecting mock. Otherwise the connect's getCollectionDataAndSendAsObject path
+            // (whose multiGet call has no .catch) would consume the mockRejectedValueOnce and
+            // leak an unhandled rejection — not the merge pre-warm we're actually exercising.
+            const collectionCallback = jest.fn();
+            Onyx.connect({
+                key: collectionKey,
+                waitForCollectionCallback: true,
+                callback: collectionCallback,
+            });
+            await waitForPromisesToResolve();
+            collectionCallback.mockClear();
+
+            // The subscriber's connect re-populated cache, so re-evict to force the merge into
+            // the slow (cold-key) path. Then reject the next Storage.multiGet so the pre-warm
+            // read fails.
+            evictFromCache(coldMemberKey);
+            const transientError = new Error('Transient IndexedDB read error');
+            StorageMock.multiGet = jest.fn(pristineMultiGet).mockRejectedValueOnce(transientError);
+
+            // Outer promise must resolve, not reject, even when the pre-warm read fails.
+            let outerRejected: unknown = null;
+            const result = await Onyx.mergeCollection(collectionKey, {
+                [coldMemberKey]: {merged: true},
+                [newMemberKey]: {value: 'new'},
+            } as GenericCollection).catch((e: unknown) => {
+                outerRejected = e;
+            });
+            expect(outerRejected).toBeNull();
+            expect(result).toBeUndefined();
+
+            // cache.merge() + keysChanged() must still have fired so subscribers see the merge.
+            // Use toMatchObject for the cold key because a successful concurrent read may have
+            // re-populated the persisted value into cache; what we care about is that the new
+            // {merged: true} delta is applied.
+            expect(collectionCallback).toHaveBeenCalled();
+            const lastBroadcast = collectionCallback.mock.calls.at(-1)?.[0] as Record<string, unknown> | undefined;
+            expect(lastBroadcast?.[coldMemberKey]).toMatchObject({merged: true});
+            expect(lastBroadcast?.[newMemberKey]).toEqual({value: 'new'});
+        });
     });
 
     describe('multiGet cache hit consistency', () => {