Silence the cache-miss diagnostic for keys removed in another tab (web) by LukasMod · Pull Request #884 · Expensify/react-native-onyx

LukasMod · 2026-10-30T15:49:15Z

Details

Follow-up to #863

This PR clears the noise from [Onyx] Cache held no value for a key storage still has, so the lines left in the logs point at real cache misses.

Production logs about 7,200 of these lines per working day (average of the five weekdays Oct 28 to Nov 3), all from web browsers. I ran two tabs with extra logging and traced every hit to the other tab removing a key and writing it back, or removing it twice. None of them was a lost value.

On web, the sync handler applies a key removed in another tab as cache.set(key, undefined), so the key stays indexed with no value. The cache-miss diagnostic from #863 reads "indexed, no value" as a lost value. It then reads storage, logs if the other tab has already written the key back, and uses up the collection's one report for the session, which can hide a later real miss in that collection.

The sync handler still calls cache.set(key, undefined) as on main, and now also marks the key as removed in another tab. The diagnostic skips marked keys. Any change to the key in this tab (set, merge, hydrate, drop) clears the mark, so a later real miss on that key, or on another member of its collection, still logs.

Cache values, the key index, the collection member registry and subscriber notifications behave as on main. Only the diagnostic changes.

Out of scope: a race in the sync receiver. It reads a key from disk and applies what it finds, so a removal answer can be older than a write this tab still has on its way to storage. That race exists on main and this PR leaves it alone. The diagnostic also skips the marked key in that case, so these logs won't tell us whether the race happens

Related Issues

Expensify/App#101910
Follow-up to #863

Linked E/App PR

Expensify/App#105902

Automated Tests

tests/unit/OnyxSubscriptionManagerTest.ts, cache miss reporting:

a key another tab removed causes no storage read and no log
another tab removes a key and writes it back before this tab reads it: no log (this is the production noise)
a real miss on another member of the same collection still logs once after a removal
a key that gets a value again after the removal still logs a real miss

tests/unit/onyxCacheTest.tsx: the mark doesn't count as cached, and set, merge, hydrate and drop clear it.

tests/unit/onyxTest.ts, stale removal synced from another instance: Onyx.clear still deletes the row, mergeCollection merges instead of overwriting, multiSet and mergeCollection with null delete the row, and setCollection deletes a member it leaves out. These all match main and fail if synced removals take the key out of the index again.

tests/unit/useOnyxTest.ts: useOnyx on a key another tab removed, known-empty or not, reports loading while a merge is pending and then the merged value.

Full suite: 787 tests passing. Lint and tsc --noEmit clean.

Manual Tests

Web, two tabs on one account, console open with Verbose level. In every test, neither tab should log [Onyx] Cache held no value for a key storage still has.

Basic flow

Sign in on tab A and tab B, open the same chat in both.
Send a few messages from tab A, react to one, delete one.
Sign out in tab A, then sign back in.
Confirm both tabs still render the chat list and the messages.

Remove then re-add the same key

In tab A, add a reaction to a message, remove it, add it again.
In tab A, delete a message, then send a new one in the same chat.
Confirm tab B shows the final reaction, the deletion and the new message without a reload, and the chat preview in the LHN matches.

Collection shrinks from the other tab

In tab A, leave a group chat or delete a workspace, then archive a chat.
Confirm tab B drops each item from the LHN and the workspace list, with no empty row left and nothing stale in search.
In tab A, create a new chat or workspace, confirm it appears in tab B.

Offline queue across tabs

In tab A, go offline in DevTools, send three messages, go back online.
Confirm tab B shows the three messages once, with no stuck sending state and no duplicates.

Expense created and deleted from the other tab

In tab A, create an expense and write a message in its thread.
Confirm tab B shows the same expense and message.
In tab A, delete the expense.
Confirm tab B shows the deletion.

Removal while the other tab is writing the same key

In tab B, open an expense and go into the amount editor. In tab A, delete that expense. In tab B, save the amount.
Confirm tab B shows no stuck loading state, and after a reload both tabs agree the expense is gone.
In both tabs, react to the same message within a second of each other, then remove the reaction in tab A.
Confirm tab B shows the final reaction state with no stuck loading state.

Sign-out during the other tab's reload

