diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index 8e1301c770..f7c144a4a4 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -331,6 +331,52 @@ def test_message(acfactory) -> None: assert reactions == snapshot.reactions +def test_receive_imf_failure(acfactory) -> None: + alice, bob = acfactory.get_online_accounts(2) + alice_contact_bob = alice.create_contact(bob, "Bob") + alice_chat_bob = alice_contact_bob.create_chat() + + bob.set_config("fail_on_receiving_full_msg", "1") + alice_chat_bob.send_text("Hello!") + event = bob.wait_for_incoming_msg_event() + chat_id = event.chat_id + msg_id = event.msg_id + message = bob.get_message_by_id(msg_id) + snapshot = message.get_snapshot() + assert snapshot.chat_id == chat_id + assert snapshot.download_state == DownloadState.AVAILABLE + assert snapshot.error is not None + assert snapshot.show_padlock + + # The failed message doesn't break the IMAP loop. + bob.set_config("fail_on_receiving_full_msg", "0") + alice_chat_bob.send_text("Hello again!") + event = bob.wait_for_incoming_msg_event() + assert event.chat_id == chat_id + msg_id = event.msg_id + message1 = bob.get_message_by_id(msg_id) + snapshot = message1.get_snapshot() + assert snapshot.chat_id == chat_id + assert snapshot.download_state == DownloadState.DONE + assert snapshot.error is None + + # The failed message can be re-downloaded later. + bob._rpc.download_full_message(bob.id, message.id) + event = bob.wait_for_event(EventType.MSGS_CHANGED) + message = bob.get_message_by_id(event.msg_id) + snapshot = message.get_snapshot() + assert snapshot.download_state == DownloadState.IN_PROGRESS + event = bob.wait_for_event(EventType.MSGS_CHANGED) + assert event.chat_id == chat_id + msg_id = event.msg_id + message = bob.get_message_by_id(msg_id) + snapshot = message.get_snapshot() + assert snapshot.chat_id == chat_id + assert snapshot.download_state == DownloadState.DONE + assert snapshot.error is None + assert snapshot.text == "Hello!" + + def test_selfavatar_sync(acfactory, data, log) -> None: alice = acfactory.get_online_account() diff --git a/src/config.rs b/src/config.rs index 02ad57374f..fa5acc0bd8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -450,6 +450,9 @@ pub enum Config { /// to avoid encrypting it differently and /// storing the same token multiple times on the server. EncryptedDeviceToken, + + /// Return an error from `receive_imf_inner()` for a fully downloaded message. For tests. + FailOnReceivingFullMsg, } impl Config { diff --git a/src/context.rs b/src/context.rs index c94780a8dd..e44f3a46b0 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1079,6 +1079,13 @@ impl Context { .await? .unwrap_or_default(), ); + res.insert( + "fail_on_receiving_full_msg", + self.sql + .get_raw_config("fail_on_receiving_full_msg") + .await? + .unwrap_or_default(), + ); let elapsed = time_elapsed(&self.creation_time); res.insert("uptime", duration_to_str(elapsed)); diff --git a/src/download.rs b/src/download.rs index 57086a43dc..3f10263279 100644 --- a/src/download.rs +++ b/src/download.rs @@ -238,14 +238,20 @@ impl MimeMessage { /// the mime-structure itself is not available. /// /// The placeholder part currently contains a text with size and availability of the message; + /// `error` is set as the part error; /// in the future, we may do more advanced things as previews here. pub(crate) async fn create_stub_from_partial_download( &mut self, context: &Context, org_bytes: u32, + error: Option, ) -> Result<()> { + let prefix = match error { + None => "", + Some(_) => "[❗] ", + }; let mut text = format!( - "[{}]", + "{prefix}[{}]", stock_str::partial_download_msg_body(context, org_bytes).await ); if let Some(delete_server_after) = context.get_config_delete_server_after().await? { @@ -259,9 +265,10 @@ impl MimeMessage { info!(context, "Partial download: {}", text); - self.parts.push(Part { + self.do_add_single_part(Part { typ: Viewtype::Text, msg: text, + error, ..Default::default() }); diff --git a/src/imap.rs b/src/imap.rs index b29422ac94..e10be9e4ae 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -1480,7 +1480,7 @@ impl Session { context, "Passing message UID {} to receive_imf().", request_uid ); - match receive_imf_inner( + let res = receive_imf_inner( context, folder, uidvalidity, @@ -1488,20 +1488,31 @@ impl Session { rfc724_mid, body, is_seen, - partial, + partial.map(|msg_size| (msg_size, None)), ) - .await - { - Ok(received_msg) => { - received_msgs_channel - .send((request_uid, received_msg)) - .await?; - } - Err(err) => { - warn!(context, "receive_imf error: {:#}.", err); - received_msgs_channel.send((request_uid, None)).await?; + .await; + let received_msg = if let Err(err) = res { + warn!(context, "receive_imf error: {:#}.", err); + if partial.is_some() { + return Err(err); } + receive_imf_inner( + context, + folder, + uidvalidity, + request_uid, + rfc724_mid, + body, + is_seen, + Some((body.len().try_into()?, Some(format!("{err:#}")))), + ) + .await? + } else { + res? }; + received_msgs_channel + .send((request_uid, received_msg)) + .await?; } // If we don't process the whole response, IMAP client is left in a broken state where diff --git a/src/mimeparser.rs b/src/mimeparser.rs index ce4d104598..573e0c5994 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -240,12 +240,12 @@ const MIME_AC_SETUP_FILE: &str = "application/autocrypt-setup"; impl MimeMessage { /// Parse a mime message. /// - /// If `partial` is set, it contains the full message size in bytes - /// and `body` contains the header only. + /// If `partial` is set, it contains the full message size in bytes and an optional error text + /// for the partially downloaded message, and `body` contains the HEADER only. pub(crate) async fn from_bytes( context: &Context, body: &[u8], - partial: Option, + partial: Option<(u32, Option)>, ) -> Result { let mail = mailparse::parse_mail(body)?; @@ -612,9 +612,9 @@ impl MimeMessage { }; match partial { - Some(org_bytes) => { + Some((org_bytes, err)) => { parser - .create_stub_from_partial_download(context, org_bytes) + .create_stub_from_partial_download(context, org_bytes, err) .await?; } None => match mail { @@ -634,7 +634,7 @@ impl MimeMessage { error: Some(format!("Decrypting failed: {err:#}")), ..Default::default() }; - parser.parts.push(part); + parser.do_add_single_part(part); } }, }; @@ -1543,7 +1543,7 @@ impl MimeMessage { Ok(true) } - fn do_add_single_part(&mut self, mut part: Part) { + pub(crate) fn do_add_single_part(&mut self, mut part: Part) { if self.was_encrypted() { part.param.set_int(Param::GuaranteeE2ee, 1); } diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 86ae562d2a..3e9e3d762d 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -196,7 +196,7 @@ pub(crate) async fn receive_imf_from_inbox( rfc724_mid, imf_raw, seen, - is_partial_download, + is_partial_download.map(|msg_size| (msg_size, None)), ) .await } @@ -494,9 +494,8 @@ async fn get_to_and_past_contact_ids( /// If the message is so wrong that we didn't even create a database entry, /// returns `Ok(None)`. /// -/// If `is_partial_download` is set, it contains the full message size in bytes. -/// Do not confuse that with `replace_msg_id` that will be set when the full message is loaded -/// later. +/// If `partial` is set, it contains the full message size in bytes and an optional error text for +/// the partially downloaded message. #[expect(clippy::too_many_arguments)] pub(crate) async fn receive_imf_inner( context: &Context, @@ -506,7 +505,7 @@ pub(crate) async fn receive_imf_inner( rfc724_mid: &str, imf_raw: &[u8], seen: bool, - is_partial_download: Option, + partial: Option<(u32, Option)>, ) -> Result> { if std::env::var(crate::DCC_MIME_DEBUG).is_ok() { info!( @@ -515,9 +514,16 @@ pub(crate) async fn receive_imf_inner( String::from_utf8_lossy(imf_raw), ); } + if partial.is_none() { + ensure!( + !context + .get_config_bool(Config::FailOnReceivingFullMsg) + .await? + ); + } - let mut mime_parser = match MimeMessage::from_bytes(context, imf_raw, is_partial_download).await - { + let is_partial_download = partial.as_ref().map(|(msg_size, _err)| *msg_size); + let mut mime_parser = match MimeMessage::from_bytes(context, imf_raw, partial).await { Err(err) => { warn!(context, "receive_imf: can't parse MIME: {err:#}."); if rfc724_mid.starts_with(GENERATED_PREFIX) { @@ -551,22 +557,11 @@ pub(crate) async fn receive_imf_inner( // make sure, this check is done eg. before securejoin-processing. let (replace_msg_id, replace_chat_id); if let Some((old_msg_id, _)) = message::rfc724_mid_exists(context, rfc724_mid).await? { - if is_partial_download.is_some() { - // Should never happen, see imap::prefetch_should_download(), but still. - info!( - context, - "Got a partial download and message is already in DB." - ); - return Ok(None); - } let msg = Message::load_from_db(context, old_msg_id).await?; replace_msg_id = Some(old_msg_id); replace_chat_id = if msg.download_state() != DownloadState::Done { // the message was partially downloaded before and is fully downloaded now. - info!( - context, - "Message already partly in DB, replacing by full message." - ); + info!(context, "Message already partly in DB, replacing."); Some(msg.chat_id) } else { None @@ -1007,7 +1002,10 @@ pub(crate) async fn receive_imf_inner( } else if received_msg.hidden { // No need to emit an event about the changed message } else if let Some(replace_chat_id) = replace_chat_id { - context.emit_msgs_changed_without_msg_id(replace_chat_id); + match replace_chat_id == chat_id { + false => context.emit_msgs_changed_without_msg_id(replace_chat_id), + true => context.emit_msgs_changed(chat_id, replace_msg_id.unwrap_or_default()), + } } else if !chat_id.is_trash() { let fresh = received_msg.state == MessageState::InFresh && mime_parser.is_system_message != SystemMessage::CallAccepted