From c7573469b1d261e6eaddb5b13f3dcff733c5d5e8 Mon Sep 17 00:00:00 2001 From: Alvin Date: Fri, 27 Mar 2026 16:44:00 +0800 Subject: [PATCH 1/3] fix(ui): long-poll room list sync after initial request --- .../src/room_list_service/mod.rs | 85 ++++++++++++++----- 1 file changed, 66 insertions(+), 19 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index bd6a046b53d..a98d098d87d 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -220,28 +220,23 @@ impl RoomListService { // returned. If true, only invited rooms are returned. is_invite: None, }))) - .requires_timeout(move |request_generator| { + .requires_timeout(move |_request_generator| { // We want Sliding Sync to apply the poll + network timeout —i.e. to do the // long-polling— in some particular cases. Let's define them. match observable_state.get() { - // These are the states where we want an immediate response from the - // server, with no long-polling. - State::Init - | State::SettingUp - | State::Recovering - | State::Error { .. } - | State::Terminated { .. } => PollTimeout::Some(0), - - // Otherwise we want long-polling if the list is fully-loaded. - State::Running => { - if request_generator.is_fully_loaded() { - // Long-polling. - PollTimeout::Default - } else { - // No long-polling yet. - PollTimeout::Some(0) - } + // The very first request must return immediately so the session can + // be established as fast as possible. + State::Init => PollTimeout::Some(0), + + // Once we have a `pos`, let the server long-poll. If a list change + // still needs to be delivered, the server can answer immediately. + State::SettingUp | State::Recovering | State::Running => { + PollTimeout::Default } + + // The sync loop doesn't keep running in these states, but keeping an + // explicit value here makes the policy exhaustive. + State::Error { .. } | State::Terminated { .. } => PollTimeout::Some(0), } }), ) @@ -562,7 +557,13 @@ pub enum SyncIndicator { #[cfg(test)] mod tests { - use std::future::ready; + use std::{ + future::ready, + sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, + }, + }; use futures_util::{StreamExt, pin_mut}; use matrix_sdk::{ @@ -677,4 +678,50 @@ mod tests { Ok(()) } + + #[async_test] + async fn test_setting_up_uses_long_poll_timeout_after_first_sync() -> Result<(), Error> { + let (client, server) = new_client().await; + let room_list = RoomListService::new(client).await?; + + let sync = room_list.sync(); + pin_mut!(sync); + + let pos = Arc::new(AtomicUsize::new(0)); + let _mock_guard = Mock::given(SlidingSyncMatcher) + .respond_with(move |_request: &Request| { + let next_pos = pos.fetch_add(1, Ordering::SeqCst).to_string(); + ResponseTemplate::new(200).set_body_json(json!({ + "pos": next_pos, + "lists": { + ALL_ROOMS_LIST_NAME: { + "count": 0, + "ops": [], + }, + }, + "rooms": {}, + })) + }) + .mount_as_scoped(&server) + .await; + + let _ = sync.next().await; + assert_eq!(room_list.state().get(), State::SettingUp); + let _ = sync.next().await; + + let requests = server.received_requests().await.unwrap(); + let second_request = + requests.iter().filter(|request| SlidingSyncMatcher.matches(request)).nth(1).unwrap(); + + assert!( + second_request + .url + .query_pairs() + .any(|(key, value)| key == "timeout" && value == "30000"), + "expected second sync request to enable long-poll timeout, got {:?}", + second_request.url + ); + + Ok(()) + } } From c47fa7e70dbb0c84da1a4c2afcd3d0138da94d51 Mon Sep 17 00:00:00 2001 From: Alvin Date: Fri, 27 Mar 2026 17:12:49 +0800 Subject: [PATCH 2/3] fix: update timeout assertions in integration tests to match new policy --- crates/matrix-sdk-ui/src/room_list_service/mod.rs | 5 ++--- .../tests/integration/room_list_service.rs | 15 +++++++-------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index a98d098d87d..22eb9f0efb2 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -234,8 +234,7 @@ impl RoomListService { PollTimeout::Default } - // The sync loop doesn't keep running in these states, but keeping an - // explicit value here makes the policy exhaustive. + // Terminal states — included for exhaustiveness. State::Error { .. } | State::Terminated { .. } => PollTimeout::Some(0), } }), @@ -680,7 +679,7 @@ mod tests { } #[async_test] - async fn test_setting_up_uses_long_poll_timeout_after_first_sync() -> Result<(), Error> { + async fn test_long_poll_after_first_sync() -> Result<(), Error> { let (client, server) = new_client().await; let room_list = RoomListService::new(client).await?; diff --git a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs index bd51ea944fc..f7fb5ed4c68 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs @@ -415,8 +415,8 @@ async fn test_sync_all_states() -> Result<(), Error> { states = SettingUp => Running, // The previous `pos`. assert pos Some("0"), - // Still no long-polling because the list isn't fully-loaded. - assert timeout Some(0), + // Long-polling is enabled for all post-init states. + assert timeout Some(30000), assert request >= { "conn_id": "room-list", "lists": { @@ -443,8 +443,8 @@ async fn test_sync_all_states() -> Result<(), Error> { [server, room_list, sync] states = Running => Running, assert pos Some("1"), - // Still no long-polling because the list isn't fully-loaded. - assert timeout Some(0), + // Long-polling is enabled for all post-init states. + assert timeout Some(30000), assert request >= { "conn_id": "room-list", "lists": { @@ -471,9 +471,8 @@ async fn test_sync_all_states() -> Result<(), Error> { [server, room_list, sync] states = Running => Running, assert pos Some("2"), - // Still no long-polling because the list isn't fully-loaded, - // but it's about to be! - assert timeout Some(0), + // Long-polling is enabled for all post-init states. + assert timeout Some(30000), assert request >= { "conn_id": "room-list", "lists": { @@ -500,7 +499,7 @@ async fn test_sync_all_states() -> Result<(), Error> { [server, room_list, sync] states = Running => Running, assert pos Some("3"), - // The list is fully-loaded, we can start long-polling. + // Long-polling is enabled for all post-init states. assert timeout Some(30000), assert request >= { "conn_id": "room-list", From a5d7247a602fdc27f8f1f5042398c0fc81e0e1b9 Mon Sep 17 00:00:00 2001 From: Alvin Date: Thu, 23 Apr 2026 14:56:07 +0800 Subject: [PATCH 3/3] fix(ui): narrow long-poll change to `Running` only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review on matrix-org/matrix-rust-sdk#6361: long-polling in `SettingUp` / `Recovering` extends the time spent outside `Running` and falsely triggers `SyncIndicator` (which derives `Show` from how long the state machine stays in non-`Running` states). Roll those states back to `PollTimeout::Some(0)` and limit the policy change to collapsing the `Running` branch so it always long-polls regardless of `is_fully_loaded()`. The idle-loop symptom from Robrix2 + local Palpo (MSC4186) still goes away because the loop sits in `Running` + `!is_fully_loaded` — the client repeatedly issues `pos=&timeout=0` while the server has nothing to deliver for the next Growing batch. Tests: - Integration `test_sync_all_states`: `SettingUp -> Running` request goes back to `timeout=0`; `Running -> Running` requests stay at `timeout=30000`. - Unit `test_long_poll_once_running` (renamed): drive 3 sync cycles, assert the `SettingUp` request still carries `timeout=0` AND the first `Running` request carries `timeout=30000`, defending the `SyncIndicator` invariant. --- .../src/room_list_service/mod.rs | 67 +++++++++++++------ .../tests/integration/room_list_service.rs | 12 ++-- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index 22eb9f0efb2..05935be8420 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -224,18 +224,22 @@ impl RoomListService { // We want Sliding Sync to apply the poll + network timeout —i.e. to do the // long-polling— in some particular cases. Let's define them. match observable_state.get() { - // The very first request must return immediately so the session can - // be established as fast as possible. - State::Init => PollTimeout::Some(0), - - // Once we have a `pos`, let the server long-poll. If a list change - // still needs to be delivered, the server can answer immediately. - State::SettingUp | State::Recovering | State::Running => { - PollTimeout::Default - } - - // Terminal states — included for exhaustiveness. - State::Error { .. } | State::Terminated { .. } => PollTimeout::Some(0), + // Pre-`Running` states must respond immediately: the session is still + // being established, recovering, or has terminated. `SyncIndicator` + // also derives from the time spent outside `Running`, so long-polling + // here would falsely trigger the loading hint. + State::Init + | State::SettingUp + | State::Recovering + | State::Error { .. } + | State::Terminated { .. } => PollTimeout::Some(0), + + // Once we are `Running`, let the server long-poll regardless of + // whether the list is fully loaded. If the next Growing batch is + // ready (or any update is queued) the server can still answer + // immediately; otherwise it holds the connection instead of forcing + // the client to spin on empty responses. + State::Running => PollTimeout::Default, } }), ) @@ -679,7 +683,7 @@ mod tests { } #[async_test] - async fn test_long_poll_after_first_sync() -> Result<(), Error> { + async fn test_long_poll_once_running() -> Result<(), Error> { let (client, server) = new_client().await; let room_list = RoomListService::new(client).await?; @@ -704,21 +708,44 @@ mod tests { .mount_as_scoped(&server) .await; + // Drive the state machine: Init -> SettingUp -> Running -> Running. let _ = sync.next().await; assert_eq!(room_list.state().get(), State::SettingUp); let _ = sync.next().await; + assert_eq!(room_list.state().get(), State::Running); + let _ = sync.next().await; let requests = server.received_requests().await.unwrap(); - let second_request = - requests.iter().filter(|request| SlidingSyncMatcher.matches(request)).nth(1).unwrap(); + let sliding_sync_requests: Vec<_> = + requests.iter().filter(|request| SlidingSyncMatcher.matches(request)).collect(); - assert!( - second_request + let timeout_of = |index: usize| -> Option { + sliding_sync_requests[index] .url .query_pairs() - .any(|(key, value)| key == "timeout" && value == "30000"), - "expected second sync request to enable long-poll timeout, got {:?}", - second_request.url + .find(|(key, _)| key == "timeout") + .map(|(_, value)| value.into_owned()) + }; + + // The `SettingUp` request must keep `timeout=0`: `SyncIndicator` is derived + // from how long we stay outside `Running`, so long-polling here would falsely + // trigger the loading hint. + assert_eq!( + timeout_of(1).as_deref(), + Some("0"), + "request issued in `SettingUp` must not long-poll: {:?}", + sliding_sync_requests[1].url, + ); + + // The first request issued while already in `Running` must long-poll, even + // though the list is not yet fully loaded — otherwise an idle homeserver + // gets flooded with back-to-back `timeout=0` requests carrying the same + // `pos`. + assert_eq!( + timeout_of(2).as_deref(), + Some("30000"), + "request issued in `Running` must long-poll: {:?}", + sliding_sync_requests[2].url, ); Ok(()) diff --git a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs index f7fb5ed4c68..ef06a1e2e58 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs @@ -415,8 +415,10 @@ async fn test_sync_all_states() -> Result<(), Error> { states = SettingUp => Running, // The previous `pos`. assert pos Some("0"), - // Long-polling is enabled for all post-init states. - assert timeout Some(30000), + // `SettingUp` keeps `timeout=0` to preserve `SyncIndicator` semantics: + // long-polling here would extend the time spent outside `Running` and + // falsely trigger the loading hint. + assert timeout Some(0), assert request >= { "conn_id": "room-list", "lists": { @@ -443,7 +445,7 @@ async fn test_sync_all_states() -> Result<(), Error> { [server, room_list, sync] states = Running => Running, assert pos Some("1"), - // Long-polling is enabled for all post-init states. + // Long-polling is enabled in `Running`, regardless of fully-loaded. assert timeout Some(30000), assert request >= { "conn_id": "room-list", @@ -471,7 +473,7 @@ async fn test_sync_all_states() -> Result<(), Error> { [server, room_list, sync] states = Running => Running, assert pos Some("2"), - // Long-polling is enabled for all post-init states. + // Long-polling is enabled in `Running`, regardless of fully-loaded. assert timeout Some(30000), assert request >= { "conn_id": "room-list", @@ -499,7 +501,7 @@ async fn test_sync_all_states() -> Result<(), Error> { [server, room_list, sync] states = Running => Running, assert pos Some("3"), - // Long-polling is enabled for all post-init states. + // Long-polling is enabled in `Running`, regardless of fully-loaded. assert timeout Some(30000), assert request >= { "conn_id": "room-list",