SMTP error refactoring

src/context.rs

@@ -19,7 +19,7 @@ use crate::login_param::LoginParam;
 use crate::lot::Lot;
 use crate::message::{self, Message, MsgId};
 use crate::param::Params;
-use crate::smtp::*;
+use crate::smtp::Smtp;
 use crate::sql::Sql;
 
 /// Callback function type for [Context]

src/job.rs

@@ -20,7 +20,6 @@ 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
@@ -185,18 +184,18 @@ 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(SmtpError::SendError(err)) => {
+                        Err(crate::smtp::send::Error::SendError(err)) => {
                             // Remote error, retry later.
                             smtp.disconnect();
                             info!(context, "SMTP failed to send: {}", err);
                             self.try_again_later(TryAgain::AtOnce, Some(err.to_string()));
                         }
-                        Err(SmtpError::EnvelopeError(err)) => {
+                        Err(crate::smtp::send::Error::EnvelopeError(err)) => {
                             // Local error, job is invalid, do not retry.
                             smtp.disconnect();
                             warn!(context, "SMTP job is invalid: {}", err);
                         }
-                        Err(SmtpError::NoTransport) => {
+                        Err(crate::smtp::send::Error::NoTransport) => {
                             // Should never happen.
                             // It does not even make sense to disconnect here.
                             error!(context, "SMTP job failed because SMTP has no transport");

src/smtp/mod.rs

@@ -1,3 +1,7 @@
+//! # SMTP transport module
+
+pub mod send;
+
 use lettre::smtp::client::net::*;
 use lettre::*;
 
@@ -5,11 +9,30 @@ use failure::Fail;
 
 use crate::constants::*;
 use crate::context::Context;
-use crate::error::Error;
 use crate::events::Event;
 use crate::login_param::{dc_build_tls_config, LoginParam};
 use crate::oauth2::*;
 
+#[derive(Debug, Fail)]
+pub enum Error {
+    #[fail(display = "Bad parameters")]
+    BadParameters,
+    #[fail(display = "Invalid login address {}: {}", address, error)]
+    InvalidLoginAddress {
+        address: String,
+        #[cause]
+        error: lettre::error::Error,
+    },
+    #[fail(display = "SMTP failed to connect: {:?}", _0)]
+    ConnectionFailure(#[cause] lettre::smtp::error::Error),
+    #[fail(display = "SMTP: failed to setup connection {:?}", _0)]
+    ConnectionSetupFailure(#[cause] lettre::smtp::error::Error),
+    #[fail(display = "SMTP: oauth2 error {:?}", _0)]
+    Oauth2Error { address: String },
+}
+
+pub type Result<T> = std::result::Result<T, Error>;
+
 #[derive(DebugStub)]
 pub struct Smtp {
     #[debug_stub(some = "SmtpTransport")]
@@ -19,16 +42,6 @@ pub struct Smtp {
     from: Option<EmailAddress>,
 }
 
-#[derive(Debug, Fail)]
-pub enum SmtpError {
-    #[fail(display = "Envelope error: {}", _0)]
-    EnvelopeError(#[cause] lettre::error::Error),
-    #[fail(display = "Send error: {}", _0)]
-    SendError(#[cause] lettre::smtp::error::Error),
-    #[fail(display = "SMTP has no transport")]
-    NoTransport,
-}
-
 impl Smtp {
     /// Create a new Smtp instances.
     pub fn new() -> Self {
@@ -56,7 +69,7 @@ impl Smtp {
     }
 
     /// Connect using the provided login params
-    pub fn connect(&mut self, context: &Context, lp: &LoginParam) -> Result<(), Error> {
+    pub fn connect(&mut self, context: &Context, lp: &LoginParam) -> Result<()> {
         if self.is_connected() {
             warn!(context, "SMTP already connected.");
             return Ok(());
@@ -64,13 +77,16 @@ impl Smtp {
 
         if lp.send_server.is_empty() || lp.send_port == 0 {
             context.call_cb(Event::ErrorNetwork("SMTP bad parameters.".into()));
-            bail!("SMTP Bad parameters");
+            return Err(Error::BadParameters);
         }
 
         self.from = match EmailAddress::new(lp.addr.clone()) {
             Ok(addr) => Some(addr),
             Err(err) => {
-                bail!("invalid login address {}: {}", lp.addr, err);
+                return Err(Error::InvalidLoginAddress {
+                    address: lp.addr.clone(),
+                    error: err,
+                })
             }
         };
 
@@ -85,11 +101,11 @@ impl Smtp {
             let addr = &lp.addr;
             let send_pw = &lp.send_pw;
             let access_token = dc_get_oauth2_access_token(context, addr, send_pw, false);
-            ensure!(
+            if access_token.is_none() {
-                access_token.is_some(),
+                return Err(Error::Oauth2Error {
-                "could not get oaut2_access token addr={}",
+                    address: addr.to_string(),
-                addr
+                });
-            );
+            }
             let user = &lp.send_user;
             (
                 lettre::smtp::authentication::Credentials::new(
@@ -137,57 +153,12 @@ impl Smtp {
                         )));
                         return Ok(());
                     }
-                    Err(err) => {
+                    Err(err) => return Err(Error::ConnectionFailure(err)),
-                        bail!("SMTP: failed to connect {:?}", err);
-                    }
                 }
             }
             Err(err) => {
-                bail!("SMTP: failed to setup connection {:?}", err);
+                return Err(Error::ConnectionSetupFailure(err));
             }
         }
     }
-
-    /// SMTP-Send a prepared mail to recipients.
-    /// on successful send out Ok() is returned.
-    pub fn send(
-        &mut self,
-        context: &Context,
-        recipients: Vec<EmailAddress>,
-        message: Vec<u8>,
-        job_id: u32,
-    ) -> Result<(), SmtpError> {
-        let message_len = message.len();
-
-        let recipients_display = recipients
-            .iter()
-            .map(|x| format!("{}", x))
-            .collect::<Vec<String>>()
-            .join(",");
-
-        if let Some(ref mut transport) = self.transport {
-            let envelope =
-                Envelope::new(self.from.clone(), recipients).map_err(SmtpError::EnvelopeError)?;
-            let mail = SendableEmail::new(
-                envelope,
-                format!("{}", job_id), // only used for internal logging
-                message,
-            );
-
-            transport.send(mail).map_err(SmtpError::SendError)?;
-
-            context.call_cb(Event::SmtpMessageSent(format!(
-                "Message len={} was smtp-sent to {}",
-                message_len, recipients_display
-            )));
-            self.transport_connected = true;
-            Ok(())
-        } else {
-            warn!(
-                context,
-                "uh? SMTP has no transport, failed to send to {}", recipients_display
-            );
-            return Err(SmtpError::NoTransport);
-        }
-    }
 }

src/smtp/send.rs

@@ -0,0 +1,64 @@
+//! # SMTP message sending
+
+use super::Smtp;
+use lettre::*;
+
+use crate::context::Context;
+use crate::events::Event;
+
+pub type Result<T> = std::result::Result<T, Error>;
+
+#[derive(Debug, Fail)]
+pub enum Error {
+    #[fail(display = "Envelope error: {}", _0)]
+    EnvelopeError(#[cause] lettre::error::Error),
+    #[fail(display = "Send error: {}", _0)]
+    SendError(#[cause] lettre::smtp::error::Error),
+    #[fail(display = "SMTP has no transport")]
+    NoTransport,
+}
+
+impl Smtp {
+    /// Send a prepared mail to recipients.
+    /// On successful send out Ok() is returned.
+    pub fn send(
+        &mut self,
+        context: &Context,
+        recipients: Vec<EmailAddress>,
+        message: Vec<u8>,
+        job_id: u32,
+    ) -> Result<()> {
+        let message_len = message.len();
+
+        let recipients_display = recipients
+            .iter()
+            .map(|x| format!("{}", x))
+            .collect::<Vec<String>>()
+            .join(",");
+
+        if let Some(ref mut transport) = self.transport {
+            let envelope =
+                Envelope::new(self.from.clone(), recipients).map_err(Error::EnvelopeError)?;
+            let mail = SendableEmail::new(
+                envelope,
+                format!("{}", job_id), // only used for internal logging
+                message,
+            );
+
+            transport.send(mail).map_err(Error::SendError)?;
+
+            context.call_cb(Event::SmtpMessageSent(format!(
+                "Message len={} was smtp-sent to {}",
+                message_len, recipients_display
+            )));
+            self.transport_connected = true;
+            Ok(())
+        } else {
+            warn!(
+                context,
+                "uh? SMTP has no transport, failed to send to {}", recipients_display
+            );
+            return Err(Error::NoTransport);
+        }
+    }
+}