Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2024-03-26] HIGH: Fix dbarrett's infinite ReconnectApp #38160

Closed
quinthar opened this issue Mar 12, 2024 · 8 comments
Closed
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering Task Weekly KSv2

Comments

@quinthar
Copy link
Contributor

quinthar commented Mar 12, 2024

Problem:
As discussed in this extensive thread, my client just seems to constantly call ReconnectApp -- on many different networks, on both web and mobile, across reboots and signin/sign out. This has the effect of causing my network queue to pause for very long periods of times (hours/days) meaning I can get incoming messages but can't send outgoing messages. Plus it's just constantly spamming my console logs making everything chaotic to diagnose, and I'm sure it can't be good for performance.

Solution:
@danieldoglas has already:

  1. Added logging to understand why ReconnectApp is triggered in the first place
  2. Improved logging to make it easier to view on the server
  3. Updated the network queue to suppress a new ReconnectApp call when one is already outstanding

But there's still more to be done to figure out why it's happening in the first place. I'm adding this to #vip-vsb as HIGH because there are just a bunch of instability bugs that might not seem super important in isolation, but all add up to an unreliable experience.

Copy link

melvin-bot bot commented Mar 12, 2024

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

@danieldoglas
Copy link
Contributor

@danieldoglas
Copy link
Contributor

1 and 3 should be in staging now, 2 was not deployed yet

@danieldoglas danieldoglas added the Reviewing Has a PR in review label Mar 13, 2024
@danieldoglas
Copy link
Contributor

Through the logs, it was possible to check that we're triggering several ReconnectApps because we have an undefined state on our NetInfo check.

Discussion about it here: https://expensify.slack.com/archives/C049HHMV9SM/p1710419891671389?thread_ts=1708910410.150829&cid=C049HHMV9SM

I think this is happening because of this line:

setOfflineStatus(state.isInternetReachable === false);

IsInternetReachable can also return null when the status is unknown. In those cases, we're calling setOfflineStatus with true, which makes the app think that it just went to online status, and so it schedules ReconnectApp/ReconnectToReport/AuthenticatePusher calls.

@danieldoglas
Copy link
Contributor

PR to fix the issue with reconnect was merged.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Mar 19, 2024
@melvin-bot melvin-bot bot changed the title HIGH: Fix dbarrett's infinite ReconnectApp [HOLD for payment 2024-03-26] HIGH: Fix dbarrett's infinite ReconnectApp Mar 19, 2024
Copy link

melvin-bot bot commented Mar 19, 2024

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Mar 19, 2024
Copy link

melvin-bot bot commented Mar 19, 2024

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

If no regressions arise, payment will be issued on 2024-03-26. 🎊

@danieldoglas
Copy link
Contributor

Closing this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering Task Weekly KSv2
Projects
No open projects
Status: CRITICAL
Development

No branches or pull requests

2 participants