Use error mechanisms for _all_ exceptional cases

rust-lang#17 (comment)

src/api.rs

@@ -69,8 +69,8 @@ pub fn is_wg_and_teams(args: &Args) -> Result<bool, Error> {
 ///
 /// A `seconds` value of 0 will disable slowmode
 pub fn slow_mode(args: &Args) -> Result<(), Error> {
-    if !is_mod(&args)? {
-        return Ok(());
+    if !is_mod(args)? {
+        return Err(Error::MissingPermissions);
     }
 
     let mut token = args.body.splitn(2, ' ');

src/commands.rs

@@ -151,9 +151,7 @@ impl Commands {
                 Ok(true) => {
                     if let Err(e) = (command.handler)(&args) {
                         error!("Error when executing command {}: {}", command.name, e);
-                        if let Err(e) =
-                            crate::api::send_reply(&args, &format!("Encountered error ({})", e))
-                        {
+                        if let Err(e) = crate::api::send_reply(&args, &e.to_string()) {
                             error!("{}", e)
                         }
                     }
@@ -164,9 +162,7 @@ impl Commands {
                         "Error when checking command permissions for {}: {}",
                         command.name, e
                     );
-                    if let Err(e) =
-                        crate::api::send_reply(&args, &format!("Encountered error ({})", e))
-                    {
+                    if let Err(e) = crate::api::send_reply(&args, &e.to_string()) {
                         error!("{}", e)
                     }
                 }

src/crates.rs

@@ -24,7 +24,7 @@ struct Crate {
 }
 
 /// Queries the crates.io crates list and yields the first result, if any
-fn get_crate(http: &reqwest::blocking::Client, query: &str) -> Result<Option<Crate>, Error> {
+fn get_crate(http: &reqwest::blocking::Client, query: &str) -> Result<Crate, Error> {
     info!("searching for crate `{}`", query);
 
     let crate_list = http
@@ -34,28 +34,27 @@ fn get_crate(http: &reqwest::blocking::Client, query: &str) -> Result<Option<Cra
         .send()?
         .json::<Crates>()?;
 
-    Ok(crate_list.crates.into_iter().next())
+    crate_list
+        .crates
+        .into_iter()
+        .next()
+        .ok_or(Error::NoCratesFound)
 }
 
 pub fn search(args: &Args) -> Result<(), Error> {
-    if let Some(krate) = get_crate(&args.http, args.body)? {
-        args.msg.channel_id.send_message(&args.cx, |m| {
-            m.embed(|e| {
-                e.title(&krate.name)
-                    .url(format!("https://crates.io/crates/{}", krate.id))
-                    .description(&krate.description)
-                    .field("Version", &krate.version, true)
-                    .field("Downloads", &krate.downloads, true)
-                    .timestamp(krate.updated.as_str())
-            });
-
-            m
-        })?;
-    } else {
-        let message = "No crates found.";
-        api::send_reply(&args, message)?;
-    }
-
+    let krate = get_crate(&args.http, args.body)?;
+    args.msg.channel_id.send_message(&args.cx, |m| {
+        m.embed(|e| {
+            e.title(&krate.name)
+                .url(format!("https://crates.io/crates/{}", krate.id))
+                .description(&krate.description)
+                .field("Version", &krate.version, true)
+                .field("Downloads", &krate.downloads, true)
+                .timestamp(krate.updated.as_str())
+        });
+
+        m
+    })?;
     Ok(())
 }
 
@@ -80,14 +79,12 @@ pub fn doc_search(args: &Args) -> Result<(), Error> {
     // The base docs url, e.g. `https://docs.rs/syn` or `https://doc.rust-lang.org/stable/std/`
     let mut doc_url = if let Some(rustc_crate) = rustc_crate_link(crate_name) {
         rustc_crate.to_string()
-    } else if let Some(krate) = get_crate(&args.http, crate_name)? {
+    } else {
+        let krate = get_crate(&args.http, crate_name)?;
         let crate_name = krate.name;
         krate
             .documentation
             .unwrap_or_else(|| format!("https://docs.rs/{}", crate_name))
-    } else {
-        api::send_reply(&args, "No crates found.")?;
-        return Ok(());
     };
 
     if let Some(item_path) = query_iter.next() {

src/godbolt.rs

@@ -63,12 +63,7 @@ fn compile_rust_source(
 }
 
 pub fn godbolt(args: &crate::Args) -> Result<(), crate::Error> {
-    let code = match crate::extract_code(&args.body) {
-        Some(x) => x,
-        None => return crate::reply_missing_code_block_err(&args),
-    };
-
-    let (lang, text) = match compile_rust_source(args.http, code)? {
+    let (lang, text) = match compile_rust_source(args.http, crate::extract_code(&args.body)?)? {
         Compilation::Success { asm } => ("x86asm", asm),
         Compilation::Error { stderr } => ("rust", stderr),
     };

src/main.rs

@@ -29,7 +29,42 @@ use indexmap::IndexMap;
 use serde::Deserialize;
 use serenity::{model::prelude::*, prelude::*};
 
-pub type Error = Box<dyn std::error::Error + Send + Sync>;
+pub enum Error {
+    MissingCodeblock,
+    NoCratesFound,
+    MissingPermissions,
+    EvalWithFnMain,
+    Unknown(Box<dyn std::error::Error + Send + Sync>),
+}
+
+impl<T> From<T> for Error
+where
+    Box<dyn std::error::Error + Send + Sync>: From<T>,
+{
+    fn from(error: T) -> Self {
+        Self::Unknown(error.into())
+    }
+}
+
+impl std::fmt::Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::MissingCodeblock => write!(
+                f,
+                "Missing code block. Please use the following markdown:
+\\`code here\\`
+or
+\\`\\`\\`rust
+code here
+\\`\\`\\`",
+            ),
+            Self::NoCratesFound => write!(f, "No crates found"),
+            Self::MissingPermissions => write!(f, "You are not allowed to use this command"),
+            Self::EvalWithFnMain => write!(f, "Code passed to ?eval should not contain `fn main`"),
+            Self::Unknown(error) => write!(f, "Unexpected error ({})", error),
+        }
+    }
+}
 
 #[derive(Deserialize)]
 struct Config {
@@ -156,7 +191,7 @@ fn app() -> Result<(), Error> {
 
     // Slow mode.
     // 0 seconds disables slowmode
-    cmds.add_protected("slowmode", api::slow_mode, api::is_mod);
+    cmds.add("slowmode", api::slow_mode);
     cmds.help_protected(
         "slowmode",
         "Set slowmode on a channel",
@@ -165,7 +200,7 @@ fn app() -> Result<(), Error> {
     );
 
     // Kick
-    cmds.add_protected("kick", api::kick, api::is_mod);
+    cmds.add("kick", api::kick);
     cmds.help_protected(
         "kick",
         "Kick a user from the guild",
@@ -174,7 +209,7 @@ fn app() -> Result<(), Error> {
     );
 
     // Ban
-    cmds.add_protected("ban", ban::temp_ban, api::is_mod);
+    cmds.add("ban", ban::temp_ban);
     cmds.help_protected(
         "ban",
         "Temporarily ban a user from the guild",
@@ -183,7 +218,7 @@ fn app() -> Result<(), Error> {
     );
 
     // Post the welcome message to the welcome channel.
-    cmds.add_protected("CoC", welcome::post_message, api::is_mod);
+    cmds.add("CoC", welcome::post_message);
     cmds.help_protected(
         "CoC",
         "Post the code of conduct message to a channel",
@@ -279,35 +314,26 @@ fn reply_potentially_long_text(
 /// assert_eq!(extract_code("```rust\nhello\n```"), Some("hello"));
 /// assert_eq!(extract_code("``` rust\nhello\n```"), Some("rust\nhello"));
 /// ```
-pub fn extract_code(input: &str) -> Option<&str> {
-    let input = input.trim();
-
-    let extracted_code = if input.starts_with("```") && input.ends_with("```") {
-        let code_starting_point = input.find(char::is_whitespace)?; // skip over lang specifier
-        let code_end_point = input.len() - 3;
-
-        // can't fail but you can never be too sure
-        input.get(code_starting_point..code_end_point)?
-    } else if input.starts_with('`') && input.ends_with('`') {
-        // can't fail but you can never be too sure
-        input.get(1..(input.len() - 1))?
-    } else {
-        return None;
-    };
-
-    Some(extracted_code.trim())
-}
-
-pub fn reply_missing_code_block_err(args: &Args) -> Result<(), Error> {
-    let message = "Missing code block. Please use the following markdown:
-\\`code here\\`
-or
-\\`\\`\\`rust
-code here
-\\`\\`\\`";
-
-    api::send_reply(args, message)?;
-    Ok(())
+pub fn extract_code(input: &str) -> Result<&str, Error> {
+    fn inner(input: &str) -> Option<&str> {
+        let input = input.trim();
+
+        let extracted_code = if input.starts_with("```") && input.ends_with("```") {
+            let code_starting_point = input.find(char::is_whitespace)?; // skip over lang specifier
+            let code_end_point = input.len() - 3;
+
+            // can't fail but you can never be too sure
+            input.get(code_starting_point..code_end_point)?
+        } else if input.starts_with('`') && input.ends_with('`') {
+            // can't fail but you can never be too sure
+            input.get(1..(input.len() - 1))?
+        } else {
+            return None;
+        };
+
+        Some(extracted_code.trim())
+    }
+    inner(input).ok_or(Error::MissingCodeblock)
 }
 
 fn main_menu(args: &Args, commands: &IndexMap<&str, (&str, GuardFn)>) -> String {

src/playground.rs

@@ -282,21 +282,14 @@ fn run_code_and_reply(args: &Args, code: &str) -> Result<(), Error> {
 }
 
 pub fn play(args: &Args) -> Result<(), Error> {
-    match crate::extract_code(args.body) {
-        Some(code) => run_code_and_reply(&args, code),
-        None => crate::reply_missing_code_block_err(&args),
-    }
+    run_code_and_reply(&args, crate::extract_code(args.body)?)
 }
 
 pub fn eval(args: &Args) -> Result<(), Error> {
-    let code = match crate::extract_code(args.body) {
-        Some(x) => x,
-        None => return crate::reply_missing_code_block_err(&args),
-    };
+    let code = crate::extract_code(args.body)?;
 
     if code.contains("fn main") {
-        api::send_reply(&args, "code passed to ?eval should not contain `fn main`")?;
-        return Ok(());
+        return Err(Error::EvalWithFnMain);
     }
 
     let mut full_code = String::from("fn main() {\n    println!(\"{:?}\", {\n");
@@ -319,10 +312,7 @@ fn generic_command<'a, R: Serialize + 'a>(
     url: &str,
     request_builder: impl FnOnce(&'a str, &CommandFlags) -> R,
 ) -> Result<(), Error> {
-    let code = match crate::extract_code(args.body) {
-        Some(x) => x,
-        None => return crate::reply_missing_code_block_err(&args),
-    };
+    let code = crate::extract_code(args.body)?;
 
     let (flags, flag_parse_errors) = parse_flags(&args);