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

Persistently store failed to send messages #3152

Closed
Velin92 opened this issue Feb 21, 2024 · 8 comments
Closed

Persistently store failed to send messages #3152

Velin92 opened this issue Feb 21, 2024 · 8 comments

Comments

@Velin92
Copy link
Member

Velin92 commented Feb 21, 2024

In Element X unsent messages stay in the timeline as long as the room is in the SS window, as soon as the room is unsubscribed from it, and the timeline is reloaded through the next sync loop, the unsent messages are completely gone, not offering the user a second chance to resend them if they are having connection issues.

As of right now they are just cached in the timeline, but not stored persistently. The idea is that they should be stored persistently, and always displayed at the bottom of the timeline, when its reloaded (even if it has been updated).

@bnjbvr
Copy link
Member

bnjbvr commented Feb 22, 2024

Thanks for opening an issue. Do we want it to be restored while the app has been running, or even after the app has been restarted? (Hint: the former is rather easy, the latter is more involved.)

@Velin92
Copy link
Member Author

Velin92 commented Feb 22, 2024

Thanks for opening an issue. Do we want it to be restored while the app has been running, or even after the app has been restarted? (Hint: the former is rather easy, the latter is more involved.)

I think at least while the app has been running is sufficient for now, maybe we can explore the latter in the future. The important thing for now is to allow users to not lose them while navigating rooms in the same app run.

@bnjbvr
Copy link
Member

bnjbvr commented Feb 22, 2024

Owait, you mean messages that haven't been sent, i.e. that are only in the text editor? If so, the Rust SDK doesn't manage this data, so it can't really memorize it, until it does manage it, and that really sounds like a UI-related responsibility to me? (Otherwise we'd end up with another API to let the SDK know that a message has been erased too)

@Velin92
Copy link
Member Author

Velin92 commented Feb 22, 2024

Unsent messages as messages that have been queued and sent, have failed to be received by the server for some network error (like bad connection) and only exist as a local message that need to be either resent or cancelled.
Let me actually rename the issue to "Persistently store failed to send messages"

@Velin92 Velin92 changed the title Persistently store unsent messages Persistently store failed to send messages Feb 22, 2024
@timokoesters
Copy link
Contributor

Here's some information that might help when implementing this:

matrix_sdk_ui::timeline::Timeline::send creates local echo and then tries to send event

  1. handle_local_event adds local echo at end of timeline, but timelines are deleted when the room is unloaded. We should save it somewhere and at some point permanently in the db, probably in a new table.
  2. When reloading the timeline, we need to remember to append those saved unsent events from (1)
  3. When the message finally arrives, we need to remove the event from the db again and also remove the local echo.

Questions:

  • Why does element x ios throw away the timeline when you leave a room
    • Element android still keeps it if you don't scroll away in the room list
    • timeline.clear() is not called according to some tests.
    • Shouldn't android and ios behave the same because that's all handled by the sdk?

@bnjbvr
Copy link
Member

bnjbvr commented Feb 26, 2024

We have a room cache in the RoomListService, so the timeline (and its failed messages, as it owns it) may be kept alive if it's still in the cache, and not otherwise.

We should save it somewhere and at some point permanently in the db, probably in a new table.

As said in #3152 (comment), saving in memory would probably be sufficient to start with, but that doesn't fundamentally change the requirements here (still necessary to clean up after sending succeeded).

I suppose if we really want to save them in a database, we'd need to think which is appropriate: definitely not the crypto store, and the state store or the event cache store would be ok if they lived in the UI crate (otherwise, we'd be putting UI-specific functionality in the main crate, which is bad tightly coupling between the two components).

@frebib
Copy link
Contributor

frebib commented Feb 26, 2024

I do want to chime in here and give a user perspective.
This is one of those issues that has burnt me time and time again. If I ever send a message, I don't expect it to ever disappear (which is also what Matthew said in the linked issue). There's nothing more frustrating than having to re-type a message, particularly on a mobile device, just because you went into a train tunnel. My expectation would be that it'd persist at the bottom of the timeline forever even through app restarts. Bonus points if I don't even have to manually retry sending (I know there's a separate issue for that one somewhere else)

@bnjbvr
Copy link
Member

bnjbvr commented May 31, 2024

Replaced by #3361, which is about implementing the in-memory store and on-disk storage 🥳

@bnjbvr bnjbvr closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants