From 070e670d3a514f50216af23059ad8481a015e51c 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 get sent MDN as SentMessage --- src/smtp.rs | 89 ++++++++++++++++++++++++++--------------------- src/test_utils.rs | 18 +++++++++- 2 files changed, 66 insertions(+), 41 deletions(-) diff --git a/src/smtp.rs b/src/smtp.rs index 7f1bdf1dc8..cf983a18ab 100644 --- a/src/smtp.rs +++ b/src/smtp.rs @@ -492,7 +492,35 @@ 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, async |rfc724_mid, body, recipients| { + 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(()); @@ -550,7 +578,7 @@ async fn send_mdn_rfc724_mid( context: &Context, rfc724_mid: &str, contact_id: ContactId, - smtp: &mut Smtp, + send_fn: impl AsyncFnOnce(&str, String, Vec) -> Result, ) -> Result { let contact = Contact::get_by_id(context, contact_id).await?; if contact.is_blocked() { @@ -590,48 +618,29 @@ 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(); message::insert_tombstone(context, &rendered_msg.rfc724_mid).await?; - match smtp_send(context, &recipients, &body, smtp, None).await { - SendResult::Success => { - if !recipients.is_empty() { - info!(context, "Successfully sent MDN for {rfc724_mid}."); - } - 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(true) - } - SendResult::Retry => { - info!( - context, - "Temporary SMTP failure while sending an MDN for {rfc724_mid}." - ); - Ok(false) - } - SendResult::Failure(err) => Err(err), + let sent = send_fn(rfc724_mid, body, recipients).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) } /// Tries to send a single MDN. Returns true if more MDNs should be sent. -async fn send_mdn(context: &Context, smtp: &mut Smtp) -> Result { +pub(crate) async fn send_mdn( + context: &Context, + send_fn: impl AsyncFnOnce(&str, String, Vec) -> Result, +) -> Result { if !context.should_send_mdns().await? { context.sql.execute("DELETE FROM smtp_mdns", []).await?; return Ok(false); @@ -668,7 +677,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, 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. diff --git a/src/test_utils.rs b/src/test_utils.rs index f5f245c8a3..cbbfad2024 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -43,7 +43,7 @@ use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::pgp::SeipdVersion; use crate::receive_imf::{ReceivedMsg, receive_imf}; use crate::securejoin::{get_securejoin_qr, join_securejoin}; -use crate::smtp::msg_has_pending_smtp_job; +use crate::smtp::{self, msg_has_pending_smtp_job}; use crate::stock_str::StockStrings; use crate::tools::time; @@ -1080,6 +1080,22 @@ ORDER BY id" res } + pub async fn get_sent_mdn(&self) -> SentMessage<'_> { + let mut sent_mdn = None; + smtp::send_mdn(self, async |_rfc724_mid, body, recipients| { + sent_mdn = Some(SentMessage { + payload: body, + sender_msg_id: MsgId::new(u32::MAX), + sender_context: &self.ctx, + recipients: recipients.join(" "), + }); + Ok(true) + }) + .await + .expect("smtp::send_mdn"); + sent_mdn.unwrap() + } + pub async fn golden_test_chat(&self, chat_id: ChatId, filename: &str) { let filename = Path::new("test-data/golden/").join(filename); From 0dfe92ffbfbb04e1d05498ab906829e5295abf72 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 | 6 ++++++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/message.rs b/src/message.rs index 4a80b57442..5bf0aa4576 100644 --- a/src/message.rs +++ b/src/message.rs @@ -84,15 +84,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? @@ -536,6 +540,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 @@ -547,6 +552,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()) { @@ -582,7 +588,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()), @@ -1485,11 +1491,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..47024aa7bf 100644 --- a/src/message/message_tests.rs +++ b/src/message/message_tests.rs @@ -450,6 +450,12 @@ async fn test_get_state() -> Result<()> { markseen_msgs(&bob, vec![bob_msg.id]).await?; assert_state(&bob, bob_msg.id, MessageState::InSeen).await; + let sent = bob.get_sent_mdn().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(()) }