Repository: lettre/lettre (language: rust)

Issue:
Trait bounds on Transport::Result
**Is your feature request related to a problem? Please describe.**
It is not uncommon to want some email delivery system that is generic over the transport used, so that you can switch between development and production just by switching the type of `Transport` than you insert into a function.

Ideally, you could just make all your functions generic over `Transport` but you run into a problem there: the `Transport::Result` type is completely unconstrained, so a function that is merely generic over `Transport` cannot tell whether sending an email was actually successful.

**Describe the solution you'd like**
The type of `Transport::Result` could be constrained to some trait `TransportResult` which would contain some methods like `is_err` etcetera to determine whether sending the result was successful and maybe implement some quality of life traits like `Debug`.

**Describe alternatives you've considered**
Since the direct problem here is that the `send` function returns a `Result` but doesn't specify this, it is also possible to instead split the associated in two and return a `Result<Transport::Success, Transport::Failure>` from `send`. This is more restrictive in the types that can be returned, so I think the trait bound is the better option.


Discussion / hints:

Fix patch:
diff --git a/src/lib.rs b/src/lib.rs
index e77a6c972..e193e2b42 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -45,6 +45,8 @@ pub use crate::transport::smtp::{SmtpTransport, Tls};
 pub use crate::transport::stub::StubTransport;
 #[cfg(feature = "builder")]
 use std::convert::TryFrom;
+use std::error::Error as StdError;
+use std::fmt;
 
 /// Simple email envelope representation
 ///
@@ -117,17 +119,18 @@ impl TryFrom<&Headers> for Envelope {
 }
 
 /// Transport method for emails
-pub trait Transport<'a> {
-    /// Result type for the transport
-    type Result;
+pub trait Transport {
+    /// Result types for the transport
+    type Ok: fmt::Debug;
+    type Error: StdError;
 
     /// Sends the email
-    fn send(&self, message: &Message) -> Self::Result {
+    fn send(&self, message: &Message) -> Result<Self::Ok, Self::Error> {
         let raw = message.formatted();
         self.send_raw(message.envelope(), &raw)
     }
 
-    fn send_raw(&self, envelope: &Envelope, email: &[u8]) -> Self::Result;
+    fn send_raw(&self, envelope: &Envelope, email: &[u8]) -> Result<Self::Ok, Self::Error>;
 }
 
 #[cfg(test)]
diff --git a/src/transport/file/error.rs b/src/transport/file/error.rs
index 5fbbd0468..572edbb19 100644
--- a/src/transport/file/error.rs
+++ b/src/transport/file/error.rs
@@ -55,8 +55,3 @@ impl From<&'static str> for Error {
         Error::Client(string)
     }
 }
-
-type Id = String;
-
-/// SMTP result type
-pub type FileResult = Result<Id, Error>;
diff --git a/src/transport/file/mod.rs b/src/transport/file/mod.rs
index f7f2d3297..c3d6fd778 100644
--- a/src/transport/file/mod.rs
+++ b/src/transport/file/mod.rs
@@ -35,7 +35,7 @@
 //! TODO
 //! ```
 
-use crate::{transport::file::error::FileResult, Envelope, Transport};
+use crate::{transport::file::error::Error, Envelope, Transport};
 use std::{
     fs::File,
     io::prelude::*,
@@ -46,6 +46,8 @@ use uuid::Uuid;
 
 pub mod error;
 
+type Id = String;
+
 /// Writes the content and the envelope information to a file
 #[derive(Debug)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
@@ -70,10 +72,11 @@ struct SerializableEmail<'a> {
     message: Option<&'a str>,
 }
 