In tab A, throttle CPU 4x in DevTools and reload. While it is still loading, sign out in tab B.
Confirm both tabs end on the sign-in screen with no console errors.

Author Checklist

Screenshots/Videos

Android: Native

N/A

Android: mWeb Chrome

android-web.mov

iOS: Native

N/A

iOS: mWeb Safari

ios-web.mov

MacOS: Chrome / Safari

web.mov

test.2.mov

test3.mov

test4-5.mov

signout.mov

LukasMod · 2026-10-30T16:24:39Z

@codex review

chatgpt-codex-connector · 2026-10-30T16:24:56Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-05T10:34:51.359866Z	`cb8bdd5`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector · 2026-10-30T16:26:01Z

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

An unknown error occurred

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

TaduJR · 2026-11-03T15:02:34Z

Assignment here @chuckdries

Thanks!

LukasMod · 2026-11-04T09:51:13Z

@codex review

chatgpt-codex-connector · 2026-11-04T09:55:36Z

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: b7751b0321

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

LukasMod · 2026-11-04T10:42:40Z

@codex review

chatgpt-codex-connector · 2026-11-04T10:46:10Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: cb8bdd5c6d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

fabioh8010

@LukasMod Could you write and verify more manual tests for tab syncing? Just to make sure the change is safe.

LukasMod · 2026-11-05T10:07:47Z

@fabioh8010 I added 4 more tests, looks okay. I didn't spot issues

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb8bdd5c6d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

TaduJR · 2026-11-05T11:00:23Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified any copy / text that was added to the app is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If the PR adds or modifies the UI:
- I asked an AI agent to review the changes for accessibility issues and addressed its findings.
- I tested with a screen reader (VoiceOver on macOS) and verified all new/changed elements are reachable with a logical focus order.
- I verified all new/changed elements have meaningful accessible names and roles.
- I verified state changes are announced (e.g. checked/unchecked, expanded/collapsed, selected).
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

Android: mWeb Chrome

iOS: HybridApp

iOS: mWeb Safari

MacOS: Chrome / Safari

TaduJR · 2026-11-05T11:02:29Z

Bump @chuckdries on this one #884 (comment) as we are coming from follow-up of this review #863 (review)

Thanks!

chuckdries · 2026-11-05T15:40:12Z

I'm stealing the review on this one since I have context on the prior PRs and the project

…iss check

LukasMod · 2026-11-09T10:34:40Z

@TaduJR I went with your version, since my main goal was to reduce the false-positive logs from multi-tab sync. I agree the race you found needs a separate PR 👍

I took your commit as is (40d1c56). It:

removes the drop
adds a guard used only by the diagnostic
adds tests that should stop us from reintroducing a similar mechanism, since, as you noticed, it can lose data

I re-ran the manual tests and couldn't reproduce those logs anymore. Thanks for your help on this one!

PR with onyx pin updated too: Expensify/App#105902

TaduJR

This looks good to me.

A few optional nits inline, none of which changes behaviour. The one worth keeping is the test: Onyx.clear must not reset the mark on a key it keeps, or a removal and write-back from another tab that straddle the clear logs the false alarm again.

TaduJR · 2026-11-09T16:25:59Z

