Skip to content
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

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

Merged
merged 2 commits into from
Jan 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/516.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix for stuck local event messages at the bottom of the screen
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
} else {
handlingStrategy.data.mapWithProgress(reporter, InitSyncStep.ImportingAccountJoinedRooms, 0.6f) {
handleJoinedRoom(realm, it.key, it.value, insertType, syncLocalTimeStampMillis, aggregator)
}.also {
fixStuckLocalEcho(it)
}
}
}
Expand Down Expand Up @@ -478,4 +480,47 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle

return result
}

/**
* There are multiple issues like #516 that report stuck local echo events
* at the bottom of each room timeline.
*
* That 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] --> [The 10 messages are in the queue] -->
* [Internet comes back for 1 second] --> [3 messages are sent] --> [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 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
* 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
* we clear all SENT events, and we are sure that we will receive it from /sync or pagination
*/
private fun fixStuckLocalEcho(rooms: List<RoomEntity>) {
rooms.forEach { roomEntity ->
roomEntity.sendingTimelineEvents.filter { timelineEvent ->
timelineEvent.root?.sendState == SendState.SENT
}.forEach {
roomEntity.sendingTimelineEvents.remove(it)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

it.deleteOnCascade(true)
}
}
}
}