-impl<'a> Transport<'a> for FileTransport {
-    type Result = FileResult;
+impl Transport for FileTransport {
+    type Ok = Id;
+    type Error = Error;
 
-    fn send_raw(&self, envelope: &Envelope, email: &[u8]) -> Self::Result {
+    fn send_raw(&self, envelope: &Envelope, email: &[u8]) -> Result<Self::Ok, Self::Error> {
         let email_id = Uuid::new_v4();
 
         let mut file = self.path.clone();
diff --git a/src/transport/sendmail/error.rs b/src/transport/sendmail/error.rs
index 975f72616..1c0995794 100644
--- a/src/transport/sendmail/error.rs
+++ b/src/transport/sendmail/error.rs
@@ -50,6 +50,3 @@ impl From<FromUtf8Error> for Error {
         Utf8Parsing(err)
     }
 }
-
-/// sendmail result type
-pub type SendmailResult = Result<(), Error>;
diff --git a/src/transport/sendmail/mod.rs b/src/transport/sendmail/mod.rs
index 909c39831..925f62bbf 100644
--- a/src/transport/sendmail/mod.rs
+++ b/src/transport/sendmail/mod.rs
@@ -23,7 +23,7 @@
 //! # }
 //! ```
 
-use crate::{transport::sendmail::error::SendmailResult, Envelope, Transport};
+use crate::{transport::sendmail::error::Error, Envelope, Transport};
 use std::{
     convert::AsRef,
     io::prelude::*,
@@ -55,10 +55,11 @@ impl SendmailTransport {
     }
 }
 
-impl<'a> Transport<'a> for SendmailTransport {
-    type Result = SendmailResult;
+impl Transport for SendmailTransport {
+    type Ok = ();
+    type Error = Error;
 
-    fn send_raw(&self, envelope: &Envelope, email: &[u8]) -> Self::Result {
+    fn send_raw(&self, envelope: &Envelope, email: &[u8]) -> Result<Self::Ok, Self::Error> {
         // Spawn the sendmail command
         let mut process = Command::new(&self.command)
             .arg("-i")
diff --git a/src/transport/smtp/client/mod.rs b/src/transport/smtp/client/mod.rs
index b2eb1f2c0..c8fe50a0c 100644
--- a/src/transport/smtp/client/mod.rs
+++ b/src/transport/smtp/client/mod.rs
@@ -5,7 +5,7 @@ use crate::{
         authentication::{Credentials, Mechanism},
         client::net::{NetworkStream, TlsParameters},
         commands::*,
-        error::{Error, SmtpResult},
+        error::Error,
         extension::{ClientId, Extension, MailBodyParameter, MailParameter, ServerInfo},
         response::Response,
     },
@@ -140,7 +140,7 @@ impl SmtpConnection {
         Ok(conn)
     }
 
-    pub fn send(&mut self, envelope: &Envelope, email: &[u8]) -> SmtpResult {
+    pub fn send(&mut self, envelope: &Envelope, email: &[u8]) -> Result<Response, Error> {
         // Mail
         let mut mail_options = vec![];
 
@@ -210,7 +210,7 @@ impl SmtpConnection {
         Ok(())
     }
 
-    pub fn quit(&mut self) -> SmtpResult {
+    pub fn quit(&mut self) -> Result<Response, Error> {
         Ok(try_smtp!(self.command(Quit), self))
     }
 
@@ -244,7 +244,11 @@ impl SmtpConnection {
     }
 
     /// Sends an AUTH command with the given mechanism, and handles challenge if needed
-    pub fn auth(&mut self, mechanisms: &[Mechanism], credentials: &Credentials) -> SmtpResult {
+    pub fn auth(
+        &mut self,
+        mechanisms: &[Mechanism],
+        credentials: &Credentials,
+    ) -> Result<Response, Error> {
         let mechanism = match self.server_info.get_auth_mechanism(mechanisms) {
             Some(m) => m,
             None => {
@@ -278,7 +282,7 @@ impl SmtpConnection {
     }
 
     /// Sends the message content
-    pub fn message(&mut self, message: &[u8]) -> SmtpResult {
+    pub fn message(&mut self, message: &[u8]) -> Result<Response, Error> {
         let mut out_buf: Vec<u8> = vec![];
         let mut codec = ClientCodec::new();
         codec.encode(message, &mut out_buf)?;
@@ -288,7 +292,7 @@ impl SmtpConnection {
     }
 
     /// Sends an SMTP command
-    pub fn command<C: Display>(&mut self, command: C) -> SmtpResult {
+    pub fn command<C: Display>(&mut self, command: C) -> Result<Response, Error> {
         self.write(command.to_string().as_bytes())?;
         self.read_response()
     }
@@ -307,7 +311,7 @@ impl SmtpConnection {
     }
 
     /// Gets the SMTP response
-    pub fn read_response(&mut self) -> SmtpResult {
+    pub fn read_response(&mut self) -> Result<Response, Error> {
         let mut raw_response = String::new();
         let mut response = raw_response.parse::<Response>();
 
diff --git a/src/transport/smtp/error.rs b/src/transport/smtp/error.rs
index 0652f11e4..edb311c2a 100644
--- a/src/transport/smtp/error.rs
+++ b/src/transport/smtp/error.rs
@@ -152,6 +152,3 @@ impl From<&'static str> for Error {
         Client(string)
     }
 }
-
-/// SMTP result type
-pub type SmtpResult = Result<Response, Error>;
diff --git a/src/transport/smtp/mod.rs b/src/transport/smtp/mod.rs
index 2afa932f3..a958a4dd6 100644
--- a/src/transport/smtp/mod.rs
+++ b/src/transport/smtp/mod.rs
@@ -182,8 +182,9 @@ use crate::{
     transport::smtp::{
         authentication::{Credentials, Mechanism, DEFAULT_MECHANISMS},
         client::SmtpConnection,
-        error::{Error, SmtpResult},
+        error::Error,
         extension::ClientId,
+        response::Response,
     },
     Envelope, Transport,
 };
@@ -419,11 +420,12 @@ impl SmtpTransport {
     }
 }
 
-impl<'a> Transport<'a> for SmtpTransport {
-    type Result = SmtpResult;
+impl Transport for SmtpTransport {
+    type Ok = Response;
+    type Error = Error;
 
     /// Sends an email
-    fn send_raw(&self, envelope: &Envelope, email: &[u8]) -> Self::Result {
+    fn send_raw(&self, envelope: &Envelope, email: &[u8]) -> Result<Self::Ok, Self::Error> {
         #[cfg(feature = "r2d2")]
         let mut conn: Box<dyn DerefMut<Target = SmtpConnection>> = match self.pool {
             Some(ref p) => Box::new(p.get()?),
diff --git a/src/transport/stub/mod.rs b/src/transport/stub/mod.rs
index 50705cb22..0fbd7e8e2 100644
--- a/src/transport/stub/mod.rs
+++ b/src/transport/stub/mod.rs
@@ -17,38 +17,60 @@
 //!     .body("Be happy!")
 //!     .unwrap();
 //!
-//! let mut sender = StubTransport::new_positive();
+//! let mut sender = StubTransport::new_ok();
 //! let result = sender.send(&email);
 //! assert!(result.is_ok());
 //! ```
 
 use crate::{Envelope, Transport};
+use std::error::Error as StdError;
+use std::fmt;
+
+#[derive(Debug, Copy, Clone)]
+pub struct Error;
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "stub error")
+    }
+}
+
+impl StdError for Error {
+    fn source(&self) -> Option<&(dyn StdError + 'static)> {
+        None
+    }
+}
 
 /// This transport logs the message envelope and returns the given response
 #[derive(Debug, Clone, Copy)]
 pub struct StubTransport {
-    response: StubResult,
+    response: Result<(), Error>,
 }
 
 impl StubTransport {
-    /// Creates a new transport that always returns the given response
-    pub fn new(response: StubResult) -> StubTransport {
+    /// Creates aResult new transport that always returns the given response
+    pub fn new(response: Result<(), Error>) -> StubTransport {
         StubTransport { response }
     }
 
     /// Creates a new transport that always returns a success response
-    pub fn new_positive() -> StubTransport {
+    pub fn new_ok() -> StubTransport {
         StubTransport { response: Ok(()) }
     }
-}
 
-/// SMTP result type
-pub type StubResult = Result<(), ()>;
+    /// Creates a new transport that always returns an error
+    pub fn new_error() -> StubTransport {
+        StubTransport {
+            response: Err(Error),
+        }
+    }
+}
 
-impl<'a> Transport<'a> for StubTransport {
-    type Result = StubResult;
+impl Transport for StubTransport {
+    type Ok = ();
+    type Error = Error;
 
-    fn send_raw(&self, _envelope: &Envelope, _email: &[u8]) -> Self::Result {
+    fn send_raw(&self, _envelope: &Envelope, _email: &[u8]) -> Result<Self::Ok, Self::Error> {
         self.response
     }
 }


Test patch:
diff --git a/tests/transport_stub.rs b/tests/transport_stub.rs
index 980c421ca..4d573483c 100644
--- a/tests/transport_stub.rs
+++ b/tests/transport_stub.rs
@@ -2,8 +2,8 @@ use lettre::{transport::stub::StubTransport, Message, Transport};
 
 #[test]
 fn stub_transport() {
-    let sender_ok = StubTransport::new_positive();
-    let sender_ko = StubTransport::new(Err(()));
+    let sender_ok = StubTransport::new_ok();
+    let sender_ko = StubTransport::new_error();
     let email = Message::builder()
         .from("NoBody <nobody@domain.tld>".parse().unwrap())
         .reply_to("Yuin <yuin@domain.tld>".parse().unwrap())