From 626ac8161a490f001253d6efe3ed6ffcc9e4642e Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 2 Apr 2026 15:12:17 +0200 Subject: [PATCH] fix: Mark a message as delivered only after it has been fully sent out (#8062) Fix https://github.com/chatmail/core/issues/8042 The problem was that after receiving the bcc_self'ed pre-message in `receive_imf`, the logic there only looked for a pending `smtp`-table-entry that matches the rfc724_mid, and if there was none then it thought "Great, apparently the message is fully sent out, we can mark it as delivered!". But with pre-messages, the same message can have two `smtp` entries (one for the pre-message and one for the post-message), and the message should only be marked as delivered once both of them are sent out. Now, I changed the logic to look for all entries with the same msg_id. This is actually the same SQL query used in smtp.rs, so, I extracted it into a new function; feel free to suggest a better name for it. I tested on Android that it now works fine. I'll add a test in a follow-up PR. There are a lot of other problems with sending large files, though: - The pre-message is sent before the post-message, so that for the receiver it looks as if the message arrived, but stays in "downloading..." forever - There is quite a time delay between clicking on "Send" and the outgoing message appearing in the chat - The message shortly gets a letter icon right after it is sent - I'm wondering if there is a way to give feedback to the user immediately if the message is too big - It's unclear when exactly we want to send read receipts I'll open a follow-up issue for these. --- src/receive_imf.rs | 10 ++-------- src/smtp.rs | 16 +++++++++++----- src/test_utils.rs | 6 ++---- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index f7ba9bc2f..4aadc0f1e 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -45,6 +45,7 @@ use crate::securejoin::{ self, get_secure_join_step, handle_securejoin_handshake, observe_securejoin_on_other_device, }; use crate::simplify; +use crate::smtp::msg_has_pending_smtp_job; use crate::stats::STATISTICS_BOT_EMAIL; use crate::stock_str; use crate::sync::Sync::*; @@ -582,14 +583,7 @@ pub(crate) async fn receive_imf_inner( (rfc724_mid_orig, &self_addr), ) .await?; - if !context - .sql - .exists( - "SELECT COUNT(*) FROM smtp WHERE rfc724_mid=?", - (rfc724_mid_orig,), - ) - .await? - { + if !msg_has_pending_smtp_job(context, msg_id).await? { msg_id.set_delivered(context).await?; } return Ok(None); diff --git a/src/smtp.rs b/src/smtp.rs index aca1f46ea..02f277410 100644 --- a/src/smtp.rs +++ b/src/smtp.rs @@ -465,11 +465,7 @@ pub(crate) async fn send_msg_to_smtp( match status { SendResult::Retry => Err(format_err!("Retry")), SendResult::Success => { - if !context - .sql - .exists("SELECT COUNT(*) FROM smtp WHERE msg_id=?", (msg_id,)) - .await? - { + if !msg_has_pending_smtp_job(context, msg_id).await? { msg_id.set_delivered(context).await?; } Ok(()) @@ -478,6 +474,16 @@ pub(crate) async fn send_msg_to_smtp( } } +pub(crate) async fn msg_has_pending_smtp_job( + context: &Context, + msg_id: MsgId, +) -> Result { + context + .sql + .exists("SELECT COUNT(*) FROM smtp WHERE msg_id=?", (msg_id,)) + .await +} + /// Attempts to send queued MDNs. async fn send_mdns(context: &Context, connection: &mut Smtp) -> Result<()> { loop { diff --git a/src/test_utils.rs b/src/test_utils.rs index 8491dda86..ea1434a59 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -40,6 +40,7 @@ use crate::message::{Message, MessageState, MsgId, update_msg_state}; use crate::mimeparser::{MimeMessage, SystemMessage}; use crate::receive_imf::receive_imf; use crate::securejoin::{get_securejoin_qr, join_securejoin}; +use crate::smtp::msg_has_pending_smtp_job; use crate::stock_str::StockStrings; use crate::tools::time; @@ -658,10 +659,7 @@ impl TestContext { .execute("DELETE FROM smtp WHERE id=?;", (rowid,)) .await .expect("failed to remove job"); - if !self - .ctx - .sql - .exists("SELECT COUNT(*) FROM smtp WHERE msg_id=?", (msg_id,)) + if !msg_has_pending_smtp_job(self, msg_id) .await .expect("Failed to check for more jobs") {