From 66fb6e7cf8e2b3f7688eb50f8f8b9aafe0998971 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Sun, 16 May 2021 22:22:06 +0000 Subject: [PATCH] Fix various issues with room events and related logic. - Processing events multiple times when re-joining rooms. - Always thinking we've not processed an event/constraint violations (arguments were reversed in record_event). - Not handling errors when fetchin users in a room, and instead just suppressing them. Now, we handle errors! - Also update dependencies (attempt to fix ID too big bug, but no fix). --- Cargo.lock | 38 +++++++++++++++++++------------------- src/bot.rs | 13 ------------- src/bot/event_handlers.rs | 21 ++++++++++++++------- src/db/sqlite/rooms.rs | 9 +++------ 4 files changed, 36 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3c6318c..2c5c2a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -342,9 +342,9 @@ checksum = "ea221b5284a47e40033bf9b66f35f984ec0ea2931eb03505246cd27a963f981b" [[package]] name = "cpufeatures" -version = "0.1.3" +version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "281f563b2c3a0e535ab12d81d3c5859045795256ad269afa7c19542585b68f93" +checksum = "ed00c67cb5d0a7d64a44f6ad2668db7e7530311dd53ea79bcd4fb022c64911c8" dependencies = [ "libc", ] @@ -1123,7 +1123,7 @@ checksum = "7ffc5c5338469d4d3ea17d269fa8ea3512ad247247c30bd2df69e68309ed0a08" [[package]] name = "matrix-sdk" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#0bdcc0fbf90286b7555fda48403151f02dde6717" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" dependencies = [ "backoff", "bytes", @@ -1147,7 +1147,7 @@ dependencies = [ [[package]] name = "matrix-sdk-base" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#0bdcc0fbf90286b7555fda48403151f02dde6717" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" dependencies = [ "chacha20poly1305", "dashmap", @@ -1170,7 +1170,7 @@ dependencies = [ [[package]] name = "matrix-sdk-common" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#0bdcc0fbf90286b7555fda48403151f02dde6717" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" dependencies = [ "async-trait", "futures", @@ -1186,7 +1186,7 @@ dependencies = [ [[package]] name = "matrix-sdk-crypto" version = "0.2.0" -source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#0bdcc0fbf90286b7555fda48403151f02dde6717" +source = "git+https://github.com/matrix-org/matrix-rust-sdk?branch=master#ffea84b64aa172b9b153a76588f4f609bf15c441" dependencies = [ "aes-ctr", "aes-gcm", @@ -1845,7 +1845,7 @@ dependencies = [ [[package]] name = "ruma" version = "0.0.3" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "assign", "js_int", @@ -1861,7 +1861,7 @@ dependencies = [ [[package]] name = "ruma-api" version = "0.17.0-alpha.4" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "bytes", "http", @@ -1877,7 +1877,7 @@ dependencies = [ [[package]] name = "ruma-api-macros" version = "0.17.0-alpha.4" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -1888,7 +1888,7 @@ dependencies = [ [[package]] name = "ruma-client-api" version = "0.10.0-alpha.3" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "assign", "bytes", @@ -1908,7 +1908,7 @@ dependencies = [ [[package]] name = "ruma-common" version = "0.5.0" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "indexmap", "js_int", @@ -1923,7 +1923,7 @@ dependencies = [ [[package]] name = "ruma-events" version = "0.22.0-alpha.3" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "indoc", "js_int", @@ -1938,7 +1938,7 @@ dependencies = [ [[package]] name = "ruma-events-macros" version = "0.22.0-alpha.3" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -1949,7 +1949,7 @@ dependencies = [ [[package]] name = "ruma-federation-api" version = "0.1.0-alpha.2" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "js_int", "ruma-api", @@ -1964,7 +1964,7 @@ dependencies = [ [[package]] name = "ruma-identifiers" version = "0.19.0" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "paste", "ruma-identifiers-macros", @@ -1977,7 +1977,7 @@ dependencies = [ [[package]] name = "ruma-identifiers-macros" version = "0.19.0" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "quote", "ruma-identifiers-validation", @@ -1987,12 +1987,12 @@ dependencies = [ [[package]] name = "ruma-identifiers-validation" version = "0.3.0" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" [[package]] name = "ruma-serde" version = "0.3.1" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "bytes", "form_urlencoded", @@ -2006,7 +2006,7 @@ dependencies = [ [[package]] name = "ruma-serde-macros" version = "0.3.1" -source = "git+https://github.com/ruma/ruma?rev=3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5#3bdead1cf207e3ab9c8fcbfc454c054c726ba6f5" +source = "git+https://github.com/ruma/ruma?rev=e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6#e1ab817e0bef78cb8241d6d3c1ced7d6b414c7f6" dependencies = [ "proc-macro-crate", "proc-macro2", diff --git a/src/bot.rs b/src/bot.rs index 926a76a..a208882 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -161,22 +161,9 @@ impl DiceBot { let client = self.client.clone(); self.login(&client).await?; - // Initial sync without event handler prevents responding to - // messages received while bot was offline. TODO: selectively - // respond to old messages? e.g. comands missed while offline. - //info!("Performing intial sync (no commands will be responded to)"); - // self.client.sync_once(SyncSettings::default()).await?; - client.set_event_handler(Box::new(self)).await; info!("Listening for commands"); - // let token = client - // .sync_token() - // .await - // .ok_or(BotError::SyncTokenRequired)?; - - // let settings = SyncSettings::default().token(token); - // TODO replace with sync_with_callback for cleaner shutdown // process. client.sync(SyncSettings::default()).await; diff --git a/src/bot/event_handlers.rs b/src/bot/event_handlers.rs index 24ac3c1..7ffa293 100644 --- a/src/bot/event_handlers.rs +++ b/src/bot/event_handlers.rs @@ -20,9 +20,9 @@ use matrix_sdk::{ room::Room, EventHandler, }; -use std::clone::Clone; use std::ops::Sub; use std::time::{Duration, SystemTime}; +use std::{clone::Clone, time::UNIX_EPOCH}; /// Check if a message is recent enough to actually process. If the /// message is within "oldest_message_age" seconds, this function @@ -32,7 +32,11 @@ fn check_message_age( event: &SyncMessageEvent, oldest_message_age: u64, ) -> bool { - let sending_time = event.origin_server_ts; + let sending_time = event + .origin_server_ts + .to_system_time() + .unwrap_or(UNIX_EPOCH); + let oldest_timestamp = SystemTime::now().sub(Duration::from_secs(oldest_message_age)); if sending_time > oldest_timestamp { @@ -127,17 +131,20 @@ impl EventHandler for DiceBot { false }; + // user_joing is true if a user is joining this room, and + // false if they have left for some reason. This user may be + // us, or another user in the room. use MembershipChange::*; - let adding_user = match event.membership_change() { + let user_joining = match event.membership_change() { Joined => true, Banned | Left | Kicked | KickedAndBanned => false, _ => return, }; - let result = if event_affects_us && !adding_user { + let result = if event_affects_us && !user_joining { info!("Clearing all information for room ID {}", room_id); self.db.clear_info(room_id_str).await.map_err(|e| e.into()) - } else if event_affects_us && adding_user { + } else if event_affects_us && user_joining { info!("Joined room {}; recording room information", room_id); record_room_information( &self.client, @@ -147,13 +154,13 @@ impl EventHandler for DiceBot { &event.state_key, ) .await - } else if !event_affects_us && adding_user { + } else if !event_affects_us && user_joining { info!("Adding user {} to room ID {}", username, room_id); self.db .add_user_to_room(username, room_id_str) .await .map_err(|e| e.into()) - } else if !event_affects_us && !adding_user { + } else if !event_affects_us && !user_joining { info!("Removing user {} from room ID {}", username, room_id); self.db .remove_user_from_room(username, room_id_str) diff --git a/src/db/sqlite/rooms.rs b/src/db/sqlite/rooms.rs index 9078a79..298c911 100644 --- a/src/db/sqlite/rooms.rs +++ b/src/db/sqlite/rooms.rs @@ -6,7 +6,7 @@ use sqlx::SqlitePool; use std::collections::{HashMap, HashSet}; use std::time::{SystemTime, UNIX_EPOCH}; -async fn record_event(conn: &SqlitePool, event_id: &str, room_id: &str) -> Result<(), DataError> { +async fn record_event(conn: &SqlitePool, room_id: &str, event_id: &str) -> Result<(), DataError> { use std::convert::TryFrom; let now: i64 = i64::try_from( SystemTime::now() @@ -122,6 +122,8 @@ impl Rooms for Database { } async fn clear_info(&self, room_id: &str) -> Result<(), DataError> { + // We do not clear event history here, because if we rejoin a + // room, we would re-process events we've already seen. sqlx::query("DELETE FROM room_info where room_id = ?") .bind(room_id) .execute(&self.conn) @@ -132,11 +134,6 @@ impl Rooms for Database { .execute(&self.conn) .await?; - sqlx::query("DELETE FROM room_events where room_id = ?") - .bind(room_id) - .execute(&self.conn) - .await?; - Ok(()) } }