You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[HOLD - for payment on July 14] Chat - Loading spinner appears after log in with different account #3838
Further investigation shows that the report draft comment becomes null in Onyx, and lodashGet only uses the default param ('') when the key is undefined.
The below console logs came from this code that I added:
changed the title [-] Android - Chat - Loading spinner appears after log in with different account[/-][+]Chat - Loading spinner appears after log in with different account[/+]on Jul 1, 2021
@Beamanator While fixing the regression does not solve this problem as I was already facing the same problem before I pushed the PR. But this time I found the root cause of it. This is little involved than it looks.
Explanation
We have a network Queue that process all the request for us. And out of all the requests, there are a few that don't require auth token but others need Auth token.
Usually, the authentication request is the one that does not need auth token.
When user login to the chat who has a long list of unread chat or let's say few IOU reports which is not yet synced. After login, we call APIs to sync this history. But if we immediately log out that user, there is a case where a few requests will continue to send and thus our queue will detect that and pause the Queue processing as the user is not logined.
When we enter an email to the login form, we send a request to get the Account status to get some important information. Due to the queue is currently paused, this request will start looping and will never complete.
I tried to find all references to this request to make sure that we do not break the app. So as result, I found that this request is only used at the time of login when we have to get the account info. So this makes a good point to bypass the token check as we will never have the token on the Login page and this request does not need the token.
changed the title [-]Chat - Loading spinner appears after log in with different account[/-][+][HOLD - for payment on July 14] Chat - Loading spinner appears after log in with different account[/+]on Jul 7, 2021
Activity
MelvinBot commentedon Jun 30, 2021
Triggered auto assignment to @Beamanator (
Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.isagoico commentedon Jun 30, 2021
This is not reproducible every time. I was able to reproduce 2 out of 5 times on my device.
Beamanator commentedon Jul 1, 2021
Hmm when I follow the steps in the OP, I even get a blank white screen in web (staging), with this error message in the console:
Beamanator commentedon Jul 1, 2021
Further investigation shows that the report draft comment becomes
nullin Onyx, andlodashGetonly uses the default param ('') when the key isundefined.The below console logs came from this code that I added:
[-] Android - Chat - Loading spinner appears after log in with different account[/-][+]Chat - Loading spinner appears after log in with different account[/+]Beamanator commentedon Jul 1, 2021
Updated title & OP to show this issue isn't specific to the
AndroidplatformBeamanator commentedon Jul 1, 2021
@isagoico I can reproduce this issue even in production web & it seems to be something that can be fixed externally so I'm going to make this external
MelvinBot commentedon Jul 1, 2021
Triggered auto assignment to @SofiedeVreese (
External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.rdjuric commentedon Jul 1, 2021
Proposal
We can modify our saveReportComment to avoid merging null
draftCommentsto Onyx by checking if(comment === null)before doing the merge.6 remaining items
aliabbasmalik8 commentedon Jul 2, 2021
PROPOSAL:
Need to add a validation check like this.
https://github.com/Expensify/Expensify.cash/blob/870c3db1675bb294c8961a8db88bc1ffa276dea9/src/libs/OptionsListUtils.js#L172-L174
https://github.com/Expensify/Expensify.cash/blob/870c3db1675bb294c8961a8db88bc1ffa276dea9/src/libs/OptionsListUtils.js#L300-L302
Thanks
parasharrajat commentedon Jul 2, 2021
This seems like a regression from my PR. Fix will be applied shortly. #3793
parasharrajat commentedon Jul 2, 2021
@Beamanator While fixing the regression does not solve this problem as I was already facing the same problem before I pushed the PR. But this time I found the root cause of it. This is little involved than it looks.
Explanation
We have a network Queue that process all the request for us. And out of all the requests, there are a few that don't require auth token but others need Auth token.
Usually, the authentication request is the one that does not need auth token.
When user login to the chat who has a long list of unread chat or let's say few IOU reports which is not yet synced. After login, we call APIs to sync this history. But if we immediately log out that user, there is a case where a few requests will continue to send and thus our queue will detect that and pause the Queue processing as the user is not logined.
When we enter an email to the login form, we send a request to get the Account status to get some important information. Due to the queue is currently paused, this request will start looping and will never complete.
As a result, the loader is shown on the button.
Fix
WE have to force send the request to Get_AccountStatus even if the queue is paused. for this, we just need to append a param in data
forceNetworkRequest: trueat https://github.com/Expensify/Expensify.cash/blob/bfc14ceddf75fa5eb024fdc2ce24430cd37c5d49/src/libs/actions/Session.js#L108.I tried to find all references to this request to make sure that we do not break the app. So as result, I found that this request is only used at the time of login when we have to get the account info. So this makes a good point to bypass the token check as we will never have the token on the Login page and this request does not need the token.
This Fixes the original issue.
Video explaining the issue in nutshell.
loop-login.mp4
chiragsalian commentedon Jul 2, 2021
Yeah, your proposal LGTM @parasharrajat. Feel free to push out a PR and suggest the same proposal in upwork so that you can be paid for your work 👍
SofiedeVreese commentedon Jul 5, 2021
Ok I accepted your proposal in Upwork @parasharrajat !
isagoico commentedon Jul 5, 2021
Issue reproducible during KI retests.
[-]Chat - Loading spinner appears after log in with different account[/-][+][HOLD - for payment on July 14] Chat - Loading spinner appears after log in with different account[/+]SofiedeVreese commentedon Jul 7, 2021
Payment scheduled for July 14.
SofiedeVreese commentedon Jul 14, 2021
@parasharrajat has been paid, the contract has been completed, and the Upwork post has been closed.
Closing issue.