When sending new chats in Desktop, an old message persists at the bottom

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open the desktop app
2. Send some messages

Expected Result:

New messages should always display at the bottom.

Actual Result:

When sending new chats in Desktop, an old message persists at the bottom.

Workaround:

Hard refreshing solves the problem.

Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App ✔️
Mobile Web

Version Number: 1.0.68-4

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:
I was unable to reproduce the issue.

2021-06-15_17-56-43.mp4

Logs:

Uncaught (in promise) Error: API is offline
    at app-588cbfa….bundle.js:141
    at app-588cbfa….bundle.js:141
4
app-588cbfa….bundle.js:141 
[LOCAL_NOTIFICATION] No notification because comment is from the currently logged in user
2
app-588cbfa….bundle.js:141 
[LOCAL_NOTIFICATION] Creating notification
8
app-588cbfa….bundle.js:141 
[LOCAL_NOTIFICATION] No notification because comment is from the currently logged in user
app-588cbfa….bundle.js:141 
[LOCAL_NOTIFICATION] Creating notification
app-588cbfa….bundle.js:141 
[LOCAL_NOTIFICATION] No notification because it was a comment for the current report
DevTools failed to load SourceMap: Could not parse content for app://-/app-588cbfa….bundle.js.map: Unexpected end of JSON input
DevTools failed to load SourceMap: Could not parse content for app://-/pdf.worker.entry-b22bab8….bundle.worker.js.map: Unexpected end of JSON input
app-588cbfa….bundle.js:141 
[Violation] 'setTimeout' handler took 154ms
app-588cbfa….bundle.js:141 
[Violation] 'setTimeout' handler took 106ms
app-588cbfa….bundle.js:141 
[LOCAL_NOTIFICATION] No notification because comment is from the currently logged in user
app-588cbfa….bundle.js:141 
[LOCAL_NOTIFICATION] No notification because comment is from the currently logged in user
app-588cbfa….bundle.js:141 
[LOCAL_NOTIFICATION] No notification because comment is from the currently logged in user
app-588cbfa….bundle.js:141 
[LOCAL_NOTIFICATION] No notification because comment is from the currently logged in user

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @mallenexpensify https://expensify.slack.com/archives/C01GTK53T8Q/p1623805090370500

When sending new chats in Desktop, an old message persists at the bottom.

[MelvinBot]

Triggered auto assignment to @timszot (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MelvinBot]

Triggered auto assignment to @JmillsExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[rdjuric]

Nice find! this one was really hard to reproduce.

Proposal

This happens when we have a ReportAction that failed to be delivered to our API (we can see the network error on the log). It's hard to reproduce because normally this ReportAction is sent to our NetworkRequestQueue and retried, but that doesn't happen in all cases. (There's an ongoing conversation about refactoring this queue at #3608)

One of the easiest reproductions paths that I found is:

1. Send a message A and another message B while offline
2. Go online for a brief moment, just enough to trigger the isLoadingAfterReconnect but not enough for the API calls of our queue to complete, and then go offline again
3. Send another message C
4. Notice that A and B are not in our NetworkRequestQueue anymore (just C is) , but also haven't been sent yet.
5. Go online. Only C is sent, but it goes above both A and B, who are now stuck at the bottom of the chat.

The reason the messages get stuck in the bottom is because in our ReportActionsView we order our ReportActions by their sequenceNumber
https://github.com/Expensify/Expensify.cash/blob/bb7d3d3f797d0463bf12ddfacdaf25d3ff181ccb/src/pages/home/report/ReportActionsView.js#L253-L266

and when we optimistically save a new ReportAction, we pass the key that we use to save them on storage as our temporary sequenceNumber. Since this number is purposefully long, when we sort our ReportActions these messages will always end up in the last place.
https://github.com/Expensify/Expensify.cash/blob/bb7d3d3f797d0463bf12ddfacdaf25d3ff181ccb/src/libs/actions/Report.js#L1003-L1004
https://github.com/Expensify/Expensify.cash/blob/bb7d3d3f797d0463bf12ddfacdaf25d3ff181ccb/src/libs/actions/Report.js#L1028-L1030

So our stuck messages are optimistically saved messages with really long sequenceNumbers that are not being corrected by the API (caused at least in this reproduction path by not being on the retry queue anymore)

We can solve this on the front by changing the sequenceNumber of our optimistically saved ReportActions to be the highestSequenceNumber of the Report. By doing this, when the messages get stuck they will retain the order in which they were sent, and I also think it would help reduce jumps in more normal scenarios.

While I think that we really should make our frontend fail more gracefully here, this issue really highlights the importance of #3608

[JmillsExpensify]

Sorry @rdjuric I missed this one last week. Adding the Exported label so a colleague can assess your proposal and then I'll get a job posting going on Upwork.

[MelvinBot]

Triggered auto assignment to @NikkiWines (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[JmillsExpensify]

Done! Upwork posting is here for reference when Nikki approves your proposal. https://www.upwork.com/jobs/~0135a8456084eb691e

[NikkiWines]

This proposal seems good to me, updating the sequenceNumbers so they align with the sent order sounds like a good solution.

cc: @marcaaron and @tgolen, in case I'm off base with approving this since I'm not the most familiar with this logic. As @rdjuric mentioned in their proposal, this relates to #3608 and the potential refactor for the network request queue