Potential fix stuck local echo events at the bottom of the screen

[ariskotsomitopoulos]

This PR will try to fix issues like #516 that report stuck local echo events. The fix not only prevent this from happening again but after a user update its device, stuck local echos will be cleared

Why this is happening?

The issue can happen when a message is SENT but not received back from the /sync.
Until now we use unsignedData.transactionId to determine whether or not the local
event should be deleted on every /sync. However, this is partially correct, lets have a look
at the following scenario:

There is no Internet connection → 10 Messages are sent → cant be sent so the 10 messages are in the queue → internet comes back for 1 second → 3 messages are sent to the homeserver → Internet drops again → No /sync response is triggered | home server can even replied with /sync but never arrived while we are offline

So the state until now is that we have 7 pending events to send and 3 sent but not received them back from /sync
Subsequently, those 3 local messages will not be deleted from the device while there is no transactionId from the /sync

lets continue:
Now lets assume that in the same room another user sent 15 events → We are finally back online! → we will receive the 10 latest events for the room and of course sent the pending 7 messages → Now /sync response will NOT contain the 3 local messages so our events will stuck in the device.

Someone can say, yes but it will come with the rooms/{roomId}/messages while paginating, so the problem will be solved. No that is not the case for two reasons:

1. rooms/{roomId}/messages response do not contain the unsignedData.transactionId so we cannot know which event to delete (in the doc says it should but, it does not)
2. even if transactionId was there, currently we are not deleting it from the pagination

---

While we cannot know when a specific event arrived from the pagination (no transactionId included), after each room /sync a solution is to clear all SENT events, and we are sure that we will receive it from /sync or pagination

Before the fix
https://user-images.githubusercontent.com/60798129/149152854-76a0aff6-5925-4e1c-bbf2-b7c96282869c.mp4

After the fix
https://user-images.githubusercontent.com/60798129/149153207-94e54325-8c2b-49cd-b7d9-39534620ae6c.mp4

[ariskotsomitopoulos]

@bmarty @ganfra @ouchadam we can further discuss about it maybe there are better alternatives

[ouchadam]

great explanation of the scenario! 💯

are the sendingTimelineEvents the local echos?

I'm wondering if we could clean up when we set the sendState to SENT (or is that what this logic is acting on?)

[ariskotsomitopoulos]

Yes and I guess yes we can, but if we do that we will remove the echos at once. So until the user is back online echos will not be visible.

[github-actions]

Unit Test Results

  66 files  ±0    66 suites  ±0   51s ⏱️ -8s
135 tests ±0  135 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
418 runs  ±0  418 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 6f5f773. ± Comparison against base commit c194568.

[bmarty]

According to your great explanation and videos, it seems to work!

I am even wondering if we should remove the existing local echo cleanup

@ganfra can you share your thoughts?

[ganfra]

Thanks for the explanation!
Indeed it will fix the issue, but it can leads to cases where we don't see at our sent events for some time (sync can be slow)
So not sure thats the best move... maybe we should try again to set the event in the last forward chunk instead of being in a dedicated event list, so it won't be stuck at the bottom at least...

[ariskotsomitopoulos]

@bmarty I think it's ok to delete it, there will be no difference without it. However, we have an optimisation logic there that passes the decrypted event at once without the need to decrypt it again. So we should keep the logic there at least

@ganfra To my understanding, yes in a very slow /sync some messages will be delayed but at the end will be with the correct order and delivered. While a lot of people complain about it, and it is really frustrating for the user (can't be undone if that happens). If the only problem is the slow sync and there are no more caveats maybe we can merge it and then try a better solution.

[bmarty]

As discussed IRL, lets merge the PR!