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

Sending a message when offline may silently lose the message #1726

Closed
Tracked by #1145
ara4n opened this issue Sep 17, 2023 · 18 comments
Closed
Tracked by #1145

Sending a message when offline may silently lose the message #1726

ara4n opened this issue Sep 17, 2023 · 18 comments
Labels
A-Offline A-Timeline O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect X-Needs-Rust This issue needs a Rust SDK change. It must have a link to a Rust SDK issue

Comments

@ara4n
Copy link
Member

ara4n commented Sep 17, 2023

Steps to reproduce

  1. Sent a message
  2. Spotted that the app said “offline”
  3. Went to room list
  4. Went back
  5. Message entirely lost forever

Outcome

What did you expect?

App should never lose msgs

What happened instead?

Bad queueing; lost msg.

https://github.com/vector-im/element-x-ios-rageshakes/issues/888

Your phone model

No response

Operating system version

No response

Application version

385

Homeserver

No response

Will you send logs?

Yes

@stefanceriu stefanceriu added X-Needs-Rust This issue needs a Rust SDK change. It must have a link to a Rust SDK issue A-Timeline T-Defect S-Critical Prevents work, causes data loss and/or has no workaround O-Occasional Affects or can be seen by some users regularly or most users rarely labels Oct 4, 2023
@jplatte
Copy link
Contributor

jplatte commented Oct 25, 2023

Is this still reproducible? (I think it shouldn't be)

Though keeping unsent messages across app restarts is still TODO.

@kittykat
Copy link
Contributor

@jplatte 100% reproducible for me:

  1. Switch on airplane mode
  2. Go to my test room (created as a room, converted to DM, I am only member)
  3. Send (2) message(s) - I was intending to test sequence
  4. Switch off airplane mode
  5. Watch both messages vanish

@jplatte
Copy link
Contributor

jplatte commented Nov 23, 2023

When you say the messages vanished, do you mean no messages are displayed at all anymore? If yes, a rageshake would be helpful.

@kittykat
Copy link
Contributor

@jplatte The two messages I tried to send from iOS went from being displayed as failed messages while I was in airplane mode to no longer being shown in the timeline at all once I switched off airplane mode. All other historical messages are still shown in the timeline. I can do a screen recording if that would be helpful?

@jplatte
Copy link
Contributor

jplatte commented Nov 23, 2023

No, I don't think a screencast helps. Have you checked that the failed messages haven't "just" moved higher up though?

@kittykat
Copy link
Contributor

@jplatte they move higher on Android, but on iOS, I cannot see them higher up

@jplatte
Copy link
Contributor

jplatte commented Nov 23, 2023

That's even more confusing 🥲

I'll look at the rageshake soon.

@jplatte jplatte self-assigned this Nov 23, 2023
@jplatte
Copy link
Contributor

jplatte commented Dec 7, 2023

Rageshakes didn't contain that much info, matrix-org/matrix-rust-sdk#2887 should help for future rageshakes.

@jplatte jplatte removed their assignment Dec 7, 2023
@timokoesters
Copy link

they move higher on Android, but on iOS, I cannot see them higher up

I cannot reproduce this on Android with the latest nightly, the messages stay where they are. Can you still reproduce it @kittykat ?

@manuroe
Copy link
Member

manuroe commented Jan 8, 2024

EIX 1.5.2 nightly is also better for handling network retries when network is back. It also keeps the messages and moves it into sent failure after the timeout. So, I cannot reproduce anymore the bug in the scenario described at #1726 (comment).
However, moving back to the room list and reenter the room can lose the failed sent messages but it is not 100% reproductible. I do not know if it is a SDK or app only issue. Android seems not impacted by this problem.

@ara4n
Copy link
Member Author

ara4n commented Jan 10, 2024

i just reproduced the 100% vanish bug. The app wasn't showing as offline and i was on 3 bars of 5G:

  • Sent two messages
  • They didn't send, for whatever reason - hollow circle
  • Wait 20s or so
  • Get bored and switch room
  • Switch back again: messges entirely gone. They had not jumped upwards 20 msgs.

Have rageshaked.

@ara4n
Copy link
Member Author

ara4n commented Jan 10, 2024

they move higher on Android, but on iOS, I cannot see them higher up

to be clear, on iOS there is also the related-but-not-the-same bug that unsent messages can end up 'stuck' 20 msgs higher in the timeline: #1279

@Velin92
Copy link
Member

Velin92 commented Jan 10, 2024

Does Android actually store the timeline/room instance even after the room is dismissed? @bmarty
Because iOS deallocates a room in the exact moment it is dismissed, which means that on the Rust side the queue for that room is actually also probably deallocated.
@timokoesters how does this bit work? https://github.com/matrix-org/matrix-rust-sdk/blob/9fd52e5df76f376519a0d27f727acc90144518d6/crates/matrix-sdk-ui/src/timeline/mod.rs#L295-L301
Does the timeline store the message immediately on the actual disk storage as soon as it gets put in the queue?

@timokoesters
Copy link

I tried to reproduce it with this patch, but it looks fine on Android. If someone wants to reproduce it on iOS, try this:

diff --git a/crates/matrix-sdk/src/room/futures.rs b/crates/matrix-sdk/src/room/futures.rs
index ebe2b6f05..8c2297c1f 100644
--- a/crates/matrix-sdk/src/room/futures.rs
+++ b/crates/matrix-sdk/src/room/futures.rs
@@ -36,7 +36,7 @@ use tracing::{debug, Instrument, Span};

 use super::Room;
 use crate::{
-    attachment::AttachmentConfig, utils::IntoRawMessageLikeEventContent, Result,
+    attachment::AttachmentConfig, utils::IntoRawMessageLikeEventContent, Error, Result,
     TransmissionProgress,
 };
 #[cfg(feature = "image-proc")]
@@ -200,8 +200,9 @@ impl<'a> IntoFuture for SendRawMessageLikeEvent<'a> {
                 content,
             );

-            let response = room.client.send(request, None).await?;
-            Ok(response)
+            Err(Error::NoOlmMachine) // Trigger an error
         };

         Box::pin(fut.instrument(tracing_span))

