From 3a1eb62c38653cb1a62088c3fa1f9cd5e91c63cc Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 30 Jan 2023 19:27:38 +0100 Subject: [PATCH] refactor(sdk): Expose event sending errors through timeline item MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … instead of through the return value of Timeline::send. --- bindings/matrix-sdk-ffi/src/api.udl | 5 +-- bindings/matrix-sdk-ffi/src/room.rs | 45 +++++++++++-------- bindings/matrix-sdk-ffi/src/timeline.rs | 4 +- .../src/room/timeline/event_item.rs | 9 +++- crates/matrix-sdk/src/room/timeline/inner.rs | 25 ----------- crates/matrix-sdk/src/room/timeline/mod.rs | 13 +++--- crates/matrix-sdk/src/room/timeline/tests.rs | 21 ++++++--- .../tests/integration/room/timeline.rs | 2 +- labs/jack-in/src/app/model.rs | 8 +--- 9 files changed, 61 insertions(+), 71 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/api.udl b/bindings/matrix-sdk-ffi/src/api.udl index 27a92d5c1..f241b0055 100644 --- a/bindings/matrix-sdk-ffi/src/api.udl +++ b/bindings/matrix-sdk-ffi/src/api.udl @@ -259,14 +259,13 @@ interface Room { // Raises an exception if there are no timeline listeners. [Throws=ClientError] void paginate_backwards(PaginationOptions opts); - + [Throws=ClientError] void send_read_receipt(string event_id); - + [Throws=ClientError] void send_read_marker(string fully_read_event_id, string? read_receipt_event_id); - [Throws=ClientError] void send(RoomMessageEventContent msg, string? txn_id); [Throws=ClientError] diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index d6696aa00..bf8c6f8dd 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -295,16 +295,18 @@ impl Room { }) } - pub fn send(&self, msg: Arc, txn_id: Option) -> Result<()> { + pub fn send(&self, msg: Arc, txn_id: Option) { let timeline = match &*self.timeline.read().unwrap() { Some(t) => Arc::clone(t), - None => bail!("Timeline not set up, can't send message"), + None => { + error!("Timeline not set up, can't send message"); + return; + } }; - RUNTIME.block_on(async move { - timeline.send((*msg).to_owned().into(), txn_id.as_deref().map(Into::into)).await?; - Ok(()) - }) + RUNTIME.spawn(async move { + timeline.send((*msg).to_owned().into(), txn_id.as_deref().map(Into::into)).await; + }); } pub fn send_reply( @@ -326,24 +328,27 @@ impl Room { let event_id: &EventId = in_reply_to_event_id.as_str().try_into().context("Failed to create EventId.")?; - RUNTIME.block_on(async move { + let reply_content = RUNTIME.block_on(async move { let timeline_event = room.event(event_id).await.context("Couldn't find event.")?; let event_content = timeline_event .event .deserialize_as::() - .context("Couldn't deserialise event")?; + .context("Couldn't deserialize event")?; let original_message = event_content.as_original().context("Couldn't retrieve original message.")?; - let reply_content = RoomMessageEventContent::text_markdown(msg) - .make_reply_to(original_message, ForwardThread::Yes); + anyhow::Ok( + RoomMessageEventContent::text_markdown(msg) + .make_reply_to(original_message, ForwardThread::Yes), + ) + })?; - timeline.send(reply_content.into(), txn_id.as_deref().map(Into::into)).await?; - - Ok(()) - }) + RUNTIME.spawn(async move { + timeline.send(reply_content.into(), txn_id.as_deref().map(Into::into)).await; + }); + Ok(()) } pub fn edit( @@ -365,7 +370,7 @@ impl Room { let event_id: &EventId = original_event_id.as_str().try_into().context("Failed to create EventId.")?; - RUNTIME.block_on(async move { + let edited_content = RUNTIME.block_on(async move { let timeline_event = room.event(event_id).await.context("Couldn't find event.")?; let event_content = timeline_event @@ -384,11 +389,13 @@ impl Room { let mut edited_content = RoomMessageEventContent::text_markdown(new_msg); edited_content.relates_to = Some(Relation::Replacement(replacement)); + Ok(edited_content) + })?; - timeline.send(edited_content.into(), txn_id.as_deref().map(Into::into)).await?; - - Ok(()) - }) + RUNTIME.spawn(async move { + timeline.send(edited_content.into(), txn_id.as_deref().map(Into::into)).await; + }); + Ok(()) } /// Redacts an event from the room. diff --git a/bindings/matrix-sdk-ffi/src/timeline.rs b/bindings/matrix-sdk-ffi/src/timeline.rs index aae2a3536..a079b44d4 100644 --- a/bindings/matrix-sdk-ffi/src/timeline.rs +++ b/bindings/matrix-sdk-ffi/src/timeline.rs @@ -177,7 +177,7 @@ pub enum EventSendState { NotSendYet, /// The local event has been sent to the server, but unsuccessfully: The /// sending has failed. - SendingFailed, + SendingFailed { error: String }, /// The local event has been sent successfully to the server. Sent { event_id: String }, } @@ -188,7 +188,7 @@ impl From<&matrix_sdk::room::timeline::EventSendState> for EventSendState { match value { NotSentYet => Self::NotSendYet, - SendingFailed => Self::SendingFailed, + SendingFailed { error } => Self::SendingFailed { error: error.to_string() }, Sent { event_id } => Self::Sent { event_id: event_id.to_string() }, } } diff --git a/crates/matrix-sdk/src/room/timeline/event_item.rs b/crates/matrix-sdk/src/room/timeline/event_item.rs index f73c981e2..5a9e64665 100644 --- a/crates/matrix-sdk/src/room/timeline/event_item.rs +++ b/crates/matrix-sdk/src/room/timeline/event_item.rs @@ -52,6 +52,8 @@ use ruma::{ OwnedTransactionId, OwnedUserId, TransactionId, UserId, }; +use crate::Error; + /// An item in the timeline that represents at least one event. /// /// There is always one main event that gives the `EventTimelineItem` its @@ -209,13 +211,16 @@ impl EventTimelineItem { } /// This type represents the "send state" of a local event timeline item. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug)] pub enum EventSendState { /// The local event has not been sent yet. NotSentYet, /// The local event has been sent to the server, but unsuccessfully: The /// sending has failed. - SendingFailed, + SendingFailed { + /// Details about how sending the event failed. + error: Arc, + }, /// The local event has been sent successfully to the server. Sent { /// The event ID assigned by the server. diff --git a/crates/matrix-sdk/src/room/timeline/inner.rs b/crates/matrix-sdk/src/room/timeline/inner.rs index b3ad25ebf..139408c4e 100644 --- a/crates/matrix-sdk/src/room/timeline/inner.rs +++ b/crates/matrix-sdk/src/room/timeline/inner.rs @@ -14,7 +14,6 @@ use matrix_sdk_base::{ locks::Mutex, }; use ruma::{ - api::client::message::send_message_event::v3::Response as SendMessageEventResponse, events::{ fully_read::FullyReadEvent, relation::Annotation, AnyMessageLikeEventContent, AnySyncTimelineEvent, @@ -151,30 +150,6 @@ impl TimelineInner

{ .handle_event(kind); } - /// Handle the response returned by the server when a local event has been - /// sent. - pub(super) fn handle_local_event_send_response( - &self, - txn_id: &TransactionId, - response: crate::error::Result, - ) -> crate::error::Result<()> { - match response { - Ok(response) => { - self.update_event_send_state( - txn_id, - EventSendState::Sent { event_id: response.event_id }, - ); - - Ok(()) - } - Err(error) => { - self.update_event_send_state(txn_id, EventSendState::SendingFailed); - - Err(error) - } - } - } - /// Update the send state of a local event represented by a transaction ID. /// /// If no local event is found, a warning is raised. diff --git a/crates/matrix-sdk/src/room/timeline/mod.rs b/crates/matrix-sdk/src/room/timeline/mod.rs index 18055d981..19fb2aa5b 100644 --- a/crates/matrix-sdk/src/room/timeline/mod.rs +++ b/crates/matrix-sdk/src/room/timeline/mod.rs @@ -370,11 +370,7 @@ impl Timeline { /// [`MessageLikeUnsigned`]: ruma::events::MessageLikeUnsigned /// [`SyncMessageLikeEvent`]: ruma::events::SyncMessageLikeEvent #[instrument(skip(self, content), fields(room_id = ?self.room().room_id()))] - pub async fn send( - &self, - content: AnyMessageLikeEventContent, - txn_id: Option<&TransactionId>, - ) -> Result<()> { + pub async fn send(&self, content: AnyMessageLikeEventContent, txn_id: Option<&TransactionId>) { let txn_id = txn_id.map_or_else(TransactionId::new, ToOwned::to_owned); self.inner.handle_local_event(txn_id.clone(), content.clone()).await; @@ -383,7 +379,12 @@ impl Timeline { let room = Joined { inner: self.room().clone() }; let response = room.send(content, Some(&txn_id)).await; - self.inner.handle_local_event_send_response(&txn_id, response) + + let send_state = match response { + Ok(response) => EventSendState::Sent { event_id: response.event_id }, + Err(error) => EventSendState::SendingFailed { error: Arc::new(error) }, + }; + self.inner.update_event_send_state(&txn_id, send_state); } } diff --git a/crates/matrix-sdk/src/room/timeline/tests.rs b/crates/matrix-sdk/src/room/timeline/tests.rs index e92ea523d..11e651c9a 100644 --- a/crates/matrix-sdk/src/room/timeline/tests.rs +++ b/crates/matrix-sdk/src/room/timeline/tests.rs @@ -14,9 +14,12 @@ //! Unit tests (based on private methods) for the timeline API. -use std::sync::{ - atomic::{AtomicU32, Ordering::SeqCst}, - Arc, +use std::{ + io, + sync::{ + atomic::{AtomicU32, Ordering::SeqCst}, + Arc, + }, }; use assert_matches::assert_matches; @@ -58,7 +61,7 @@ use super::{ EventTimelineItem, MembershipChange, Profile, TimelineInner, TimelineItem, TimelineItemContent, VirtualTimelineItem, }; -use crate::room::timeline::event_item::EventSendState; +use crate::{room::timeline::event_item::EventSendState, Error}; static ALICE: Lazy<&UserId> = Lazy::new(|| user_id!("@alice:server.name")); static BOB: Lazy<&UserId> = Lazy::new(|| user_id!("@bob:other.server")); @@ -388,20 +391,24 @@ async fn remote_echo_full_trip() { { let item = assert_matches!(stream.next().await, Some(VecDiff::Push { value }) => value); let event = item.as_event().unwrap().as_local().unwrap(); - assert_eq!(event.send_state, EventSendState::NotSentYet); + assert_matches!(event.send_state, EventSendState::NotSentYet); } // Scenario 2: The local event has not been sent to the server successfully, it // has failed. In this case, there is no event ID. { - timeline.inner.update_event_send_state(&txn_id, EventSendState::SendingFailed); + let some_io_error = Error::Io(io::Error::new(io::ErrorKind::Other, "this is a test")); + timeline.inner.update_event_send_state( + &txn_id, + EventSendState::SendingFailed { error: Arc::new(some_io_error) }, + ); let item = assert_matches!( stream.next().await, Some(VecDiff::UpdateAt { value, index: 1 }) => value ); let event = item.as_event().unwrap().as_local().unwrap(); - assert_eq!(event.send_state, EventSendState::SendingFailed); + assert_matches!(event.send_state, EventSendState::SendingFailed { .. }); } // Scenario 3: The local event has been sent successfully to the server and an diff --git a/crates/matrix-sdk/tests/integration/room/timeline.rs b/crates/matrix-sdk/tests/integration/room/timeline.rs index 88c4631eb..0bb890a24 100644 --- a/crates/matrix-sdk/tests/integration/room/timeline.rs +++ b/crates/matrix-sdk/tests/integration/room/timeline.rs @@ -194,7 +194,7 @@ async fn echo() { assert_eq!(text.body, "Hello, World!"); // Wait for the sending to finish and assert everything was successful - send_hdl.await.unwrap().unwrap(); + send_hdl.await.unwrap(); let sent_confirmation = assert_matches!( timeline_stream.next().await, diff --git a/labs/jack-in/src/app/model.rs b/labs/jack-in/src/app/model.rs index 435a007c8..5a9b93200 100644 --- a/labs/jack-in/src/app/model.rs +++ b/labs/jack-in/src/app/model.rs @@ -7,7 +7,7 @@ use std::{ops::Deref, time::Duration}; use futures::executor::block_on; use matrix_sdk::{ruma::events::room::message::RoomMessageEventContent, Client}; use tokio::sync::mpsc; -use tracing::{error, info, warn}; +use tracing::warn; use tuirealm::{ props::{Alignment, Borders, Color}, terminal::TerminalBridge, @@ -215,11 +215,7 @@ impl Update for Model { if let Some(tl) = self.sliding_sync.room_timeline.lock_ref().deref() { block_on(async move { // fire and forget - match tl.send(RoomMessageEventContent::text_plain(m).into(), None).await - { - Ok(_r) => info!("Message send"), - Err(e) => error!("Sending message failed: {e}"), - } + tl.send(RoomMessageEventContent::text_plain(m).into(), None).await; }); } else { warn!("asked to send message, but no room is selected");