Resolve OOM crash for high-traffic accounts caused by retryParams#95813
Resolve OOM crash for high-traffic accounts caused by retryParams#95813QichenZhu wants to merge 3 commits into
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@mountiny, since @dilshodmackbook-sketch said he doesn't have enough bandwidth to work on #95663, I took over the implementation. Could you take a look? Beforebefore.movAfterafter.movBased on the comparison videos, this fixes part of the issue: after clicking create expense, memory usage is lower than before, and the app doesn't crash at that step. However, some users can still experience a crash before that step, which has a separate cause that #94976 aims to fix. |
|
@QichenZhu Can you give more explanation to why removing a dead code fixes an issue? that does not make much sense to me |
|
@mountiny |
|
Ah ok got it, thank you! Can you make the pr ready for a review? |
Explanation of Change
This PR removes
retryParamsfrom the expense creation flow to resolve an OOM crash for high-traffic accounts.retryParamsis dead code since we removed the retry link as part of issue #80905 via PR #82139.Fixed Issues
$ #93846
PROPOSAL:
Tests
This PR focuses on resolving the Sentry crash on iOS HybridApp only. For other platforms, just test the normal expense creation flow.
Precondition: download the HEIC image from #68778 (comment), duplicate it 30 copies, and import them into iPhone 17
Offline tests
N/A.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
android-native.mov
Android: mWeb Chrome
android-web.mov
iOS: Native
Before
before.mov
After
after.mov
iOS: mWeb Safari
ios-web.mov
MacOS: Chrome / Safari
mac-web.mov