diff --git a/crates/matrix-sdk/src/sliding_sync/error.rs b/crates/matrix-sdk/src/sliding_sync/error.rs index f5f968667..594f754c5 100644 --- a/crates/matrix-sdk/src/sliding_sync/error.rs +++ b/crates/matrix-sdk/src/sliding_sync/error.rs @@ -1,10 +1,10 @@ //! Sliding Sync errors. use thiserror::Error; +use tokio::task::JoinError; /// Internal representation of errors in Sliding Sync. #[derive(Error, Debug)] -#[cfg_attr(test, derive(PartialEq))] #[non_exhaustive] pub enum Error { /// The response we've received from the server can't be parsed or doesn't @@ -41,6 +41,11 @@ pub enum Error { InvalidSlidingSyncIdentifier, /// A task failed to execute to completion. - #[error("A task failed to execute to completion; task description: {0}")] - JoinError(String), + #[error("A task failed to execute to completion; task description: {task_description}")] + JoinError { + /// Task description. + task_description: String, + /// The original `JoinError`. + error: JoinError, + }, } diff --git a/crates/matrix-sdk/src/sliding_sync/list/request_generator.rs b/crates/matrix-sdk/src/sliding_sync/list/request_generator.rs index 80e6aa0f1..fd016c0a6 100644 --- a/crates/matrix-sdk/src/sliding_sync/list/request_generator.rs +++ b/crates/matrix-sdk/src/sliding_sync/list/request_generator.rs @@ -326,53 +326,57 @@ fn create_range( #[cfg(test)] mod tests { + use std::ops::RangeInclusive; + + use assert_matches::assert_matches; + use super::*; #[test] fn test_create_range_from() { // From 0, we want 100 items. - assert_eq!(create_range(0, 100, None, None), Ok(0..=99)); + assert_matches!(create_range(0, 100, None, None), Ok(range) if range == RangeInclusive::new(0, 99)); // From 100, we want 100 items. - assert_eq!(create_range(100, 100, None, None), Ok(100..=199)); + assert_matches!(create_range(100, 100, None, None), Ok(range) if range == RangeInclusive::new(100, 199)); // From 0, we want 100 items, but there is a maximum number of rooms to fetch // defined at 50. - assert_eq!(create_range(0, 100, Some(50), None), Ok(0..=49)); + assert_matches!(create_range(0, 100, Some(50), None), Ok(range) if range == RangeInclusive::new(0, 49)); // From 49, we want 100 items, but there is a maximum number of rooms to fetch // defined at 50. There is 1 item to load. - assert_eq!(create_range(49, 100, Some(50), None), Ok(49..=49)); + assert_matches!(create_range(49, 100, Some(50), None), Ok(range) if range == RangeInclusive::new(49, 49)); // From 50, we want 100 items, but there is a maximum number of rooms to fetch // defined at 50. - assert_eq!( + assert_matches!( create_range(50, 100, Some(50), None), Err(Error::InvalidRange { start: 50, end: 49 }) ); // From 0, we want 100 items, but there is a maximum number of rooms defined at // 50. - assert_eq!(create_range(0, 100, None, Some(50)), Ok(0..=49)); + assert_matches!(create_range(0, 100, None, Some(50)), Ok(range) if range == RangeInclusive::new(0, 49)); // From 49, we want 100 items, but there is a maximum number of rooms defined at // 50. There is 1 item to load. - assert_eq!(create_range(49, 100, None, Some(50)), Ok(49..=49)); + assert_matches!(create_range(49, 100, None, Some(50)), Ok(range) if range == RangeInclusive::new(49, 49)); // From 50, we want 100 items, but there is a maximum number of rooms defined at // 50. - assert_eq!( + assert_matches!( create_range(50, 100, None, Some(50)), Err(Error::InvalidRange { start: 50, end: 49 }) ); // From 0, we want 100 items, but there is a maximum number of rooms to fetch // defined at 75, and a maximum number of rooms defined at 50. - assert_eq!(create_range(0, 100, Some(75), Some(50)), Ok(0..=49)); + assert_matches!(create_range(0, 100, Some(75), Some(50)), Ok(range) if range == RangeInclusive::new(0, 49)); // From 0, we want 100 items, but there is a maximum number of rooms to fetch // defined at 50, and a maximum number of rooms defined at 75. - assert_eq!(create_range(0, 100, Some(50), Some(75)), Ok(0..=49)); + assert_matches!(create_range(0, 100, Some(50), Some(75)), Ok(range) if range == RangeInclusive::new(0, 49)); } #[test] diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 80b57598d..5d6d20d02 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -485,16 +485,21 @@ impl SlidingSync { if self.inner.sticky.read().unwrap().data().extensions.e2ee.enabled == Some(true) { debug!("Sliding Sync is sending the request along with outgoing E2EE requests"); - // If the sliding sync request fails faster than the e2ee requests get their - // responses, we want to fail as early as possible, so as to invalidate the - // sliding sync session as early as possible. + // Here, we need to run 2 things: // - // Also, we don't want an interruption in the sliding sync request to abort - // processing of the e2ee response. + // 1. Send the sliding sync request and get a response, + // 2. Send the E2EE requests. // - // For those reasons, start the e2ee requests in a background task. If this is - // aborted while everything is running, the same data might be sent later, which - // is fine. + // We don't want to use a `join` or `try_join` because we want to fail if and + // only if sending the sliding sync request fails. Failing to send the E2EE + // requests should just result in a log. + // + // We also want to give the priority to sliding sync request. E2EE requests are + // sent concurrently to the sliding sync request, but the priority is on waiting + // a sliding sync response. + // + // If sending sliding sync request fails, the sending of E2EE requests must be + // aborted as soon as possible. let client = self.inner.client.clone(); let e2ee_uploads = spawn(async move { @@ -513,7 +518,10 @@ impl SlidingSync { // `e2ee_uploads`. It did run concurrently, so it should not be blocking for too // long. Otherwise —if `request` has failed— `e2ee_uploads` has // been dropped, so aborted. - e2ee_uploads.await.map_err(|_| Error::JoinError("e2ee_uploads".to_string()))?; + e2ee_uploads.await.map_err(|error| Error::JoinError { + task_description: "e2ee_uploads".to_owned(), + error, + })?; response } else { diff --git a/crates/matrix-sdk/src/sliding_sync/utils.rs b/crates/matrix-sdk/src/sliding_sync/utils.rs index d3f1da357..0bc1e998d 100644 --- a/crates/matrix-sdk/src/sliding_sync/utils.rs +++ b/crates/matrix-sdk/src/sliding_sync/utils.rs @@ -12,7 +12,7 @@ use tokio::task::{JoinError, JoinHandle}; pub(crate) struct AbortOnDrop(JoinHandle); impl AbortOnDrop { - pub fn new(join_handle: JoinHandle) -> Self { + fn new(join_handle: JoinHandle) -> Self { Self(join_handle) } }