Commit 0b8ce8dc9a1c
tarball5 files changed+115-74
@@ -82,17 +82,16 @@ update carries its diffs and the items they left, so apply the diffs or
8282
re-render the list, whichever suits. a subscriber that falls behind gets a fresh8383
`State` instead of an error8484
85
-a batch that doesn't fit leaves the timeline diverged at its last good items.86
-the mirror asks for the resync itself, off the draining path, and keeps asking87
-until the reset or a close turns up. rooms are refetched on `RoomsChanged`, one88
-fetch at a time with at most one more owed89
-90
-with a store feature, `with_session_store` saves the record on every91
-`SessionTokensChanged` before the next event is touched, and a failed save shows92
-up as an update. without one, catch `SessionTokensChanged` on the raw stream93
-and save it yourself, a subscriber that lags could miss it. `save` is all the94
-mirror calls. `soft_log_out` and `discard` wait for `Engine::run`'s `Termination`95
-once the client is gone85
+a batch that doesn't fit leaves the timeline diverged at its last good items,86
+and so does a diff for a timeline it never saw reset. the mirror asks for the87
+resync itself, off the draining path, and keeps asking until the reset or a88
+close turns up. rooms are refetched on `RoomsChanged`, one fetch at a time with89
+at most one more owed90
+91
+failures stay in the `State`, so a late subscriber sees them too. what stopped92
+the engine sits in `engine_failure`, and a failed save of the session record in93
+`session_save_failure` until the next one works. the mirror never touches the94
+store9695
9796
## wasm9897
@@ -243,9 +243,9 @@ over from a fresh state under the same lock
243243
nothing on the draining path waits for an engine answer. a resync runs on its244244
own task, retrying with backoff until a reset or a close for its timeline245245
aborts it. rooms are fetched on a task too, one at a time with one more owed at246
-most. the only thing it does wait for is `SessionStore::save`, since a refresh247
-token spent before the save lands loses the login. `SessionTokensChanged`248
-never becomes an update, a lagging subscriber would lose it246
+most. a diff for a timeline with no reset is a divergence too, so a wrapper247
+that swallowed the reset still heals. anything a late subscriber needs, failures248
+included, lives in the state rather than only in an update249249
250250
## public model251251
@@ -49,6 +49,10 @@ pub struct State {
4949
pub timelines: imbl::HashMap<TimelineId, TimelineState>,5050
pub typing: imbl::HashMap<OwnedRoomId, Vec<OwnedUserId>>,5151
pub uploads: imbl::HashMap<OwnedTransactionId, UploadState>,52
+ /// what stopped the engine, if something went wrong53
+ pub engine_failure: Option<EngineFailure>,54
+ /// the last save of the session record that failed, until one succeeds55
+ pub session_save_failure: Option<String>,5256
}5357
5458
#[derive(Clone, Debug, PartialEq)]@@ -111,8 +115,9 @@ pub enum StateChange {
111115
Authenticated(OwnedUserId),112116
Session(SessionStatus),113117
/// the record on disk may now hold a spent refresh token114
- #[cfg(any(feature = "sqlite", feature = "indexeddb"))]115
- SessionSaveFailed(Arc<crate::AccountError>),118
+ SessionSaveFailed(String),119
+ /// saved again after `SessionSaveFailed`120
+ SessionSaved,116121
Connection(ConnectionState),117122
Encryption(EncryptionState),118123
Verification(VerificationState),@@ -164,8 +169,6 @@ pub struct Mirror {
164169
shared: Arc<Shared>,165170
resyncs: HashMap<TimelineId, AbortHandle>,166171
rooms_fetch: Option<AbortHandle>,167
- #[cfg(any(feature = "sqlite", feature = "indexeddb"))]168
- session_store: Option<Arc<dyn crate::SessionStore>>,169172
}170173
171174
impl fmt::Debug for Mirror {@@ -183,7 +186,8 @@ impl Mirror {
183186
Self::with_capacity(handle, events, DEFAULT_CAPACITY)184187
}185188
186
- /// `capacity` updates can queue for a subscriber before it lags and starts over189
+ /// `capacity` updates can queue for a subscriber before it lags and starts over. a zero190
+ /// capacity counts as one187191
pub fn with_capacity(188192
handle: ClientHandle,189193
events: impl Stream<Item = Event> + SendOutsideWasm + 'static,@@ -200,12 +204,14 @@ impl Mirror {
200204
timelines: imbl::HashMap::new(),201205
typing: imbl::HashMap::new(),202206
uploads: imbl::HashMap::new(),207
+ engine_failure: None,208
+ session_save_failure: None,203209
};204210
let shared = Arc::new(Shared {205211
published: Mutex::new(Published {206212
state: Arc::new(state),207213
sequence: 0,208
- updates: Some(broadcast::channel(capacity).0),214
+ updates: Some(broadcast::channel(capacity.max(1)).0),209215
rooms_fetch: RoomsFetch::Idle,210216
}),211217
});@@ -219,32 +225,21 @@ impl Mirror {
219225
shared: shared.clone(),220226
resyncs: HashMap::new(),221227
rooms_fetch: None,222
- #[cfg(any(feature = "sqlite", feature = "indexeddb"))]223
- session_store: None,224228
};225229
(mirror, MirrorHandle { shared })226230
}227231
228
- /// saves the record on every `SessionTokensChanged` before taking another event, and229
- /// calls nothing but `save`. the store can't go while the client holds it open230
- #[cfg(any(feature = "sqlite", feature = "indexeddb"))]231
- #[must_use]232
- pub fn with_session_store(mut self, store: Arc<dyn crate::SessionStore>) -> Self {233
- self.session_store = Some(store);234
- self235
- }236
-237232
/// resolves once the event stream ends, which is just after `Stopped`238233
pub fn run(mut self) -> EngineBoxFuture<'static, ()> {239234
Box::pin(async move {240235
self.refetch_rooms();241236
while let Some(event) = self.events.next().await {242
- self.apply(event).await;237
+ self.apply(event);243238
}244239
})245240
}246241
247
- async fn apply(&mut self, event: Event) {242
+ fn apply(&mut self, event: Event) {248243
match event {249244
Event::ConnectionChanged(connection) => self.publish(|state| {250245
state.connection = connection;@@ -254,10 +249,14 @@ impl Mirror {
254249
state.user_id = Some(user_id.clone());255250
Some(StateChange::Authenticated(user_id))256251
}),257
- #[cfg(any(feature = "sqlite", feature = "indexeddb"))]258
- Event::SessionTokensChanged => self.save_session().await,259
- #[cfg(not(any(feature = "sqlite", feature = "indexeddb")))]260
- Event::SessionTokensChanged => {}252
+ Event::SessionSaveFailed { message } => self.publish(|state| {253
+ state.session_save_failure = Some(message.clone());254
+ Some(StateChange::SessionSaveFailed(message))255
+ }),256
+ Event::SessionTokensChanged => self.publish(|state| {257
+ state.session_save_failure.take()?;258
+ Some(StateChange::SessionSaved)259
+ }),261260
Event::SessionAccessRejected { soft_logout } => self.publish(|state| {262261
state.session = match soft_logout {263262
Some(device_id) => SessionStatus::SoftLoggedOut { device_id },@@ -279,7 +278,10 @@ impl Mirror {
279278
Some(StateChange::Session(SessionStatus::Stopped))280279
});281280
}282
- Event::Failed(failure) => self.publish(|_| Some(StateChange::Failed(failure))),281
+ Event::Failed(failure) => self.publish(|state| {282
+ state.engine_failure = Some(failure.clone());283
+ Some(StateChange::Failed(failure))284
+ }),283285
Event::NotificationSettingsChanged => {284286
self.publish(|_| Some(StateChange::NotificationSettings));285287
}@@ -406,7 +408,21 @@ impl Mirror {
406408
fn apply_diffs(&mut self, timeline_id: TimelineId, diffs: Vec<TimelineDiff>) {407409
let mut diverged = false;408410
self.publish(|state| {409
- let timeline = state.timelines.get_mut(&timeline_id)?;411
+ let Some(timeline) = state.timelines.get_mut(&timeline_id) else {412
+ diverged = true;413
+ state.timelines.insert(414
+ timeline_id.clone(),415
+ TimelineState {416
+ items: Vector::new(),417
+ history: HistoryState::Idle,418
+ diverged: true,419
+ },420
+ );421
+ return Some(StateChange::Timeline {422
+ timeline_id: timeline_id.clone(),423
+ change: TimelineChange::Diverged,424
+ });425
+ };410426
if timeline.diverged {411427
return None;412428
}@@ -434,21 +450,6 @@ impl Mirror {
434450
}435451
}436452
437
- #[cfg(any(feature = "sqlite", feature = "indexeddb"))]438
- async fn save_session(&mut self) {439
- let Some(store) = &self.session_store else {440
- return;441
- };442
- let saved = match self.handle.session_record() {443
- Ok(record) => store.save(&record).await,444
- Err(error) => Err(error),445
- };446
- if let Err(error) = saved {447
- tracing::warn!(%error, "saving the session record failed");448
- self.publish(|_| Some(StateChange::SessionSaveFailed(Arc::new(error))));449
- }450
- }451
-452453
fn refetch_rooms(&mut self) {453454
let mut published = self.shared.lock();454455
match published.rooms_fetch {@@ -1324,17 +1324,18 @@ mod tests {
13241324
let mut items = vector![item(1), item(2)];13251325
let batch = vec![13261326
TimelineDiff::PushBack { value: item(3) },1327
+ TimelineDiff::PushBack { value: item(4) },13271328
TimelineDiff::Remove { index: 0 },13281329
TimelineDiff::Set {1329
- index: 2,1330
- value: item(4),1330
+ index: 3,1331
+ value: item(5),13311332
},13321333
];13331334
assert_eq!(13341335
TimelineDiff::try_apply_all(batch, &mut items)13351336
.unwrap_err()13361337
.length,1337
- 21338
+ 313381339
);13391340
assert_eq!(items, vector![item(1), item(2)]);13401341
@@ -9,7 +9,10 @@ use macltok::{
99
UploadOutcome,1010
};1111
use matrix_sdk::{12
- ruma::{OwnedRoomId, TransactionId, api::client::sync::sync_events::v5, event_id, room_id},12
+ ruma::{13
+ OwnedRoomId, OwnedTransactionId, TransactionId, api::client::sync::sync_events::v5,14
+ event_id, room_id,15
+ },1316
test_utils::mocks::MatrixMockServer,1417
};1518
use tokio::sync::mpsc;@@ -149,6 +152,30 @@ async fn diverged_batch_keeps_last_good_items_until_a_reset() {
149152
}150153
151154
#[tokio::test]155
+async fn a_diff_for_a_timeline_without_a_reset_diverges_it() {156
+ let harness = start(EngineConfig::default(), 256).await;157
+ let timeline_id = TimelineId::from(harness.room_id.clone());158
+ let mut updates = harness.mirror.subscribe();159
+ harness160
+ .inject161
+ .send(Event::TimelineChanged {162
+ timeline_id: timeline_id.clone(),163
+ diffs: vec![TimelineDiff::PushBack { value: item(1) }],164
+ })165
+ .unwrap();166
+ next_matching(&mut updates, |change| {167
+ timeline_change(change, &timeline_id, |change| {168
+ matches!(change, TimelineChange::Diverged)169
+ })170
+ })171
+ .await;172
+ let snapshot = harness.mirror.snapshot();173
+ let timeline = &snapshot.timelines[&timeline_id];174
+ assert!(timeline.diverged);175
+ assert!(timeline.items.is_empty());176
+}177
+178
+#[tokio::test]152179
async fn room_and_thread_timelines_stay_apart() {153180
let harness = start(EngineConfig::default(), 256).await;154181
let room = TimelineId::from(harness.room_id.clone());@@ -183,25 +210,38 @@ async fn room_and_thread_timelines_stay_apart() {
183210
);184211
}185212
186
-#[tokio::test]187
-async fn late_subscriber_gets_state_then_every_update_after_it() {188
- let harness = start(EngineConfig::default(), 1024).await;213
+#[tokio::test(flavor = "multi_thread", worker_threads = 4)]214
+async fn subscribers_joining_mid_stream_see_no_gap_and_nothing_twice() {215
+ const CHUNKS: u64 = 64;216
+ const CHUNK: u64 = 32;217
+ let harness = start(EngineConfig::default(), 4096).await;189218
let transaction_id = TransactionId::new();190
- let progress = |uploaded_bytes| Event::MediaUploadProgress {191
- room_id: harness.room_id.clone(),192
- transaction_id: transaction_id.clone(),193
- uploaded_bytes,194
- total_bytes: 100,195
- complete: false,196
- };197
- for uploaded_bytes in 1..=50 {198
- harness.inject.send(progress(uploaded_bytes)).unwrap();219
+ let mut followers = Vec::new();220
+ for chunk in 0..CHUNKS {221
+ for uploaded_bytes in chunk * CHUNK + 1..=(chunk + 1) * CHUNK {222
+ harness223
+ .inject224
+ .send(Event::MediaUploadProgress {225
+ room_id: harness.room_id.clone(),226
+ transaction_id: transaction_id.clone(),227
+ uploaded_bytes,228
+ total_bytes: CHUNKS * CHUNK,229
+ complete: false,230
+ })231
+ .unwrap();232
+ }233
+ followers.push(tokio::spawn(follow(234
+ harness.mirror.subscribe(),235
+ transaction_id.clone(),236
+ CHUNKS * CHUNK,237
+ )));199238
}200
- let mut updates = harness.mirror.subscribe();201
- for uploaded_bytes in 51..=100 {202
- harness.inject.send(progress(uploaded_bytes)).unwrap();239
+ for follower in followers {240
+ follower.await.unwrap();203241
}242
+}204243
244
+async fn follow(mut updates: Subscription, transaction_id: OwnedTransactionId, total: u64) {205245
let first = updates.next().await.unwrap();206246
let StateChange::State(state) = &first.change else {207247
panic!("a subscription opened with {first:?}");@@ -211,7 +251,7 @@ async fn late_subscriber_gets_state_then_every_update_after_it() {
211251
.get(&transaction_id)212252
.map_or(0, |upload| upload.uploaded_bytes);213253
let mut sequence = first.sequence;214
- while seen < 100 {254
+ while seen < total {215255
let update = next_matching(&mut updates, |_| true).await;216256
assert_eq!(update.sequence, sequence + 1);217257
sequence = update.sequence;