From ac34759b02620d0b8310f9a793a2ec6e51afacbc Mon Sep 17 00:00:00 2001 From: YoursFunny Date: Thu, 24 Sep 2026 15:21:05 +0800 Subject: [PATCH] fix(state): report a failed chat-state write to /set_format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit set()/update() logged a failed DB write and returned nothing: the cache already held the new value, so /set_format answered 'Format set' for a change that vanishes on the next restart — a promise the retry queue's enqueue path is already forbidden from making. Both now return whether the write landed, and /set_format's two save paths (set and reset) answer 'in memory only … lost on restart' when it did not. The flag travels as the second tuple element; callers that only relay the value destructure it, callers that ignore it are untouched. A false cannot be forced in a test without a pool-failure seam, so the honest path is pinned by the code, not a fixture. --- crates/xmedia-bot/src/handlers/commands.rs | 24 +++++++++++++++------- crates/xmedia-bot/src/state.rs | 22 ++++++++++++-------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/crates/xmedia-bot/src/handlers/commands.rs b/crates/xmedia-bot/src/handlers/commands.rs index 65c334e..d984319 100644 --- a/crates/xmedia-bot/src/handlers/commands.rs +++ b/crates/xmedia-bot/src/handlers/commands.rs @@ -323,7 +323,7 @@ pub(crate) async fn execute_command( } Command::RemoveForwardChannel => { let chat_id = message.chat.id.0; - let text = ctx + let (text, _) = ctx .chat_store .update(chat_id, |data| { if data.forward_channel_id.is_some() { @@ -338,7 +338,7 @@ pub(crate) async fn execute_command( } Command::EditBeforeForward => { let chat_id = message.chat.id.0; - let text = ctx + let (text, _) = ctx .chat_store .update(chat_id, |data| { if data.forward_channel_id.is_none() { @@ -393,7 +393,7 @@ pub(crate) async fn execute_command( .await?; return Ok(()); } - let removed = ctx + let (removed, _) = ctx .chat_store .update(chat_id, |data| data.template.remove(&name).is_some()) .await; @@ -461,7 +461,8 @@ pub(crate) async fn execute_command( // set a format once could never get back to the default (the // built-in format string is not something a user can retype). if format == "-" { - ctx.chat_store + let (_, saved) = ctx + .chat_store .update(chat_id, |data| { data.message_format.remove(site); }) @@ -470,7 +471,11 @@ pub(crate) async fn execute_command( ctx.sender, message.chat.id.0, message.id, - "Format reset to the built-in one.", + if saved { + "Format reset to the built-in one.".to_string() + } else { + "Reset in memory only: the database write failed, so it will be lost on restart.".to_string() + }, ) .await?; return Ok(()); @@ -495,7 +500,8 @@ pub(crate) async fn execute_command( .await?; return Ok(()); } - ctx.chat_store + let (_, saved) = ctx + .chat_store .update(chat_id, |data| { data.message_format.insert(site.to_string(), format); }) @@ -504,7 +510,11 @@ pub(crate) async fn execute_command( ctx.sender, message.chat.id.0, message.id, - "Format set. Use /debug to preview the caption.", + if saved { + "Format set. Use /debug to preview the caption.".to_string() + } else { + "Set in memory only: the database write failed, so it will be lost on restart. Use /debug to preview the caption.".to_string() + }, ) .await?; } diff --git a/crates/xmedia-bot/src/state.rs b/crates/xmedia-bot/src/state.rs index e92f18c..85df662 100644 --- a/crates/xmedia-bot/src/state.rs +++ b/crates/xmedia-bot/src/state.rs @@ -105,8 +105,11 @@ impl ChatStore { data } - /// Write-through: update the cache and the DB. - pub async fn set(&self, chat_id: i64, data: &ChatData) { + /// Write-through: update the cache and the DB. Returns whether the DB + /// write landed: the cache is updated either way, so `false` means the + /// change lives only until the next restart and the caller has to say so + /// instead of reporting a save that did not happen. + pub async fn set(&self, chat_id: i64, data: &ChatData) -> bool { self.cache.lock().insert(chat_id, data.clone()); let payload = serde_json::to_string(data).expect("chat state serializes"); let chat_id = chat_id.to_string(); @@ -114,16 +117,16 @@ impl ChatStore { .with_conn_or( log::Level::Warn, "chat_state write failed", - (), + false, move |conn| { conn.execute( "INSERT OR REPLACE INTO chat_state (chat_id, payload) VALUES (?1, ?2)", params![chat_id, payload], )?; - Ok(()) + Ok(true) }, ) - .await; + .await } /// The per-chat async lock serializing get→mutate→set cycles. @@ -139,14 +142,15 @@ impl ChatStore { /// (the batch-forward design spawns several per chat) each snapshot the /// same `ChatData` and last-writer-wins would silently drop mutations, /// e.g. a second `edit_message` record. The per-chat lock makes the - /// cycle atomic. Returns the closure's result. - pub async fn update(&self, chat_id: i64, f: impl FnOnce(&mut ChatData) -> R) -> R { + /// cycle atomic. Returns the closure's result plus whether the DB write + /// landed (see [`Self::set`]); callers that do not care ignore the flag. + pub async fn update(&self, chat_id: i64, f: impl FnOnce(&mut ChatData) -> R) -> (R, bool) { let lock = self.lock_for(chat_id); let _guard = lock.lock().await; let mut data = self.get(chat_id).await; let r = f(&mut data); - self.set(chat_id, &data).await; - r + let saved = self.set(chat_id, &data).await; + (r, saved) } /// Removes edit-before-forward records whose `created_at + ttl` is in the