Skip to content

Commit 81ba2d2

Browse files
committed
fix: add device message instead of partial message when receive_imf fails
1 parent f04c881 commit 81ba2d2

File tree

5 files changed

+33
-77
lines changed

5 files changed

+33
-77
lines changed

deltachat-rpc-client/tests/test_something.py

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -338,44 +338,27 @@ def test_receive_imf_failure(acfactory) -> None:
338338

339339
bob.set_config("fail_on_receiving_full_msg", "1")
340340
alice_chat_bob.send_text("Hello!")
341-
event = bob.wait_for_incoming_msg_event()
342-
chat_id = event.chat_id
341+
event = bob.wait_for_event(EventType.MSGS_CHANGED)
342+
assert event.chat_id == bob.get_device_chat().id
343343
msg_id = event.msg_id
344344
message = bob.get_message_by_id(msg_id)
345345
snapshot = message.get_snapshot()
346-
assert snapshot.chat_id == chat_id
347-
assert snapshot.download_state == DownloadState.AVAILABLE
348-
assert snapshot.error is not None
349-
assert snapshot.show_padlock
350-
snapshot.chat.accept()
346+
assert (
347+
snapshot.text == "❌ Failed to receive a message:"
348+
" Condition failed: `!context.get_config_bool(Config::FailOnReceivingFullMsg).await?`."
349+
" Please report this bug to delta@merlinux.eu or https://support.delta.chat/."
350+
)
351351

352352
# The failed message doesn't break the IMAP loop.
353353
bob.set_config("fail_on_receiving_full_msg", "0")
354354
alice_chat_bob.send_text("Hello again!")
355355
event = bob.wait_for_incoming_msg_event()
356-
assert event.chat_id == chat_id
357-
msg_id = event.msg_id
358-
message1 = bob.get_message_by_id(msg_id)
359-
snapshot = message1.get_snapshot()
360-
assert snapshot.chat_id == chat_id
361-
assert snapshot.download_state == DownloadState.DONE
362-
assert snapshot.error is None
363-
364-
# The failed message can be re-downloaded later.
365-
bob._rpc.download_full_message(bob.id, message.id)
366-
event = bob.wait_for_event(EventType.MSGS_CHANGED)
367-
message = bob.get_message_by_id(event.msg_id)
368-
snapshot = message.get_snapshot()
369-
assert snapshot.download_state == DownloadState.IN_PROGRESS
370-
event = bob.wait_for_event(EventType.MSGS_CHANGED)
371-
assert event.chat_id == chat_id
372356
msg_id = event.msg_id
373357
message = bob.get_message_by_id(msg_id)
374358
snapshot = message.get_snapshot()
375-
assert snapshot.chat_id == chat_id
359+
assert snapshot.text == "Hello again!"
376360
assert snapshot.download_state == DownloadState.DONE
377361
assert snapshot.error is None
378-
assert snapshot.text == "Hello!"
379362

380363

381364
def test_selfavatar_sync(acfactory, data, log) -> None:

