Add Tell specific errors and improve error logging for commands

3 files changed, 110 insertions, 78 deletions

diff --git a/src/lib.rs b/src/lib.rs
index a740ec7..ebadb86 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -228,7 +228,7 @@ impl ThreadedPlugins {
             // Send the message to the plugin if the plugin needs it
             match plugin.execute(client, &message) {
                 ExecutionStatus::Done => (),
-                ExecutionStatus::Err(e) => error!("Error in {} - {}", name, e),
+                ExecutionStatus::Err(e) => log_error(e),
                 ExecutionStatus::RequiresThread => {
                     debug!(
                         "Spawning thread to execute {} with {}",
diff --git a/src/plugins/tell/database.rs b/src/plugins/tell/database.rs
index 277847e..40ec6fc 100644
--- a/src/plugins/tell/database.rs
+++ b/src/plugins/tell/database.rs
@@ -16,10 +16,10 @@ use r2d2_diesel::ConnectionManager;
 
 use chrono::NaiveDateTime;
 
-pub enum DbResponse {
-    Success,
-    Failed(&'static str),
-}
+#[cfg(feature = "mysql")]
+use failure::ResultExt;
+
+use super::error::*;
 
 #[cfg_attr(feature = "mysql", derive(Queryable))]
 #[derive(PartialEq, Clone, Debug)]
@@ -41,14 +41,14 @@ pub struct NewTellMessage<'a> {
 }
 
 pub trait Database: Send {
-    fn insert_tell(&mut self, tell: &NewTellMessage) -> DbResponse;
-    fn get_tells(&self, receiver: &str) -> Option<Vec<TellMessage>>;
-    fn delete_tells(&mut self, receiver: &str) -> DbResponse;
+    fn insert_tell(&mut self, tell: &NewTellMessage) -> Result<(), TellError>;
+    fn get_tells(&self, receiver: &str) -> Result<Vec<TellMessage>, TellError>;
+    fn delete_tells(&mut self, receiver: &str) -> Result<(), TellError>;
 }
 
 // HashMap
 impl Database for HashMap<String, Vec<TellMessage>> {
-    fn insert_tell(&mut self, tell: &NewTellMessage) -> DbResponse {
+    fn insert_tell(&mut self, tell: &NewTellMessage) -> Result<(), TellError> {
         let tell = TellMessage {
             id: 0,
             sender: tell.sender.to_string(),
@@ -62,17 +62,17 @@ impl Database for HashMap<String, Vec<TellMessage>> {
             .or_insert_with(|| Vec::with_capacity(3));
         (*tell_messages).push(tell);
 
-        DbResponse::Success
+        Ok(())
     }
 
-    fn get_tells(&self, receiver: &str) -> Option<Vec<TellMessage>> {
-        self.get(receiver).cloned()
+    fn get_tells(&self, receiver: &str) -> Result<Vec<TellMessage>, TellError> {
+        Ok(self.get(receiver).cloned().ok_or(ErrorKind::NotFound)?)
     }
 
-    fn delete_tells(&mut self, receiver: &str) -> DbResponse {
+    fn delete_tells(&mut self, receiver: &str) -> Result<(), TellError> {
         match self.remove(receiver) {
-            Some(_) => DbResponse::Success,
-            None => DbResponse::Failed("Tells not found"),
+            Some(_) => Ok(()),
+            None => Err(ErrorKind::NotFound)?,
         }
     }
 }
@@ -97,47 +97,37 @@ use self::schema::tells;
 
 #[cfg(feature = "mysql")]
 impl Database for Arc<Pool<ConnectionManager<MysqlConnection>>> {
-    fn insert_tell(&mut self, tell: &NewTellMessage) -> DbResponse {
+    fn insert_tell(&mut self, tell: &NewTellMessage) -> Result<(), TellError> {
         use diesel;
 
         let conn = &*self.get().expect("Failed to get connection");
-        match diesel::insert_into(tells::table).values(tell).execute(conn) {
-            Ok(_) => DbResponse::Success,
-            Err(e) => {
-                error!("DB failed to insert tell: {}", e);
-                DbResponse::Failed("Failed to save Tell")
-            }
-        }
+        diesel::insert_into(tells::table)
+            .values(tell)
+            .execute(conn)
+            .context(ErrorKind::MysqlError)?;
+
+        Ok(())
     }
 
-    fn get_tells(&self, receiver: &str) -> Option<Vec<TellMessage>> {
+    fn get_tells(&self, receiver: &str) -> Result<Vec<TellMessage>, TellError> {
         use self::tells::columns;
 
-        let conn = &*self.get().expect("Failed to get connection");
-        match tells::table
+        let conn = &*self.get().context(ErrorKind::NoConnection)?;
+        Ok(tells::table
             .filter(columns::receiver.eq(receiver))
             .order(columns::time.asc())
             .load::<TellMessage>(conn)
-        {
-            Ok(f) => Some(f),
-            Err(e) => {
-                error!("DB failed to get tells: {}", e);
-                None
-            }
-        }
+            .context(ErrorKind::MysqlError)?)
     }
 
-    fn delete_tells(&mut self, receiver: &str) -> DbResponse {
+    fn delete_tells(&mut self, receiver: &str) -> Result<(), TellError> {
         use diesel;
         use self::tells::columns;
 
-        let conn = &*self.get().expect("Failed to get connection");
-        match diesel::delete(tells::table.filter(columns::receiver.eq(receiver))).execute(conn) {
-            Ok(_) => DbResponse::Success,
-            Err(e) => {
-                error!("DB failed to delete tells: {}", e);
-                DbResponse::Failed("Failed to delete tells")
-            }
-        }
+        let conn = &*self.get().context(ErrorKind::NoConnection)?;
+        diesel::delete(tells::table.filter(columns::receiver.eq(receiver)))
+            .execute(conn)
+            .context(ErrorKind::MysqlError)?;
+        Ok(())
     }
 }
diff --git a/src/plugins/tell/mod.rs b/src/plugins/tell/mod.rs
index f781ed8..ccca300 100644
--- a/src/plugins/tell/mod.rs
+++ b/src/plugins/tell/mod.rs
@@ -9,13 +9,14 @@ use humantime::format_duration;
 
 use plugin::*;
 
-use error::FrippyError;
-use error::ErrorKind as FrippyErrorKind;
 use failure::Fail;
 use failure::ResultExt;
+use error::ErrorKind as FrippyErrorKind;
+use error::FrippyError;
+use self::error::*;
 
 pub mod database;
-use self::database::{Database, DbResponse};
+use self::database::Database;
 
 macro_rules! try_lock {
     ( $m:expr ) => {
@@ -38,16 +39,20 @@ impl<T: Database> Tell<T> {
         }
     }
 
-    fn tell_command(&self, client: &IrcClient, command: PluginCommand) -> Result<&str, String> {
+    fn tell_command(
+        &self,
+        client: &IrcClient,
+        command: PluginCommand,
+    ) -> Result<String, TellError> {
         if command.tokens.len() < 2 {
-            return Err(self.invalid_command(client));
+            return Ok(self.invalid_command(client));
         }
 
         let receiver = &command.tokens[0];
         let sender = command.source;
 
         if receiver.eq_ignore_ascii_case(&sender) {
-            return Err(String::from("That's your name!"));
+            return Ok(String::from("That's your name!"));
         }
 
         if let Some(channels) = client.list_channels() {
@@ -57,7 +62,7 @@ impl<T: Database> Tell<T> {
                         .iter()
                         .any(|u| u.get_nickname().eq_ignore_ascii_case(&receiver))
                     {
-                        return Err(format!("{} is currently online.", receiver));
+                        return Ok(format!("{} is currently online.", receiver));
                     }
                 }
             }
@@ -72,36 +77,49 @@ impl<T: Database> Tell<T> {
             message: &message,
         };
 
-        match try_lock!(self.tells).insert_tell(&tell) {
-            DbResponse::Success => Ok("Got it!"),
-            DbResponse::Failed(e) => Err(e.to_string()),
-        }
+        try_lock!(self.tells).insert_tell(&tell)?;
+
+        Ok(String::from("Got it!"))
     }
 
     fn send_tells(&self, client: &IrcClient, receiver: &str) -> ExecutionStatus {
         let mut tells = try_lock!(self.tells);
-        if let Some(tell_messages) = tells.get_tells(&receiver.to_lowercase()) {
-            for tell in tell_messages {
-                let now = Duration::new(time::now().to_timespec().sec as u64, 0);
-                let dur = now - Duration::new(tell.time.timestamp() as u64, 0);
-                let human_dur = format_duration(dur);
-
-                if let Err(e) = client.send_notice(
-                    receiver,
-                    &format!(
-                        "Tell from {} {} ago: {}",
-                        tell.sender, human_dur, tell.message
-                    ),
-                ) {
-                    return ExecutionStatus::Err(e.context(FrippyErrorKind::Connection).into());
-                }
-                debug!(
-                    "Sent {:?} from {:?} to {:?}",
-                    tell.message, tell.sender, receiver
-                );
+
+        let tell_messages = match tells.get_tells(&receiver.to_lowercase()) {
+            Ok(t) => t,
+            Err(e) => {
+                // This warning only occurs if frippy is built without a database
+                #[allow(unreachable_patterns)]
+                return match e.kind() {
+                    ErrorKind::NotFound => ExecutionStatus::Done,
+                    _ => ExecutionStatus::Err(e.context(FrippyErrorKind::Tell).into()),
+                };
+            }
+        };
+
+        for tell in tell_messages {
+            let now = Duration::new(time::now().to_timespec().sec as u64, 0);
+            let dur = now - Duration::new(tell.time.timestamp() as u64, 0);
+            let human_dur = format_duration(dur);
+
+            if let Err(e) = client.send_notice(
+                receiver,
+                &format!(
+                    "Tell from {} {} ago: {}",
+                    tell.sender, human_dur, tell.message
+                ),
+            ) {
+                return ExecutionStatus::Err(e.context(FrippyErrorKind::Connection).into());
             }
+            debug!(
+                "Sent {:?} from {:?} to {:?}",
+                tell.message, tell.sender, receiver
+            );
         }
-        tells.delete_tells(&receiver.to_lowercase());
+
+        if let Err(e) = tells.delete_tells(&receiver.to_lowercase()) {
+            return ExecutionStatus::Err(e.context(FrippyErrorKind::Tell).into());
+        };
 
         ExecutionStatus::Done
     }
@@ -126,7 +144,9 @@ impl<T: Database> Tell<T> {
 impl<T: Database> Plugin for Tell<T> {
     fn execute(&self, client: &IrcClient, message: &Message) -> ExecutionStatus {
         match message.command {
-            Command::JOIN(_, _, _) => self.send_tells(client, message.source_nickname().unwrap()),
+            Command::JOIN(_, _, _) => {
+                self.send_tells(client, message.source_nickname().unwrap())
+            }
             _ => ExecutionStatus::Done,
         }
     }
@@ -147,14 +167,16 @@ impl<T: Database> Plugin for Tell<T> {
         Ok(match command.tokens[0].as_ref() {
             "help" => client
                 .send_notice(&command.source, &self.help(client))
-                .context(FrippyErrorKind::Connection),
+                .context(FrippyErrorKind::Connection)
+                .into(),
             _ => match self.tell_command(client, command) {
                 Ok(msg) => client
-                    .send_notice(&sender, msg)
-                    .context(FrippyErrorKind::Connection),
-                Err(msg) => client
                     .send_notice(&sender, &msg)
                     .context(FrippyErrorKind::Connection),
+                Err(e) => client
+                        .send_notice(&sender, &e.to_string())
+                        .context(FrippyErrorKind::Connection)
+                        .into()
             },
         }?)
     }
@@ -170,3 +192,23 @@ impl<T: Database> fmt::Debug for Tell<T> {
         write!(f, "Tell {{ ... }}")
     }
 }
+
+pub mod error {
+    #[derive(Copy, Clone, Eq, PartialEq, Debug, Fail, Error)]
+    #[error = "TellError"]
+    pub enum ErrorKind {
+        /// Not found command error
+        #[fail(display = "Tell was not found")]
+        NotFound,
+
+        /// MySQL error
+        #[cfg(feature = "mysql")]
+        #[fail(display = "Failed to execute MySQL Query")]
+        MysqlError,
+
+        /// No connection error
+        #[cfg(feature = "mysql")]
+        #[fail(display = "No connection to the database")]
+        NoConnection,
+    }
+}