Fix: Prevent Onyx.clear() from removing unsynced PERSISTED_REQUESTS data#87874
Fix: Prevent Onyx.clear() from removing unsynced PERSISTED_REQUESTS data#87874marufsharifi wants to merge 8 commits intoExpensify:mainfrom
Conversation
melvin-bot
Bot
requested review from
ShridharGoel and
flaviadefaria
and removed request for
a team and
flaviadefaria
|
@ShridharGoel Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| Onyx.clear(KEYS_TO_PRESERVE).then(() => { | ||
| // eslint-disable-next-line @typescript-eslint/no-use-before-define |
There was a problem hiding this comment.
❌ CONSISTENCY-5 (docs)
The eslint-disable-next-line @typescript-eslint/no-use-before-define comment lacks an accompanying justification explaining why the rule needs to be suppressed. Even though the original code had a similar disable, the new usage should include a clear reason.
Add a justification comment, for example:
// eslint-disable-next-line @typescript-eslint/no-use-before-define -- clearOnyxAndResetApp is defined later in this file but must be called here in the RESET_REQUIRED callback
clearOnyxAndResetApp().finally(() => {Reviewed at: 8fab326 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fab326610
ℹ️ 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".
| Onyx.multiSet({ | ||
| [ONYXKEYS.PERSISTED_REQUESTS]: persistedRequests, | ||
| [ONYXKEYS.PERSISTED_ONGOING_REQUESTS]: ongoingRequest, | ||
| }); |
There was a problem hiding this comment.
processNextRequest() now persists [PERSISTED_REQUESTS]: persistedRequests immediately after popping the head, but that write is async and not coordinated with save(). If a new request is queued while the current one is in flight, a later-resolving multiSet from this block can overwrite the newer save() result with a stale snapshot; the PERSISTED_REQUESTS Onyx callback then blindly assigns that stale disk value back to memory, dropping newly queued requests from retry processing.
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| if (persistedRequestsLength === 0 && !hasOnyxUpdates) { | ||
| if (persistedRequestsLength === 0 && !currentOngoingRequest && !hasOnyxUpdates) { |
There was a problem hiding this comment.
This new guard lets flush() run when there is only an ongoing request. In the offline recovery path, process() returns early, but flush completion still decides “queue empty” from getAllPersistedRequests().length only, so it can resolve readiness and flush queued Onyx updates while PERSISTED_ONGOING_REQUESTS is still pending. That reorders post-write update application relative to the recovered ongoing write.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the queue completion path. flush() can still start when only currentOngoingRequest exists, but we now also check for an ongoing request before resolving readiness or flushing queued Onyx updates, so the queue is no longer treated as empty while PERSISTED_ONGOING_REQUESTS is still present.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 525ff2a3a4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (newRequest.persistWhenOngoing) { | ||
| Onyx.set(ONYXKEYS.PERSISTED_ONGOING_REQUESTS, newRequest as AnyRequest); | ||
| } | ||
| Onyx.set(ONYXKEYS.PERSISTED_ONGOING_REQUESTS, newRequest as AnyRequest); |
There was a problem hiding this comment.
Persisting every updated ongoing request unconditionally can reintroduce DataCloneError for requests whose data contains non-serializable File/Blob values (the same case already called out in processNextRequest()). HandleUnusedOptimisticID can call updateOngoingRequest() while rewriting report IDs, so if the ongoing command is attachment-related, this Onyx.set() can fail and leave the old ongoing request on disk, which means a post-crash retry may still use the stale report ID you were trying to replace.
Useful? React with 👍 / 👎.
|
@ShridharGoel, I've addressed AI feedback. all yours. thanks. |
2 participants
Explanation of Change
These changes make offline expense creation more reliable during app recovery.
Before this, an expense created offline could disappear if the app reset local storage during recovery, or if the app closed while that expense request was already being processed. The result was that the server never received the write, and after reconnect the expense appeared to be missing.
With this update, recovery resets now reuse the safer reset flow that preserves pending work, and the request queue now treats in-flight requests as durable state instead of temporary memory-only state. That means offline-created expenses are much more likely to survive app resets, reloads, and reconnects until they successfully sync. Logout-style destructive flows were not expanded here; this is specifically about protecting recovery behavior.
Fixed Issues
$ #86710
PROPOSAL: #86710 (comment)
Tests
Other Test Cases:
Case 1: Offline expense survives reset
Settings > Troubleshoot.Force offline.Clear cache and restart/Reset and refresh.Force offlineoff.Case 2: Multiple offline expenses survive reset
Force offline.Clear cache and restart.Case 3: Offline split-expense flow survives reset
Force offline.Splitexpense.Clear cache and restart.Case 4: Track expense survives reset
Force offline.Track expense.Troubleshoot.Case 5: Restart while request is in progress
This is important because your PR also touches ongoing request behavior.
Troubleshoot, turn onSimulate poor internet connection.Case 6: Normal online expense flow still works
Case 7: No bad side effect on non-expense navigation
Troubleshoot.Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Not applicable.
Android: mWeb Chrome
Not applicable.
iOS: Native
Not applicable.
iOS: mWeb Safari
Not applicable.
MacOS: Chrome / Safari
Screen.Recording.2026-04-14.at.4.31.25.PM.mp4
Screen.Recording.2026-04-20.at.1.09.35.PM.mp4
Screen.Recording.2026-04-20.at.1.11.04.PM.mp4
Screen.Recording.2026-04-20.at.1.22.09.PM.mov
Screen.Recording.2026-04-20.at.1.24.32.PM.mov
Screen.Recording.2026-04-20.at.1.27.14.PM.mov
Screen.Recording.2026-04-20.at.1.29.08.PM.mov
Screen.Recording.2026-04-20.at.1.30.31.PM.mov