From 890ebf2651d246bca0c1b81be4e30082a0be765c Mon Sep 17 00:00:00 2001 From: iequidoo Date: Thu, 30 Apr 2026 11:23:42 -0300 Subject: [PATCH 1/2] test: Add function to queue MDN into `smtp` --- src/smtp.rs | 108 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 72 insertions(+), 36 deletions(-) diff --git a/src/smtp.rs b/src/smtp.rs index 5065fbdf87..987375a4e6 100644 --- a/src/smtp.rs +++ b/src/smtp.rs @@ -492,7 +492,39 @@ async fn send_mdns(context: &Context, connection: &mut Smtp) -> Result<()> { return Ok(()); } - let more_mdns = send_mdn(context, connection).await?; + let more_mdns = send_mdn( + context, + connection, + async |context, rfc724_mid, body, recipients, connection| { + let recipients: Vec<_> = recipients + .into_iter() + .filter_map(|addr| { + async_smtp::EmailAddress::new(addr.clone()) + .with_context(|| format!("Invalid recipient: {addr}")) + .log_err(context) + .ok() + }) + .collect(); + + match smtp_send(context, &recipients, &body, connection, None).await { + SendResult::Success => { + if !recipients.is_empty() { + info!(context, "Successfully sent MDN for {rfc724_mid}."); + } + Ok(true) + } + SendResult::Retry => { + info!( + context, + "Temporary SMTP failure while sending an MDN for {rfc724_mid}." + ); + Ok(false) + } + SendResult::Failure(err) => Err(err), + } + }, + ) + .await?; if !more_mdns { // No more MDNs to send or one of them failed. return Ok(()); @@ -551,6 +583,7 @@ async fn send_mdn_rfc724_mid( rfc724_mid: &str, contact_id: ContactId, smtp: &mut Smtp, + send_fn: impl AsyncFn(&Context, &str, String, Vec, &mut Smtp) -> Result, ) -> Result { let contact = Contact::get_by_id(context, contact_id).await?; if contact.is_blocked() { @@ -590,48 +623,51 @@ async fn send_mdn_rfc724_mid( if context.get_config_bool(Config::BccSelf).await? { add_self_recipients(context, &mut recipients, encrypted).await?; } - let recipients: Vec<_> = recipients - .into_iter() - .filter_map(|addr| { - async_smtp::EmailAddress::new(addr.clone()) - .with_context(|| format!("Invalid recipient: {addr}")) - .log_err(context) - .ok() - }) - .collect(); + let sent = send_fn(context, rfc724_mid, body, recipients, smtp).await?; + if sent { + context + .sql + .transaction(|transaction| { + let mut stmt = transaction.prepare("DELETE FROM smtp_mdns WHERE rfc724_mid = ?")?; + stmt.execute((rfc724_mid,))?; + for additional_rfc724_mid in additional_rfc724_mids { + stmt.execute((additional_rfc724_mid,))?; + } + Ok(()) + }) + .await?; + } + Ok(sent) +} - match smtp_send(context, &recipients, &body, smtp, None).await { - SendResult::Success => { - if !recipients.is_empty() { - info!(context, "Successfully sent MDN for {rfc724_mid}."); - } +#[cfg(test)] +pub(crate) async fn queue_mdn(context: &Context) -> Result<()> { + let queued = send_mdn( + context, + &mut Smtp::new(), + async |context, rfc724_mid, body, recipients, _smtp| { context .sql - .transaction(|transaction| { - let mut stmt = - transaction.prepare("DELETE FROM smtp_mdns WHERE rfc724_mid = ?")?; - stmt.execute((rfc724_mid,))?; - for additional_rfc724_mid in additional_rfc724_mids { - stmt.execute((additional_rfc724_mid,))?; - } - Ok(()) - }) + .execute( + "INSERT INTO smtp (rfc724_mid, recipients, mime, msg_id) + VALUES (?, ?, ?, ?)", + (rfc724_mid, recipients.join(" "), body, u32::MAX), + ) .await?; Ok(true) - } - SendResult::Retry => { - info!( - context, - "Temporary SMTP failure while sending an MDN for {rfc724_mid}." - ); - Ok(false) - } - SendResult::Failure(err) => Err(err), - } + }, + ) + .await?; + assert!(queued); + Ok(()) } /// Tries to send a single MDN. Returns true if more MDNs should be sent. -async fn send_mdn(context: &Context, smtp: &mut Smtp) -> Result { +async fn send_mdn( + context: &Context, + smtp: &mut Smtp, + send_fn: impl AsyncFn(&Context, &str, String, Vec, &mut Smtp) -> Result, +) -> Result { if !context.should_send_mdns().await? { context.sql.execute("DELETE FROM smtp_mdns", []).await?; return Ok(false); @@ -668,7 +704,7 @@ async fn send_mdn(context: &Context, smtp: &mut Smtp) -> Result { .await .context("Failed to update MDN retries count")?; - match send_mdn_rfc724_mid(context, &rfc724_mid, contact_id, smtp).await { + match send_mdn_rfc724_mid(context, &rfc724_mid, contact_id, smtp, send_fn).await { Err(err) => { // If there is an error, for example there is no message corresponding to the msg_id in the // database, do not try to send this MDN again. From 6c2b2b56888c4e9a9f25f1b5129745eef0e3bfe0 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Fri, 29 May 2026 18:12:59 -0300 Subject: [PATCH 2/2] fix: Return MessageState::OutMdnRcvd for 1:1 failed-to-deliver messages with MDNs (#8241) The docs for `OutFailed` say that it's for unrecoverable errors, but if a message failed to be delivered to one relay of the contact, but was delivered to another relay and an MDN was received, it's rather an expected condition and there's no much sense in displaying the message as failed. MDNs are enabled by default, so this should work for 1:1 chats for most users. For group chats this isn't easy to fix however, a message may fail to be delivered for one member, but there may be MDNs from other members. --- src/message.rs | 32 ++++++++++++++++++++++---------- src/message/message_tests.rs | 8 ++++++++ 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/message.rs b/src/message.rs index 306f19258e..5089d4b9e5 100644 --- a/src/message.rs +++ b/src/message.rs @@ -85,15 +85,19 @@ impl MsgId { let result = context .sql .query_row_optional( - "SELECT m.state, mdns.msg_id - FROM msgs m LEFT JOIN msgs_mdns mdns ON mdns.msg_id=m.id - WHERE id=? - LIMIT 1", + " +SELECT m.state, mdns.msg_id, c.type +FROM msgs m +LEFT JOIN msgs_mdns mdns ON mdns.msg_id=m.id +LEFT JOIN chats c ON c.id=m.chat_id +WHERE m.id=? +LIMIT 1", (self,), |row| { let state: MessageState = row.get(0)?; let mdn_msg_id: Option = row.get(1)?; - Ok(state.with_mdns(mdn_msg_id.is_some())) + let chat_type: Option = row.get(2)?; + Ok(state.with_mdns(mdn_msg_id.is_some(), chat_type)) }, ) .await? @@ -537,6 +541,7 @@ impl Message { m.param AS param, m.hidden AS hidden, m.location_id AS location, + c.type AS chat_type, c.archived AS visibility, c.blocked AS blocked FROM msgs m @@ -548,6 +553,7 @@ impl Message { |row| { let state: MessageState = row.get("state")?; let mdn_msg_id: Option = row.get("mdn_msg_id")?; + let chat_type: Option = row.get("chat_type")?; let text = match row.get_ref("txt")? { rusqlite::types::ValueRef::Text(buf) => { match String::from_utf8(buf.to_vec()) { @@ -583,7 +589,7 @@ impl Message { ephemeral_timer: row.get("ephemeral_timer")?, ephemeral_timestamp: row.get("ephemeral_timestamp")?, viewtype: row.get("type").unwrap_or_default(), - state: state.with_mdns(mdn_msg_id.is_some()), + state: state.with_mdns(mdn_msg_id.is_some(), chat_type), download_state: row.get("download_state")?, error: Some(row.get::<_, String>("error")?) .filter(|error| !error.is_empty()), @@ -1486,11 +1492,17 @@ impl MessageState { } /// Returns adjusted message state if the message has MDNs. - pub(crate) fn with_mdns(self, has_mdns: bool) -> Self { - if self == MessageState::OutDelivered && has_mdns { - return MessageState::OutMdnRcvd; + pub(crate) fn with_mdns(self, has_mdns: bool, chat_type: Option) -> Self { + if !has_mdns { + return self; + } + match self { + MessageState::OutFailed if chat_type == Some(Chattype::Single) => { + MessageState::OutMdnRcvd + } + MessageState::OutDelivered => MessageState::OutMdnRcvd, + _ => self, } - self } } diff --git a/src/message/message_tests.rs b/src/message/message_tests.rs index 407e247e43..b2d03b7575 100644 --- a/src/message/message_tests.rs +++ b/src/message/message_tests.rs @@ -6,6 +6,7 @@ use crate::chatlist::Chatlist; use crate::config::Config; use crate::reaction::send_reaction; use crate::receive_imf::receive_imf; +use crate::smtp; use crate::test_utils; use crate::test_utils::{E2EE_INFO_MSGS, TestContext, TestContextManager}; @@ -450,6 +451,13 @@ async fn test_get_state() -> Result<()> { markseen_msgs(&bob, vec![bob_msg.id]).await?; assert_state(&bob, bob_msg.id, MessageState::InSeen).await; + smtp::queue_mdn(&bob).await?; + let sent = bob.pop_sent_msg().await; + alice.recv_msg_trash(&sent).await; + let alice_msg = Message::load_from_db(&alice, alice_msg.id).await?; + assert_eq!(alice_msg.get_state(), MessageState::OutMdnRcvd); + assert_eq!(alice_msg.error().unwrap(), "badly failed"); + Ok(()) }