Merge pull request #1154 from matrix-org/stefan/sessionVerificationFixes

Sliding Sync FFI and DevEx fixes

bindings/matrix-sdk-ffi/src/client.rs

@@ -13,7 +13,7 @@ use matrix_sdk::{
             session::get_login_types,
             sync::sync_events::v3::Filter,
         },
-        events::room::MediaSource,
+        events::{room::MediaSource, AnyToDeviceEvent},
         serde::Raw,
         TransactionId, UInt,
     },
@@ -49,11 +49,29 @@ pub struct Client {
 
 impl Client {
     pub fn new(client: MatrixClient, state: ClientState) -> Self {
+        let session_verification_controller: Arc<
+            matrix_sdk::locks::RwLock<Option<SessionVerificationController>>,
+        > = Default::default();
+        let ctrl = session_verification_controller.clone();
+
+        client.add_event_handler(move |ev: AnyToDeviceEvent| {
+            let ctrl = ctrl.clone();
+            async move {
+                if let Some(session_verification_controller) = &*ctrl.clone().read().await {
+                    session_verification_controller.process_to_device_message(ev).await;
+                } else {
+                    tracing::warn!(
+                        "received to-device message, but verification controller isn't ready"
+                    );
+                }
+            }
+        });
+
         Client {
             client,
             state: Arc::new(RwLock::new(state)),
             delegate: Arc::new(RwLock::new(None)),
-            session_verification_controller: Arc::new(matrix_sdk::locks::RwLock::new(None)),
+            session_verification_controller,
         }
     }
 
@@ -234,7 +252,6 @@ impl Client {
             {
                 return Ok(Arc::new(session_verification_controller.clone()));
             }
-
             let user_id = self.client.user_id().context("Failed retrieving current user_id")?;
             let user_identity = self
                 .client
@@ -263,19 +280,20 @@ impl Client {
     }
 
     /// Process a sync error and return loop control accordingly
-    fn process_sync_error(&self, sync_error: Error) -> LoopCtrl {
-        let mut control = LoopCtrl::Continue;
+    pub(crate) fn process_sync_error(&self, sync_error: Error) -> LoopCtrl {
         if let Some(RumaApiError::ClientApi(error)) = sync_error.as_ruma_api_error() {
             if let ErrorKind::UnknownToken { soft_logout } = error.kind {
                 self.state.write().unwrap().is_soft_logout = soft_logout;
                 if let Some(delegate) = &*self.delegate.read().unwrap() {
                     delegate.did_update_restore_token();
                     delegate.did_receive_auth_error(soft_logout);
                 }
-                control = LoopCtrl::Break
+                return LoopCtrl::Break;
             }
         }
-        control
+
+        tracing::warn!("Ignoring sync error: {:?}", sync_error);
+        LoopCtrl::Continue
     }
 }
 
@@ -315,7 +333,6 @@ impl Client {
         let client = self.client.clone();
         let state = self.state.clone();
         let delegate = self.delegate.clone();
-        let session_verification_controller = self.session_verification_controller.clone();
         let local_self = self.clone();
         RUNTIME.spawn(async move {
             let mut filter = FilterDefinition::default();
@@ -337,7 +354,7 @@ impl Client {
 
             client
                 .sync_with_result_callback(sync_settings, |result| async {
-                    Ok(if let Ok(sync_response) = result {
+                    Ok(if result.is_ok() {
                         if !state.read().unwrap().has_first_synced {
                             state.write().unwrap().has_first_synced = true;
                         }
@@ -353,14 +370,6 @@ impl Client {
                             delegate.did_receive_sync_update()
                         }
 
-                        if let Some(session_verification_controller) =
-                            &*session_verification_controller.read().await
-                        {
-                            session_verification_controller
-                                .process_to_device_messages(sync_response.to_device_events)
-                                .await;
-                        }
-
                         LoopCtrl::Continue
                     } else {
                         local_self.process_sync_error(result.err().unwrap())

bindings/matrix-sdk-ffi/src/session_verification.rs

@@ -109,64 +109,66 @@ impl SessionVerificationController {
         })
     }
 
-    pub async fn process_to_device_messages(&self, to_device_events: Vec<Raw<AnyToDeviceEvent>>) {
-        let sas_verification = self.sas_verification.clone();
-
-        for event in to_device_events.into_iter().filter_map(|e| e.deserialize().ok()) {
-            match event {
-                AnyToDeviceEvent::KeyVerificationReady(event) => {
-                    if !self.is_transaction_id_valid(event.content.transaction_id.to_string()) {
-                        return;
-                    }
-                    self.start_sas_verification().await;
+    pub async fn process_to_device_message(&self, event: AnyToDeviceEvent) {
+        match event {
+            AnyToDeviceEvent::KeyVerificationReady(event) => {
+                if !self.is_transaction_id_valid(event.content.transaction_id.to_string()) {
+                    return;
+                }
+                self.start_sas_verification().await;
+            }
+            AnyToDeviceEvent::KeyVerificationCancel(event) => {
+                if !self.is_transaction_id_valid(event.content.transaction_id.to_string()) {
+                    return;
                 }
-                AnyToDeviceEvent::KeyVerificationCancel(event) => {
-                    if !self.is_transaction_id_valid(event.content.transaction_id.to_string()) {
-                        return;
-                    }
 
-                    if let Some(delegate) = &*self.delegate.read().unwrap() {
-                        delegate.did_cancel()
-                    }
+                if let Some(delegate) = &*self.delegate.read().unwrap() {
+                    delegate.did_cancel()
+                }
+            }
+            AnyToDeviceEvent::KeyVerificationKey(event) => {
+                if !self.is_transaction_id_valid(event.content.transaction_id.to_string()) {
+                    return;
                 }
-                AnyToDeviceEvent::KeyVerificationKey(event) => {
-                    if !self.is_transaction_id_valid(event.content.transaction_id.to_string()) {
-                        return;
-                    }
 
-                    if let Some(sas_verification) = &*sas_verification.read().unwrap() {
-                        if let Some(emojis) = sas_verification.emoji() {
-                            if let Some(delegate) = &*self.delegate.read().unwrap() {
-                                let emojis = emojis
-                                    .iter()
-                                    .map(|e| {
-                                        Arc::new(SessionVerificationEmoji {
-                                            symbol: e.symbol.to_owned(),
-                                            description: e.description.to_owned(),
-                                        })
+                if let Some(sas_verification) = &*self.sas_verification.read().unwrap() {
+                    if let Some(emojis) = sas_verification.emoji() {
+                        if let Some(delegate) = &*self.delegate.read().unwrap() {
+                            let emojis = emojis
+                                .iter()
+                                .map(|e| {
+                                    Arc::new(SessionVerificationEmoji {
+                                        symbol: e.symbol.to_owned(),
+                                        description: e.description.to_owned(),
                                     })
-                                    .collect::<Vec<_>>();
+                                })
+                                .collect::<Vec<_>>();
 
-                                delegate.did_receive_verification_data(emojis);
-                            }
-                        } else if let Some(delegate) = &*self.delegate.read().unwrap() {
-                            delegate.did_fail()
+                            delegate.did_receive_verification_data(emojis);
                         }
                     } else if let Some(delegate) = &*self.delegate.read().unwrap() {
                         delegate.did_fail()
                     }
+                } else if let Some(delegate) = &*self.delegate.read().unwrap() {
+                    delegate.did_fail()
+                }
+            }
+            AnyToDeviceEvent::KeyVerificationDone(event) => {
+                if !self.is_transaction_id_valid(event.content.transaction_id.to_string()) {
+                    return;
                 }
-                AnyToDeviceEvent::KeyVerificationDone(event) => {
-                    if !self.is_transaction_id_valid(event.content.transaction_id.to_string()) {
-                        return;
-                    }
 
-                    if let Some(delegate) = &*self.delegate.read().unwrap() {
-                        delegate.did_finish()
-                    }
+                if let Some(delegate) = &*self.delegate.read().unwrap() {
+                    delegate.did_finish()
                 }
-                _ => (),
             }
+            _ => (),
+        }
+    }
+
+    pub async fn process_to_device_messages(&self, to_device_events: Vec<Raw<AnyToDeviceEvent>>) {
+        for event in to_device_events.into_iter().filter_map(|e| e.deserialize().ok()) {
+            self.process_to_device_message(event).await;
         }
     }
 

bindings/matrix-sdk-ffi/src/sliding_sync.rs

@@ -17,7 +17,7 @@ use matrix_sdk::ruma::{
     assign, IdParseError, OwnedRoomId,
 };
 pub use matrix_sdk::{
-    Client as MatrixClient, RoomListEntry as MatrixRoomEntry,
+    Client as MatrixClient, LoopCtrl, RoomListEntry as MatrixRoomEntry,
     SlidingSyncBuilder as MatrixSlidingSyncBuilder, SlidingSyncMode, SlidingSyncState,
 };
 use tokio::task::JoinHandle;
@@ -521,6 +521,7 @@ impl SlidingSync {
         let inner_spawn = spawn.clone();
         {
             let mut sync_handle = self.sync_handle.write().unwrap();
+            let client = self.client.clone();
 
             if let Some(handle) = sync_handle.take() {
                 handle.abort();
@@ -533,9 +534,11 @@ impl SlidingSync {
                     let update = match stream.next().await {
                         Some(Ok(u)) => u,
                         Some(Err(e)) => {
-                            // FIXME: send this over the FFI
-                            tracing::warn!("Sliding Sync failure: {:?}", e);
-                            continue;
+                            if client.process_sync_error(e) == LoopCtrl::Break {
+                                break;
+                            } else {
+                                continue;
+                            }
                         }
                         None => {
                             tracing::debug!("No update from loop, cancelled");

crates/matrix-sdk/Cargo.toml

@@ -47,7 +47,6 @@ experimental-timeline = ["ruma/unstable-msc2676", "ruma/unstable-msc2677"]
 
 sliding-sync = [
     "matrix-sdk-base/sliding-sync",
-    "anyhow",
     "dep:derive_builder",
 ]
 

crates/matrix-sdk/src/error.rs

@@ -193,6 +193,11 @@ pub enum Error {
     #[error(transparent)]
     ImageError(#[from] ImageError),
 
+    /// An error occurred within sliding-sync
+    #[cfg(feature = "sliding-sync")]
+    #[error(transparent)]
+    SlidingSync(#[from] crate::sliding_sync::Error),
+
     /// An other error was raised
     /// this might happen because encryption was enabled on the base-crate
     /// but not here and that raised.