From 242e8e2bb3a651ee3ad0e0f03191e5e4263f3570 Mon Sep 17 00:00:00 2001
From: link2xt <link2xt@testrun.org>
Date: Sat, 12 Feb 2022 16:20:13 +0000
Subject: [PATCH] smtp: remove the message in case of permanent failure

When `smtp_send` returns `Status::Finished`,
the message should be removed from the queue even in case of
failure, such as a permanent error.

In addition to this bugfix, move the retry count increase to
the beginning of `send_msg_to_smtp` to ensure no message is
retried infinitely even in case of similar bugs.
---
 CHANGELOG.md |  1 +
 src/smtp.rs  | 34 ++++++++++++++++++----------------
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index daa41af9a..31519b11b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@
 - treat "NO" IMAP response to MOVE and COPY commands as an error #3058
 - Fix a bug where messages in the Spam folder created contact requests #3015
 - Fix a bug where drafts disappeared after some days #3067
+- do not retry message sending infinitely in case of permanent SMTP failure #3070
 
 
 ## 1.75.0
diff --git a/src/smtp.rs b/src/smtp.rs
index e8f19cc38..a1ad1da57 100644
--- a/src/smtp.rs
+++ b/src/smtp.rs
@@ -343,6 +343,19 @@ pub(crate) async fn send_msg_to_smtp(
         return Err(err);
     }
 
+    // Increse retry count as soon as we have an SMTP connection. This ensures that the message is
+    // eventually removed from the queue by exceeding retry limit even in case of an error that
+    // keeps happening early in the message sending code, e.g. failure to read the message from the
+    // database.
+    context
+        .sql
+        .execute(
+            "UPDATE smtp SET retries=retries+1 WHERE id=?",
+            paramsv![rowid],
+        )
+        .await
+        .context("failed to update retries count")?;
+
     let (body, recipients, msg_id) = context
         .sql
         .query_row(
@@ -421,27 +434,16 @@ pub(crate) async fn send_msg_to_smtp(
     };
     match status {
         Status::Finished(res) => {
+            context
+                .sql
+                .execute("DELETE FROM smtp WHERE id=?", paramsv![rowid])
+                .await?;
             if res.is_ok() {
                 msg_id.set_delivered(context).await?;
-
-                context
-                    .sql
-                    .execute("DELETE FROM smtp WHERE id=?", paramsv![rowid])
-                    .await?;
             }
             res
         }
-        Status::RetryNow | Status::RetryLater => {
-            context
-                .sql
-                .execute(
-                    "UPDATE smtp SET retries=retries+1 WHERE id=?",
-                    paramsv![rowid],
-                )
-                .await
-                .context("failed to update retries count")?;
-            Err(format_err!("Retry"))
-        }
+        Status::RetryNow | Status::RetryLater => Err(format_err!("Retry")),
     }
 }