From 73bb46b5ea9c18ce3dc78f5b34814404be2ab1cc Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 17 Aug 2023 16:27:12 +0200 Subject: [PATCH] Revert "feat(sdk): `SlidingSync` makes timed out silent." This reverts commit 9b811009e130d5b72d88f611ff472a20705beb15. We have realized that the server might not handle timeouts as expected. Thus, it's hard to know the difference between network timeout and poll timeout (resp. the server is unreachable vs. the server has nothing to respond with). We will come back on this later. --- crates/matrix-sdk/src/sliding_sync/mod.rs | 59 +---------------------- 1 file changed, 1 insertion(+), 58 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index e0e8136c6..fc4435d34 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -60,8 +60,7 @@ use self::{ sticky_parameters::{LazyTransactionId, SlidingSyncStickyManager, StickyData}, }; use crate::{ - config::RequestConfig, sliding_sync::client::SlidingSyncResponseProcessor, Client, HttpError, - Result, + config::RequestConfig, sliding_sync::client::SlidingSyncResponseProcessor, Client, Result, }; /// The Sliding Sync instance. @@ -719,14 +718,9 @@ impl SlidingSync { } // Here, errors we can safely ignore. - // * when a response has already been received from the server. Err(crate::Error::SlidingSync(Error::ResponseAlreadyReceived { .. })) => { continue; } - // * when the server timed out, so there is no response. - Err(crate::Error::Http(HttpError::Reqwest(reqwest_error))) if reqwest_error.is_timeout() => { - continue; - } // Here, errors we **cannot** ignore, and that must stop the sync-loop. Err(error) => { @@ -1951,55 +1945,4 @@ mod tests { Ok(()) } - - #[async_test] - async fn test_timeout() -> Result<()> { - let server = MockServer::start().await; - let client = logged_in_client(Some(server.uri())).await; - - let sliding_sync = client - .sliding_sync("test-slidingsync")? - .poll_timeout(Duration::from_millis(100)) - .network_timeout(Duration::from_millis(100)) - .build() - .await?; - - let sync = sliding_sync.sync(); - pin_mut!(sync); - - let _mock_guard1 = Mock::given(SlidingSyncMatcher) - .respond_with(|_request: &Request| { - ResponseTemplate::new(200) - .set_body_json(json!({ - "pos": "0", - })) - .set_delay(Duration::from_secs(5)) // reply after 5s. - }) - .up_to_n_times(1) // run it once. - .mount_as_scoped(&server) - .await; - - let _mock_guard2 = Mock::given(SlidingSyncMatcher) - .respond_with(|_request: &Request| { - ResponseTemplate::new(200).set_body_json(json!({ - "pos": "1", - })) - }) - .up_to_n_times(1) // run it once. - .mount_as_scoped(&server) - .await; - - let next = sync.next().await; - assert_matches!(next, Some(Ok(_update_summary))); - - // `pos` has been updated to `1`. - assert_eq!(sliding_sync.inner.position.lock().await.pos, Some("1".to_owned())); - - // `past_positions` contains only `1` because `0` has timed out and is absent. - let past_positions = sliding_sync.inner.past_positions.read().unwrap(); - assert_eq!(past_positions.len(), 1); - assert_eq!(past_positions.get(0).unwrap().pos, Some("1".to_owned())); - - Ok(()) - } }