From 14287b12aec83a03446af454b31d6be0b85d5b09 Mon Sep 17 00:00:00 2001
From: Alexander Krotov <ilabdsf@gmail.com>
Date: Fri, 29 Nov 2019 16:08:37 +0100
Subject: [PATCH] Resultify Smtp::send

---
 src/job.rs  | 16 ++++++++++++++--
 src/smtp.rs | 31 ++++++++++++++++++++-----------
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/src/job.rs b/src/job.rs
index 260a5a6da..7655da793 100644
--- a/src/job.rs
+++ b/src/job.rs
@@ -20,6 +20,7 @@ use crate::message::MsgId;
 use crate::message::{self, Message, MessageState};
 use crate::mimefactory::{vec_contains_lowercase, Loaded, MimeFactory};
 use crate::param::*;
+use crate::smtp::SmtpError;
 use crate::sql;
 
 // results in ~3 weeks for the last backoff timespan
@@ -184,11 +185,22 @@ impl Job {
                     // otherwise might send it twice.
                     let mut smtp = context.smtp.lock().unwrap();
                     match smtp.send(context, recipients_list, body, self.job_id) {
-                        Err(err) => {
+                        Err(SmtpError::SendError(err)) => {
+                            // Remote error, retry later.
                             smtp.disconnect();
-                            warn!(context, "smtp failed: {}", err);
+                            info!(context, "SMTP failed to send: {}", err);
                             self.try_again_later(TryAgain::AtOnce, Some(err.to_string()));
                         }
+                        Err(SmtpError::EnvelopeError(err)) => {
+                            // Local error, job is invalid, do not retry.
+                            smtp.disconnect();
+                            warn!(context, "SMTP job is invalid: {}", err);
+                        }
+                        Err(SmtpError::NoTransport) => {
+                            // Should never happen.
+                            // It does not even make sense to disconnect here.
+                            error!(context, "SMTP job failed because SMTP has no transport");
+                        }
                         Ok(()) => {
                             // smtp success, update db ASAP, then delete smtp file
                             if 0 != self.foreign_id {
diff --git a/src/smtp.rs b/src/smtp.rs
index e8d0fe435..429b04212 100644
--- a/src/smtp.rs
+++ b/src/smtp.rs
@@ -1,6 +1,8 @@
 use lettre::smtp::client::net::*;
 use lettre::*;
 
+use failure::Fail;
+
 use crate::constants::*;
 use crate::context::Context;
 use crate::error::Error;
@@ -17,6 +19,16 @@ pub struct Smtp {
     from: Option<EmailAddress>,
 }
 
+#[derive(Debug, Fail)]
+pub enum SmtpError {
+    #[fail(display = "Envelope error: {}", _0)]
+    EnvelopeError(lettre::error::Error),
+    #[fail(display = "Send error: {}", _0)]
+    SendError(lettre::smtp::error::Error),
+    #[fail(display = "SMTP has no transport")]
+    NoTransport,
+}
+
 impl Smtp {
     /// Create a new Smtp instances.
     pub fn new() -> Self {
@@ -144,7 +156,7 @@ impl Smtp {
         recipients: Vec<EmailAddress>,
         message: Vec<u8>,
         job_id: u32,
-    ) -> Result<(), Error> {
+    ) -> Result<(), SmtpError> {
         let message_len = message.len();
 
         let recipients_display = recipients
@@ -155,10 +167,8 @@ impl Smtp {
 
         if let Some(ref mut transport) = self.transport {
             let envelope = match Envelope::new(self.from.clone(), recipients) {
-                Ok(env) => env,
-                Err(err) => {
-                    bail!("{}", err);
-                }
+                Ok(envelope) => envelope,
+                Err(e) => return Err(SmtpError::EnvelopeError(e)),
             };
             let mail = SendableEmail::new(
                 envelope,
@@ -175,15 +185,14 @@ impl Smtp {
                     self.transport_connected = true;
                     Ok(())
                 }
-                Err(err) => {
-                    bail!("SMTP failed len={}: error: {}", message_len, err);
-                }
+                Err(err) => return Err(SmtpError::SendError(err)),
             }
         } else {
-            bail!(
-                "uh? SMTP has no transport,  failed to send to {:?}",
-                recipients_display
+            warn!(
+                context,
+                "uh? SMTP has no transport, failed to send to {}", recipients_display
             );
+            return Err(SmtpError::NoTransport);
         }
     }
 }