@@ -320,6 +339,7 @@ class OnyxCache {
                }
            } else {
                this.nullishStorageKeys.delete(key);


Suggested change

this.nullishStorageKeys.delete(key);

this.forgetEmptyMarks(key);

TaduJR · 2026-11-09T16:25:59Z

@@ -249,6 +267,7 @@ class OnyxCache {
                }
            } else {
                this.nullishStorageKeys.delete(key);


Suggested change

this.nullishStorageKeys.delete(key);

this.forgetEmptyMarks(key);

TaduJR · 2026-11-09T16:26:01Z

@@ -173,6 +189,7 @@ class OnyxCache {
        // When a key is explicitly set in cache, we can remove it from the list of nullish keys,
        // since it will either be set to a non nullish value or removed from the cache completely.
        this.nullishStorageKeys.delete(key);


Suggested change

this.nullishStorageKeys.delete(key);

this.forgetEmptyMarks(key);

TaduJR · 2026-11-09T16:26:03Z

+        this.removedInOtherInstanceKeys.add(key);
+    }
+
+    /** Whether another instance removed this key and it hasn't changed here since */


set, hydrate and merge clear the two sets together by hand. A small helper keeps them in step; the next three suggestions use it.

Suggested change

/** Whether another instance removed this key and it hasn't changed here since */

/** Whether another instance removed this key and it hasn't changed here since */

hasRemovedInOtherInstanceKey(key: OnyxKey): boolean {

return this.removedInOtherInstanceKeys.has(key);

}

/** A write here replaces what the cache knew about the key being empty, including another instance's removal */

private forgetEmptyMarks(key: OnyxKey): void {

this.nullishStorageKeys.delete(key);

this.removedInOtherInstanceKeys.delete(key);

}

right, fixed

TaduJR · 2026-11-09T16:26:43Z

@@ -79,6 +79,10 @@ function init({
                // Capture the previous cached value before cache.set() so notifyCollection() can diff old vs new per member.
                const previousValue = isCollectionMember ? cache.get(key) : undefined;
                cache.set(key, value);


Nit: this comment explains the cache.set (why the key stays indexed), so it reads better above it.

Suggested change

cache.set(key, value);

// The key stays indexed because this answer can predate a write of ours still on its way to storage, and only indexed keys get deleted by Onyx.clear and collection sweeps.

cache.set(key, value);

yup, good idea

TaduJR · 2026-11-09T16:27:52Z


+        // The handler Onyx.init registered for changes another tab wrote to storage.
+        const getSyncCallback = (): ((pairs: Array<[OnyxKey, unknown]>) => void) => {
+            const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0] as ((pairs: Array<[OnyxKey, unknown]>) => void) | undefined;


Nit: these sync tests rely on shouldSyncMultipleInstances defaulting from global.localStorage. Passing shouldSyncMultipleInstances: true in this file's Onyx.init, and in useOnyxTest.ts, would make that explicit.

…lear keeps marks

LukasMod · 2026-11-10T09:11:38Z

@TaduJR Thanks for review and all tips, I applied all your suggestions in f62fa78

melvin-bot Bot assigned LukasMod Oct 30, 2026

LukasMod force-pushed the fix/mark-removed-keys-nullish branch from 5e70e90 to 54d9cc0 Compare October 30, 2026 16:16

LukasMod mentioned this pull request Oct 30, 2026

Fix cache-miss diagnostic for undefined keys #863

Merged

42 tasks done

LukasMod force-pushed the fix/mark-removed-keys-nullish branch 2 times, most recently from 32b3733 to 9306e4f Compare October 30, 2026 16:20

LukasMod force-pushed the fix/mark-removed-keys-nullish branch from 9306e4f to db151fe Compare November 2, 2026 10:00

LukasMod changed the title ~~Mark keys set to null in cache as known empty and keep in-flight local writes on sync~~ Mark keys removed in another tab or missing in storage as known empty Nov 2, 2026

LukasMod changed the title ~~Mark keys removed in another tab or missing in storage as known empty~~ Drop keys removed in another tab instead of leaving them indexed with no value Nov 2, 2026

LukasMod mentioned this pull request Nov 3, 2026

[$1500] Expose Onyx.get Expensify/App#101910

Open

LukasMod force-pushed the fix/mark-removed-keys-nullish branch from db6a92f to a8e806b Compare November 3, 2026 13:51

LukasMod added 2 commits November 3, 2026 16:44

Mark keys removed in another tab or missing in storage as known empty

8c4a4a5

Drop keys removed in another tab instead of marking them known empty

b7751b0

LukasMod force-pushed the fix/mark-removed-keys-nullish branch from a8e806b to b7751b0 Compare November 3, 2026 15:45

Log when another tab removes a key this tab is still writing

0905b80

LukasMod force-pushed the fix/mark-removed-keys-nullish branch from 4e82545 to 0905b80 Compare November 4, 2026 10:22

Split the sync removal tests and match the sibling sync callback pattern

cb8bdd5

LukasMod force-pushed the fix/mark-removed-keys-nullish branch from 8881b39 to cb8bdd5 Compare November 4, 2026 10:37

LukasMod changed the title ~~Drop keys removed in another tab instead of leaving them indexed with no value~~ Drop keys removed in another tab instead of leaving them indexed with no value (web) Nov 4, 2026

LukasMod mentioned this pull request Nov 4, 2026

[No QA] [ONYX] Pin react-native-onyx to 40d1c56 for onyx#884 testing Expensify/App#105902

Draft

38 tasks

fabioh8010 reviewed Nov 5, 2026

View reviewed changes

LukasMod marked this pull request as ready for review November 5, 2026 10:31

LukasMod requested a review from a team as a code owner November 5, 2026 10:31

melvin-bot Bot requested review from stitesExpensify and removed request for a team November 5, 2026 10:32

chatgpt-codex-connector Bot reviewed Nov 5, 2026

View reviewed changes

Comment thread lib/Onyx.ts Resolved

aimane-chnaif suggested changes Nov 5, 2026

View reviewed changes

Comment thread lib/Onyx.ts Resolved

Comment thread tests/unit/OnyxSubscriptionManagerTest.ts Resolved

Comment thread tests/unit/OnyxSubscriptionManagerTest.ts Resolved

Comment thread lib/Onyx.ts Resolved

LukasMod added 2 commits November 5, 2026 12:09

Pin that a known-empty key removed in another tab stays known empty

d64f6b8

Drop the sync removal log and its tests, keep the drop fix only

e057b8f

LukasMod requested a review from aimane-chnaif November 5, 2026 12:23

aimane-chnaif approved these changes Nov 5, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

chuckdries requested review from chuckdries and removed request for stitesExpensify November 5, 2026 15:40

fabioh8010 mentioned this pull request Nov 6, 2026

[Tracking] Onyx Improvements Expensify/App#80355

Open

Keep keys removed in another tab indexed and only silence the cache-m…

40d1c56

…iss check

LukasMod changed the title ~~Drop keys removed in another tab instead of leaving them indexed with no value (web)~~ Silence the cache-miss diagnostic for keys removed in another tab (web) Nov 9, 2026

TaduJR suggested changes Nov 9, 2026

View reviewed changes

TaduJR reviewed Nov 9, 2026

View reviewed changes

Comment thread tests/unit/OnyxSubscriptionManagerTest.ts Resolved

Apply review nits: forgetEmptyMarks helper, explicit sync in tests, c…

f62fa78

…lear keeps marks

LukasMod requested a review from TaduJR November 10, 2026 09:13

	this.nullishStorageKeys.delete(key);
	this.forgetEmptyMarks(key);

-    /** Whether another instance removed this key and it hasn't changed here since */
+    /** Whether another instance removed this key and it hasn't changed here since */
+    hasRemovedInOtherInstanceKey(key: OnyxKey): boolean {
+        return this.removedInOtherInstanceKeys.has(key);
+    }
+    /** A write here replaces what the cache knew about the key being empty, including another instance's removal */
+    private forgetEmptyMarks(key: OnyxKey): void {
+        this.nullishStorageKeys.delete(key);
+        this.removedInOtherInstanceKeys.delete(key);
+    }

	cache.set(key, value);
	// The key stays indexed because this answer can predate a write of ours still on its way to storage, and only indexed keys get deleted by Onyx.clear and collection sweeps.
	cache.set(key, value);

Conversation

LukasMod commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Related Issues

Linked E/App PR

Automated Tests

Manual Tests

Author Checklist

Screenshots/Videos

Uh oh!

LukasMod commented Oct 30, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot commented Oct 30, 2026

Uh oh!

TaduJR commented Nov 3, 2026

Uh oh!

LukasMod commented Nov 4, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 4, 2026

Uh oh!

LukasMod commented Nov 4, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 4, 2026

Uh oh!

fabioh8010 left a comment

Choose a reason for hiding this comment

Uh oh!

LukasMod commented Nov 5, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TaduJR commented Nov 5, 2026

Reviewer Checklist

Screenshots/Videos

Uh oh!

TaduJR commented Nov 5, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

chuckdries commented Nov 5, 2026

Uh oh!

LukasMod commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TaduJR left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LukasMod commented Nov 10, 2026

Uh oh!

LukasMod commented Oct 30, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 30, 2026 •

edited

Loading

LukasMod commented Nov 9, 2026 •

edited

Loading