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

@@ -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

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 => {
+        Status::RetryNow | Status::RetryLater => Err(format_err!("Retry")),
-            context
-                .sql
-                .execute(
-                    "UPDATE smtp SET retries=retries+1 WHERE id=?",
-                    paramsv![rowid],
-                )
-                .await
-                .context("failed to update retries count")?;
-            Err(format_err!("Retry"))
-        }
     }
 }