From 00f3bb5c2a9c117aeeb8623b250e9703d5cab6c1 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 1 Jul 2024 14:38:55 +0200 Subject: [PATCH 1/2] send queue: wake up the sending task after editing an event It could be that the last event in a room's send queue has been marked as wedged. In that case, the task will sleep until it's notified again. If the event is being edited, then nothing would wake up the task; a manual wakeup might be required in that case. The new integration test shows the issue; the last `assert_update` would fail with a timeout before this patch. --- crates/matrix-sdk/src/send_queue.rs | 3 + .../tests/integration/send_queue.rs | 67 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/crates/matrix-sdk/src/send_queue.rs b/crates/matrix-sdk/src/send_queue.rs index 2cfb3dc3e7f..ea282ac07bc 100644 --- a/crates/matrix-sdk/src/send_queue.rs +++ b/crates/matrix-sdk/src/send_queue.rs @@ -899,6 +899,9 @@ impl SendHandle { if self.room.inner.queue.replace(&self.transaction_id, serializable.clone()).await? { trace!("successful edit"); + // Wake up the queue, in case the room was asleep before the edit. + self.room.inner.notifier.notify_one(); + // Propagate a replaced update too. let _ = self.room.inner.updates.send(RoomSendQueueUpdate::ReplacedLocalEvent { transaction_id: self.transaction_id.clone(), diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs index 82fe22df7dc..86fbd6ab20b 100644 --- a/crates/matrix-sdk/tests/integration/send_queue.rs +++ b/crates/matrix-sdk/tests/integration/send_queue.rs @@ -885,6 +885,73 @@ async fn test_edit() { assert!(watch.is_empty()); } +#[async_test] +async fn test_edit_wakes_the_sending_task() { + let (client, server) = logged_in_client_with_server().await; + + // Mark the room as joined. + let room_id = room_id!("!a:b.c"); + + let room = mock_sync_with_new_room( + |builder| { + builder.add_joined_room(JoinedRoomBuilder::new(room_id)); + }, + &client, + &server, + room_id, + ) + .await; + + let q = room.send_queue(); + + let (local_echoes, mut watch) = q.subscribe().await.unwrap(); + + assert!(local_echoes.is_empty()); + assert!(watch.is_empty()); + + mock_encryption_state(&server, false).await; + + let send_mock_scope = Mock::given(method("PUT")) + .and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*")) + .and(header("authorization", "Bearer 1234")) + .respond_with(ResponseTemplate::new(413).set_body_json(json!({ + // From https://spec.matrix.org/v1.10/client-server-api/#standard-error-response + "errcode": "M_TOO_LARGE", + }))) + .expect(1) + .mount_as_scoped(&server) + .await; + + let handle = + q.send(RoomMessageEventContent::text_plain("welcome to my ted talk").into()).await.unwrap(); + + // Receiving an update for the local echo. + let (txn, _) = assert_update!(watch => local echo { body = "welcome to my ted talk" }); + assert!(watch.is_empty()); + + // Let the background task start now. + tokio::task::yield_now().await; + + assert_update!(watch => error { recoverable = false, txn = txn }); + assert!(watch.is_empty()); + + // Now edit the event's content (imagine we make it "shorter"). + drop(send_mock_scope); + mock_send_event(event_id!("$1")).mount(&server).await; + + let edited = handle + .edit(RoomMessageEventContent::text_plain("here's the summary of my ted talk").into()) + .await + .unwrap(); + assert!(edited); + + // Let the server process the message. + assert_update!(watch => edit { body = "here's the summary of my ted talk", txn = txn }); + assert_update!(watch => sent { txn = txn, }); + + assert!(watch.is_empty()); +} + #[async_test] async fn test_abort_after_disable() { let (client, server) = logged_in_client_with_server().await; From 97993e89811fd2fe5167fdd1c35abe21aed950d7 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 1 Jul 2024 15:22:48 +0200 Subject: [PATCH 2/2] timeline: update test expectation after previous bugfix --- .../tests/integration/timeline/edit.rs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs b/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs index 1eb1412b83b..f2e674e1d28 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/edit.rs @@ -215,6 +215,17 @@ async fn test_edit_local_echo() { assert!(timeline_stream.next().now_or_never().is_none()); + // Set up the success response before editing, since edit causes an immediate + // retry (the room's send queue is not blocked, since the one event it couldn't + // send failed in an unrecoverable way). + drop(mounted_send); + Mock::given(method("PUT")) + .and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "event_id": "$1" }))) + .expect(1) + .mount(&server) + .await; + // Let's edit the local echo. let edit_info = item.edit_info().expect("getting the edit info for the local echo"); @@ -240,18 +251,6 @@ async fn test_edit_local_echo() { let edit_message = item.content().as_message().unwrap(); assert_eq!(edit_message.body(), "hello, world"); - // Now, reenable the send queue for that room, and observe the new event being - // sent. - drop(mounted_send); - Mock::given(method("PUT")) - .and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "event_id": "$1" }))) - .expect(1) - .mount(&server) - .await; - - room.send_queue().set_enabled(true); - // Observe the event being sent, and replacing the local echo. assert_let!(Some(VectorDiff::Set { index: 1, value: item }) = timeline_stream.next().await);