src/download.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -221,21 +221,14 @@ impl MimeMessage {
221221
/// To create the placeholder, only the outermost header can be used,
222222
/// the mime-structure itself is not available.
223223
///
224-
/// The placeholder part currently contains a text with size and availability of the message;
225-
/// `error` is set as the part error;
226-
/// in the future, we may do more advanced things as previews here.
224+
/// The placeholder part currently contains a text with size and availability of the message.
227225
pub(crate) async fn create_stub_from_partial_download(
228226
&mut self,
229227
context: &Context,
230228
org_bytes: u32,
231-
error: Option<String>,
232229
) -> Result<()> {
233-
let prefix = match error {
234-
None => "",
235-
Some(_) => "[❗] ",
236-
};
237230
let mut text = format!(
238-
"{prefix}[{}]",
231+
"[{}]",
239232
stock_str::partial_download_msg_body(context, org_bytes).await
240233
);
241234
if let Some(delete_server_after) = context.get_config_delete_server_after().await? {
@@ -252,7 +245,6 @@ impl MimeMessage {
252245
self.do_add_single_part(Part {
253246
typ: Viewtype::Text,
254247
msg: text,
255-
error,
256248
..Default::default()
257249
});
258250

src/imap.rs

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use ratelimit::Ratelimit;
2424
use url::Url;
2525

2626
use crate::calls::{create_fallback_ice_servers, create_ice_servers_from_metadata};
27-
use crate::chat::{self, ChatId, ChatIdBlocked};
27+
use crate::chat::{self, ChatId, ChatIdBlocked, add_device_msg};
2828
use crate::chatlist_events;
2929
use crate::config::Config;
3030
use crate::constants::{self, Blocked, Chattype, ShowEmails};
@@ -1468,29 +1468,19 @@ impl Session {
14681468
context,
14691469
"Passing message UID {} to receive_imf().", request_uid
14701470
);
1471-
let res = receive_imf_inner(
1472-
context,
1473-
rfc724_mid,
1474-
body,
1475-
is_seen,
1476-
partial.map(|msg_size| (msg_size, None)),
1477-
)
1478-
.await;
1479-
let received_msg = if let Err(err) = res {
1480-
warn!(context, "receive_imf error: {:#}.", err);
1481-
if partial.is_some() {
1482-
return Err(err);
1471+
let res = receive_imf_inner(context, rfc724_mid, body, is_seen, partial).await;
1472+
let received_msg = match res {
1473+
Err(err) => {
1474+
warn!(context, "receive_imf error: {err:#}.");
1475+
1476+
let text = format!(
1477+
"❌ Failed to receive a message: {err:#}. Please report this bug to delta@merlinux.eu or https://support.delta.chat/."
1478+
);
1479+
let mut msg = Message::new_text(text);
1480+
add_device_msg(context, None, Some(&mut msg)).await?;
1481+
None
14831482
}
1484-
receive_imf_inner(
1485-
context,
1486-
rfc724_mid,
1487-
body,
1488-
is_seen,
1489-
Some((body.len().try_into()?, Some(format!("{err:#}")))),
1490-
)
1491-
.await?
1492-
} else {
1493-
res?
1483+
Ok(msg) => msg,
14941484
};
14951485
received_msgs_channel
14961486
.send((request_uid, received_msg))

src/mimeparser.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,12 +240,11 @@ const MIME_AC_SETUP_FILE: &str = "application/autocrypt-setup";
240240
impl MimeMessage {
241241
/// Parse a mime message.
242242
///
243-
/// If `partial` is set, it contains the full message size in bytes and an optional error text
244-
/// for the partially downloaded message, and `body` contains the HEADER only.
243+
/// If `partial` is set, it contains the full message size in bytes.
245244
pub(crate) async fn from_bytes(
246245
context: &Context,
247246
body: &[u8],
248-
partial: Option<(u32, Option<String>)>,
247+
partial: Option<u32>,
249248
) -> Result<Self> {
250249
let mail = mailparse::parse_mail(body)?;
251250

@@ -619,9 +618,9 @@ impl MimeMessage {
619618
};
620619

621620
match partial {
622-
Some((org_bytes, err)) => {
621+
Some(org_bytes) => {
623622
parser
624-
.create_stub_from_partial_download(context, org_bytes, err)
623+
.create_stub_from_partial_download(context, org_bytes)
625624
.await?;
626625
}
627626
None => match mail {

src/receive_imf.rs

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -186,14 +186,7 @@ pub(crate) async fn receive_imf_from_inbox(
186186
seen: bool,
187187
is_partial_download: Option<u32>,
188188
) -> Result<Option<ReceivedMsg>> {
189-
receive_imf_inner(
190-
context,
191-
rfc724_mid,
192-
imf_raw,
193-
seen,
194-
is_partial_download.map(|msg_size| (msg_size, None)),
195-
)
196-
.await
189+
receive_imf_inner(context, rfc724_mid, imf_raw, seen, is_partial_download).await
197190
}
198191

199192
/// Inserts a tombstone into `msgs` table
@@ -490,14 +483,13 @@ async fn get_to_and_past_contact_ids(
490483
/// If the message is so wrong that we didn't even create a database entry,
491484
/// returns `Ok(None)`.
492485
///
493-
/// If `partial` is set, it contains the full message size in bytes and an optional error text for
494-
/// the partially downloaded message.
486+
/// If `is_partial_download` is set, it contains the full message size in bytes.
495487
pub(crate) async fn receive_imf_inner(
496488
context: &Context,
497489
rfc724_mid: &str,
498490
imf_raw: &[u8],
499491
seen: bool,
500-
partial: Option<(u32, Option<String>)>,
492+
is_partial_download: Option<u32>,
501493
) -> Result<Option<ReceivedMsg>> {
502494
if std::env::var(crate::DCC_MIME_DEBUG).is_ok() {
503495
info!(
@@ -506,16 +498,16 @@ pub(crate) async fn receive_imf_inner(
506498
String::from_utf8_lossy(imf_raw),
507499
);
508500
}
509-
if partial.is_none() {
501+
if is_partial_download.is_none() {
510502
ensure!(
511503
!context
512504
.get_config_bool(Config::FailOnReceivingFullMsg)
513505
.await?
514506
);
515507
}
516508

517-
let is_partial_download = partial.as_ref().map(|(msg_size, _err)| *msg_size);
518-
let mut mime_parser = match MimeMessage::from_bytes(context, imf_raw, partial).await {
509+
let mut mime_parser = match MimeMessage::from_bytes(context, imf_raw, is_partial_download).await
510+
{
519511
Err(err) => {
520512
warn!(context, "receive_imf: can't parse MIME: {err:#}.");
521513
if rfc724_mid.starts_with(GENERATED_PREFIX) {

0 commit comments

Comments
 (0)