@Velin92
Copy link
Member

Velin92 commented Jan 29, 2024

I tried to reproduce it with this patch, but it looks fine on Android. If someone wants to reproduce it on iOS, try this:

diff --git a/crates/matrix-sdk/src/room/futures.rs b/crates/matrix-sdk/src/room/futures.rs
index ebe2b6f05..8c2297c1f 100644
--- a/crates/matrix-sdk/src/room/futures.rs
+++ b/crates/matrix-sdk/src/room/futures.rs
@@ -36,7 +36,7 @@ use tracing::{debug, Instrument, Span};

 use super::Room;
 use crate::{
-    attachment::AttachmentConfig, utils::IntoRawMessageLikeEventContent, Result,
+    attachment::AttachmentConfig, utils::IntoRawMessageLikeEventContent, Error, Result,
     TransmissionProgress,
 };
 #[cfg(feature = "image-proc")]
@@ -200,8 +200,9 @@ impl<'a> IntoFuture for SendRawMessageLikeEvent<'a> {
                 content,
             );

-            let response = room.client.send(request, None).await?;
-            Ok(response)
+            Err(Error::NoOlmMachine) // Trigger an error
         };

         Box::pin(fut.instrument(tracing_span))

I'll give it a quick try, thanks

@Velin92
Copy link
Member

Velin92 commented Jan 29, 2024

I investigate the issue a bit and this seems to be caused by the fact that we call the unsubscribe function from RoomListItem every time the room is removed from the navigation, which I mean makes total sense.
But as soon as we resubscribe to the room, and the timeline is generated again, but is removing all the unsent items.
Just to test this I tried with @timokoesters 's patch, sent a bunch of failed messages, and every time I was unsubscribing to a room, and resubscribing to it, the timeline was regenerated without the unsent items.
As soon as I stopped calling the unsubscribe function the messages were kept... however this is not ideal ofc.
From what I understand Android is also calling this function when exiting a room, so I really don't get why the issue is present only on iOS.
Maybe tomorrow we can't try to check this together @timokoesters

@Velin92
Copy link
Member

Velin92 commented Feb 21, 2024

@manuroe @timokoesters
Btw opened this issue on the rust sdk repo:
matrix-org/matrix-rust-sdk#3152

This should fix the issue on iOS, and also fix the issue on both platforms that happens when we have unsent messages in the room, but we scroll down in the room list, removing the room from the SS window, which when re-entering deletes the unsent messages

@stefanceriu
Copy link
Member

I believe this has been fixed with introduction of the new message sending queue. Please reopen if you see it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Offline A-Timeline O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect X-Needs-Rust This issue needs a Rust SDK change. It must have a link to a Rust SDK issue
Projects
None yet
Development

No branches or pull requests

7 participants