LHN does not update properly when calling ReconnectApp #33771
Comments
|
Triggered auto assignment to @MitchExpensify ( |
Bug0 Triage Checklist (Main S/O)
|
|
What steps should I take to reproduce this @iwiznia ? |
|
Probably it will be hard to reproduce since you need to miss x number of updates in order to trigger this part:
It will be easier on dev where we can just comment out some part of the code (not sure where exactly) and see what happens. |
marcaaron
added
Internal
|
Job added to Upwork: https://www.upwork.com/jobs/~01ea2b6c10b081507e |
|
Triggered auto assignment to Contributor Plus for review of internal employee PR - @situchan ( |
Different way to say the same thing while in focus mode, chats end up with stale data because an update from reliable updates queue is never received. If we fallback to OpenApp in this case we will send back only the reports for the focus mode query. Not the reports which the client has locally and something about them changed. What is the best way to fix this? As discussed, we think something like a We will need to make sure that still works if you have pending actions. I think it would be fine as stuff like |
This makes sense to me. An alternative I can think of is to send the reportIDs from the client, like we did with policyIds, but I like that less and I guess it scales worse. |
|
Ok, on a first pass - I cannot reproduce this problem and it does not seem like the current theory is the whole story. If you look at the code for api.php AppInit.php (defaults to false) Which means we should be syncing with the full list when GetOnyxUpdates throws the 423 jsonCode. And when I test locally I see everything sync up fine 🤔 |
|
To reproduce and have |
|
No I just did this: diff --git a/lib/AppInit.php b/lib/AppInit.php
index 52612dbdca9..049c39521f6 100644
--- a/lib/AppInit.php
+++ b/lib/AppInit.php
@@ -375,6 +375,7 @@ class AppInit
/** @psalm-suppress InvalidGlobal */
global $trackMetric;
try {
+ throw new ExpError('asdf', 'asdf', jsonCode: 423);
$response = Auth::getOnyxUpdates($authToken, $updateIDFrom, $updateIDTo);
// GetOnyxUpdates will return the format array{accountID => array{updates, previousUpdateID, lastUpdateID}}. |
|
This leaves me to think this possibly has to do with the Reminder: This pre-dates the "reliable updates" stuff where we will send only the new reports with based on the report actions that were last modified. That is very much based on the assumption that you would have everything that comes before this most recent update. Also, note this logic does not change based on whether the priority mode filter is enabled or not... so again, I don't think this has to do with focus mode. However, when you switch to Most recent mode we do a hard sync of the reports list (and do not pass the Maybe... somehow... a more recent report action sneaks in before the missing updates are fetched + all the updates fail to fetch? Something like:
Not sure exactly how it can happen, but that's my best improved theory. |
|
Few more logs (not sure if they help): 2023-12-29 14:42:32 707 Message created by Garret here: https://www.expensify.com/_devportal/tools/logSearch/#query=request_id:(%2283d2cf69cc627200-LHR%22)+AND+timestamp:[2023-12-29T13:42:32.708Z+TO+2023-12-29T15:42:32.708Z]&index=logs_expensify-026033 2023-12-29 14:42:50 826 @JmillsExpensify calls ReconnectApp here unsuccessfully: The important part being: That's the same as the message from Garrett. The time of that log line was This is the last successful time Jason got Onyx updates:
All times in between either his auth token was expired or the 423 was triggered. The trail is pretty hard to follow. All I can advise for now is that we should either:
|
|
I don't think this is limited to focus mode.
ScreenshotsHere is the push notification on mobile:
Here's what I saw in web after receiving the push notification on mobile:
And here's what I saw in web after clicking to another chat then clicking back:
|
|
@sakluger looks a bit different than this issue Can't say with certainty but it seems like maybe:
|
|
Bringing the focus back to the main theory we have for this issue… Theory:
Is there some way to test it? I can't really think of any way. Took a quick look at the |
|
Not overdue, Melvin |
|
Impossible to say without looking at a specific incident and scanning through the logs (which is very time consuming) |
|
FWIW I think we should try to pop this in #vip-vsb or something. There is clearly some sort of hole that needs to be patched and a ton of context about how it might be happening. We should patch it so that the unexpected stuff we have observed can't happen. |
marcaaron
changed the title
|
Adding some more logs to help diagnose this. Also proposed to temporarily stop passing the |
|
@MitchExpensify @situchan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks! |
|
@MitchExpensify, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick! |
|
@marcaaron is LHN not updating report to read after report is opened by clicking a notification part of this issue? #34698 Not on focus mode |
|
@mallenexpensify Do you think this fits in wave 5 like #34273 does? |
|
@sonialiap possibly - but doesn't seem related to me. That one seems to imply something went wrong with marking chats as read. |
|
The PR hit production 2 days ago, is anyone witnessing any changes/improvements? I don't think I have for the issue I'm tracking for 'LHN not updating for chats in #social'. @MitchExpensify I'm unsure if this is Wave 5, and/or if there's more work left to do. |
|
|
|
For #social, LHN still isn't updating with new chats, been testing all day. Scott and Flo both reported not getting new chats too (which I consider the same and #social not uploading cuz the chat isn't updating in LHN, if it's a DM or a room) |
|
@MitchExpensify, @situchan Still overdue 6 days?! Let's take care of this! |
@quinthar, do you agree here? |
|
@MitchExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this? |
|
Is anyone still experiencing this? @iwiznia @marcaaron @mallenexpensify |
|
@MitchExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this? |
|
Going to close this in the hope that #33372 fixed it. Please feel free to reopen if anyone is experiencing it |
Coming from #33372
Problem
Left hand nav is outdated (in focus mode?) when calling ReconnectApp, which should never happen since after calling ReconnectApp everything should be up to date.
You can see logs from the App here and the call to ReconnectApp here and see that it failed to use the updates and instead called auth's
OpenAppMy very wild guess is that for focus mode, it is never clearing the existing data in onyx, so a chat that was showing (since it was unread) would not get explicitly removed when OpenApp does not send it back.
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: