-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix: Infinite loader on login #4397
Conversation
I'm still able to reproduce the issue while testing this branch on web:
|
Thanks, @roryabraham for the feedback. I will try to reproduce this and update it here. |
I found out what is happening. Even I unpause the queue. Old requests that are being retried will pause it again because those requests are checking the old Line 54 in c2d21c0
Here we check auth token from request data. The token is only set after call to App/src/libs/actions/Session.js Line 172 in 36b86e9
Thus I checked we can force this request as well. It's safe to use Pass this check Line 133 in c2d21c0
We can update this Line 20 in 36b86e9
or We unpause the queue after this App/src/libs/actions/Session.js Line 172 in 36b86e9
As Token is set by now. But I have little doubt that should I wait for I have found a way to reproduce this reliably and I will post both videos here (fix and issue). |
ISSUEissue-loader.mp4FIXissue-loader-fix.mp4Seems like the video got corrupted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work on web, can no longer reproduce the issue, even with your updated steps.
@AndrewGable All yours. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for hopping on this one so quickly!
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
CP'ing this so it can be re-tested on the deploy blocker checklist |
Fix: Infinite loader on login (cherry picked from commit f693233)
🚀 Deployed to staging in version: 1.0.82-5🚀
|
🚀 Deployed to production in version: 1.0.82-7🚀
|
🚀 Deployed to staging in version: 1.0.82-8🚀
|
🚀 Deployed to production by @francoisl in version: 1.0.83-1 🚀
|
Details
After login,
We call many APIs asynchronously. but it is possible that users can logout before one of these requests is about to make. In this case, Queue will pause as Auth token is not present.
I checked that we pause it
App/src/libs/API.js
Line 62 in e511b1f
Now when you enter the password and send that request, the First auth token will be received as this overrides the paused queue. but I think the second call to createTemporaryLogin is not going through. Only after this call is completed loader is stopped and navigation happens.
In Code, the queue is resumed when re-authentication happens but we don't do this on sign up. So if we call
Network.unpauseRequestQueue();
inApp/src/libs/actions/Session.js
Lines 207 to 225 in e511b1f
There was one similar issue like this where the user was blocked on the first stage where the user enters the email which I fixed by forcefully sending that request even if the queue is paused as that did not need auth token but now this issue is happening on the password step which does require Auth token.
Fixed Issues
$ Fixes #4221
Tests | QA Steps
ctrl+shit+T
to restore the tab. As soon as you see the App window. Quickly log out before data sync is complete. You should be logging out the USER A.Tested On
Screenshots
I am not able to reproduce the breaking behavior consistently. If someone can help me do that I will try to show that being fixed here.
Web
Mobile Web
Desktop
iOS
Android