[HOLD for payment 2024-10-11] Reduce the number of redundant ReconnectApp calls or clean up the logic that enqueues them

[muttmuure]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

What performance issue do we need to solve?

e.g. memory consumption, storage read/write times, React native bridge concerns, inefficient React component rendering, etc.

@gedu has noticed that app calls ReconnectApp a lot, often redundantly

Here: https://github.com/Expensify/App/blob/main/src/libs/PusherUtils.ts
Thread: https://expensify.slack.com/archives/C05LX9D6E07/p1724159545236079?thread_ts=1721152209.600379&cid=C05LX9D6E07

Here: https://github.com/Expensify/App/blob/main/src/libs/Middleware/RecheckConnection.ts
Thread: https://expensify.slack.com/archives/C05LX9D6E07/p1724076205528369?thread_ts=1721152209.600379&cid=C05LX9D6E07

Here: https://expensify.slack.com/archives/C05LX9D6E07/p1723806097226589?thread_ts=1721152209.600379&cid=C05LX9D6E07

What is the impact of this on end-users?

List specific user experiences that will be improved by solving this problem e.g. app boot time, time to for some interaction to complete, etc.

If a user's request queue gets stuck (like it does here) then ReconnectApp can cause the request queue to climb with unprocessed requests

It has been suggested as the root cause with issues with partially loaded LHNs, but also broken queues of requests

List any benchmarks that show the severity of the issue

Please also provide exact steps taken to collect metrics above if any so we can independently verify the results.

https://expensify.slack.com/archives/C05LX9D6E07/p1723566890894989?thread_ts=1721152209.600379&cid=C05LX9D6E07

@danielrvidal's queue of requests climbed to 630 here

Proposed solution (if any)

Please list out the steps you think we should take to solve this issue.

Reduce the number of queued ReconnectApp calls or clean up the logic that queues them so they don't climb so high

List any benchmarks after implementing the changes to show impacts of the proposed solution (if any)

Note: These should be the same as the benchmarks collected before any changes.

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Version Number:
Reproducible in staging?:
Reproducible in production?:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by: @danielrvidal
Slack conversation: https://expensify.slack.com/archives/C05LX9D6E07/p1723566890894989?thread_ts=1721152209.600379&cid=C05LX9D6E07

View all open jobs on Upwork

[melvin-bot]

Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.

[melvin-bot]

Triggered auto assignment to @marcochavezf (AutoAssignerNewDotQuality)

[muttmuure]

@gedu I think you're working on this one?

[gedu]

Hey, yes, I'm working on this one

[gedu]

Sent a first "proposal" for discussion: on slack

[melvin-bot]

This issue has not been updated in over 15 days. @gedu, @marcochavezf eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[muttmuure]

Bugs that have this root cause are being spread across different threads -

https://expensify.slack.com/archives/C05LX9D6E07/p1723132613137659?thread_ts=1722970682.900369&cid=C05LX9D6E07

https://expensify.slack.com/archives/C05LX9D6E07/p1721152209600379

So we're going to consolidate in this issue, because it describes the problem the best

[iwiznia]

BTW I see this also happening with OpenApp: https://github.com/Expensify/Expensify/issues/431330 maybe the issues are related

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.44-12 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• De-dupe ReconnectApp in the persisted requests queue #47913

If no regressions arise, payment will be issued on 2024-10-11. 🎊

For reference, here are some details about the assignees on this issue:

• @gedu does not require payment (Contractor)

[melvin-bot]

Skipping the payment summary for this issue since all the assignees are employees or vendors. If this is incorrect, please manually add the payment summary SO.

[melvin-bot]

@gedu, @marcochavezf Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[marcochavezf]

PR has been merged, and it looks like we're good here. Closing it out, but feel free to re-open if something is missing