fix: Don't try to send more MDNs if there's a tmp SMTP error (#4534)

If there's a temporary SMTP error, pretend there are no more MDNs to send in send_mdn(). It's
unlikely that other MDNs could be sent successfully in case of connectivity problems. This approach
is simpler and perhaps even better than adding a progressive backoff between MDN sending retries --
MDNs just will be resent after a reconnection, which makes some sense.
This commit is contained in:
iequidoo
2023-10-12 19:08:55 -03:00
parent bda6cea0ce
commit a54f3c4b31

View File

@@ -673,12 +673,14 @@ pub(crate) async fn send_smtp_messages(context: &Context, connection: &mut Smtp)
/// On failure returns an error without removing any `smtp_mdns` entries, the caller is responsible /// On failure returns an error without removing any `smtp_mdns` entries, the caller is responsible
/// for removing the corresponding entry to prevent endless loop in case the entry is invalid, e.g. /// for removing the corresponding entry to prevent endless loop in case the entry is invalid, e.g.
/// points to non-existent message or contact. /// points to non-existent message or contact.
///
/// Returns true on success, false on temporary error.
async fn send_mdn_msg_id( async fn send_mdn_msg_id(
context: &Context, context: &Context,
msg_id: MsgId, msg_id: MsgId,
contact_id: ContactId, contact_id: ContactId,
smtp: &mut Smtp, smtp: &mut Smtp,
) -> Result<()> { ) -> Result<bool> {
let contact = Contact::get_by_id(context, contact_id).await?; let contact = Contact::get_by_id(context, contact_id).await?;
if contact.is_blocked() { if contact.is_blocked() {
return Err(format_err!("Contact is blocked")); return Err(format_err!("Contact is blocked"));
@@ -730,14 +732,14 @@ async fn send_mdn_msg_id(
.execute(&q, rusqlite::params_from_iter(additional_msg_ids)) .execute(&q, rusqlite::params_from_iter(additional_msg_ids))
.await?; .await?;
} }
Ok(()) Ok(true)
} }
SendResult::Retry => { SendResult::Retry => {
info!( info!(
context, context,
"Temporary SMTP failure while sending an MDN for {}", msg_id "Temporary SMTP failure while sending an MDN for {}", msg_id
); );
Ok(()) Ok(false)
} }
SendResult::Failure(err) => Err(err), SendResult::Failure(err) => Err(err),
} }
@@ -784,15 +786,16 @@ async fn send_mdn(context: &Context, smtp: &mut Smtp) -> Result<bool> {
.await .await
.context("failed to update MDN retries count")?; .context("failed to update MDN retries count")?;
if let Err(err) = send_mdn_msg_id(context, msg_id, contact_id, smtp).await { let res = send_mdn_msg_id(context, msg_id, contact_id, smtp).await;
if res.is_err() {
// If there is an error, for example there is no message corresponding to the msg_id in the // 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. // database, do not try to send this MDN again.
context context
.sql .sql
.execute("DELETE FROM smtp_mdns WHERE msg_id = ?", (msg_id,)) .execute("DELETE FROM smtp_mdns WHERE msg_id = ?", (msg_id,))
.await?; .await?;
Err(err)
} else {
Ok(true)
} }
// If there's a temporary error, pretend there are no more MDNs to send. It's unlikely that
// other MDNs could be sent successfully in case of connectivity problems.
res
} }