From d4cbcd397d28b79733b8f7c6908eff01d01345e8 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 17 Aug 2023 15:35:51 +0200 Subject: [PATCH] feat(sdk): `SlidingSync` is able to ignore some errors. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All errors inside `SlidingSync` are stopping the sync-loop, and errors are returned to the caller. However, in some situation, some errors should be ignored, i.e. they should not stop the sync-loop and they should not be returned to the caller: the sync-loop just continues to run. This patch does that for `Error::ResponseAlreadyReceived`. More errors will come. Why is it annoying? When `matrix_sdk_ui::SyncService` sees an error, it stops all the sync-loops (`RoomListService`, `EncryptionSync`…) and restarts them properly. In the case of `Error::ResponseAlreadyReceived`, this is a waste of time and resources. This error is an error from the `SlidingSync` point of view, but _not_ from the caller point of view. --- crates/matrix-sdk/src/sliding_sync/mod.rs | 42 +++++++---------------- 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 3bd5e8035..fc4435d34 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -717,6 +717,12 @@ impl SlidingSync { yield Ok(updates); } + // Here, errors we can safely ignore. + Err(crate::Error::SlidingSync(Error::ResponseAlreadyReceived { .. })) => { + continue; + } + + // Here, errors we **cannot** ignore, and that must stop the sync-loop. Err(error) => { if error.client_api_error_kind() == Some(&ErrorKind::UnknownPos) { // The Sliding Sync session has expired. Let's reset `pos` and sticky parameters. @@ -1494,7 +1500,8 @@ mod tests { // Next request isn't successful because it receives an already // received `pos` from the server. { - let _mock_guard = Mock::given(SlidingSyncMatcher) + // First response with an already seen `pos`. + let _mock_guard1 = Mock::given(SlidingSyncMatcher) .respond_with(|request: &Request| { // Repeat the txn_id in the response, if set. let request: PartialRequest = request.body_json().unwrap(); @@ -1504,44 +1511,19 @@ mod tests { "pos": "0", // <- already received! })) }) + .up_to_n_times(1) // run this mock only once. .mount_as_scoped(&server) .await; - let next = sync.next().await; - assert_matches!( - next, - Some(Err(crate::Error::SlidingSync(Error::ResponseAlreadyReceived { pos }))) => { - assert_eq!(pos, Some("0".to_owned())); - } - ); - - // `sync` has been stopped. - assert!(sync.next().await.is_none()); - - // `pos` has not been updated. - assert_eq!(sliding_sync.inner.position.lock().await.pos, Some("1".to_owned())); - - // `past_positions` has not been updated. - let past_positions = sliding_sync.inner.past_positions.read().unwrap(); - assert_eq!(past_positions.len(), 2); - assert_eq!(past_positions.get(0).unwrap().pos, Some("0".to_owned())); - assert_eq!(past_positions.get(1).unwrap().pos, Some("1".to_owned())); - } - - // Restart the sync. - let sync = sliding_sync.sync(); - pin_mut!(sync); - - // Next request is successful. - { - let _mock_guard = Mock::given(SlidingSyncMatcher) + // Second response with a new `pos`. + let _mock_guard2 = Mock::given(SlidingSyncMatcher) .respond_with(|request: &Request| { // Repeat the txn_id in the response, if set. let request: PartialRequest = request.body_json().unwrap(); ResponseTemplate::new(200).set_body_json(json!({ "txn_id": request.txn_id, - "pos": "2", + "pos": "2", // <- new! })) }) .mount_as_scoped(&server)