Return error on unrecognized commands.

src/bin/dicebot-cmd.rs

@@ -7,7 +7,7 @@ use chronicle_dicebot::error::BotError;
 async fn main() -> Result<(), BotError> {
     let db = Database::new_temp()?;
     let input = std::env::args().skip(1).collect::<Vec<String>>().join(" ");
-    let command = match commands::parse(&input) {
+    let command = match commands::parser::parse_command(&input) {
         Ok(command) => command,
         Err(e) => return Err(e),
     };

src/bot.rs

@@ -128,9 +128,8 @@ impl DiceBot {
 
         for command in commands {
             let ctx = Context::new(&self.db, &room_id.as_str(), &sender_username, &command);
-            if let Some(cmd_result) = execute_command(&ctx).await {
-                results.push(cmd_result);
-            }
+            let cmd_result = execute_command(&ctx).await;
+            results.push(cmd_result);
         }
 
         if results.len() >= 1 {

src/commands.rs

@@ -1,7 +1,5 @@
 use crate::context::Context;
-use crate::error::{BotError, BotError::CommandParsingError};
 use async_trait::async_trait;
-use parser::CommandParsingError::UnrecognizedCommand;
 use thiserror::Error;
 
 pub mod basic_rolling;
@@ -41,20 +39,6 @@ pub trait Command: Send + Sync {
     fn name(&self) -> &'static str;
 }
 
-/// Parse a command string into a dynamic command execution trait
-/// object. Returns an error if a command was recognized but not
-/// parsed correctly. Returns IgnoredCommand error if no command was
-/// recognized.
-pub fn parse(s: &str) -> Result<Box<dyn Command>, BotError> {
-    match parser::parse_command(s) {
-        Ok(command) => Ok(command),
-        Err(CommandParsingError(UnrecognizedCommand(_))) => {
-            Err(CommandError::IgnoredCommand.into())
-        }
-        Err(e) => Err(e),
-    }
-}
-
 #[derive(Debug)]
 pub struct CommandResult {
     pub plain: String,
@@ -65,15 +49,14 @@ pub struct CommandResult {
 /// go back to Matrix, if the command was executed (successfully or
 /// not). If a command is determined to be ignored, this function will
 /// return None, signifying that we should not send a response.
-pub async fn execute_command(ctx: &Context<'_>) -> Option<CommandResult> {
-    let res = parse(&ctx.message_body);
+pub async fn execute_command(ctx: &Context<'_>) -> CommandResult {
+    let res = parser::parse_command(&ctx.message_body);
 
     let (plain, html) = match res {
         Ok(cmd) => {
             let execution = cmd.execute(ctx).await;
             (execution.plain().into(), execution.html().into())
         }
-        Err(BotError::CommandError(CommandError::IgnoredCommand)) => return None,
         Err(e) => {
             let message = format!("Error parsing command: {}", e);
             let html_message = format!("<p><strong>{}</strong></p>", message);
@@ -84,51 +67,21 @@ pub async fn execute_command(ctx: &Context<'_>) -> Option<CommandResult> {
     let plain = format!("{}\n{}", ctx.username, plain);
     let html = format!("<p>{}</p>\n{}", ctx.username, html);
 
-    Some(CommandResult {
+    CommandResult {
         plain: plain,
         html: html,
-    })
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
-    #[test]
-    fn chance_die_is_not_malformed() {
-        assert!(parse("!chance").is_ok());
-    }
-
-    #[test]
-    fn roll_malformed_expression_test() {
-        assert!(parse("!roll 1d20asdlfkj").is_err());
-        assert!(parse("!roll 1d20asdlfkj   ").is_err());
-    }
-
-    #[test]
-    fn roll_dice_pool_malformed_expression_test() {
-        assert!(parse("!pool 8abc").is_err());
-        assert!(parse("!pool 8abc    ").is_err());
-    }
-
-    #[test]
-    fn pool_whitespace_test() {
-        parse("!pool ns3:8   ").expect("was error");
-        parse("   !pool ns3:8").expect("was error");
-        parse("   !pool ns3:8   ").expect("was error");
-    }
-
-    #[test]
-    fn help_whitespace_test() {
-        parse("!help stuff   ").expect("was error");
-        parse("   !help stuff").expect("was error");
-        parse("   !help stuff   ").expect("was error");
-    }
-
-    #[test]
-    fn roll_whitespace_test() {
-        parse("!roll 1d4 + 5d6 -3   ").expect("was error");
-        parse("!roll 1d4 + 5d6 -3   ").expect("was error");
-        parse("   !roll 1d4 + 5d6 -3   ").expect("was error");
+    #[tokio::test]
+    async fn unrecognized_command() {
+        let db = crate::db::Database::new_temp().unwrap();
+        let ctx = Context::new(&db, "myroomid", "@testuser:example.com", "!notacommand");
+        let result = execute_command(&ctx).await;
+        assert!(result.plain.contains("Error"));
     }
 }

src/commands/parser.rs

@@ -116,7 +116,7 @@ fn split_command(input: &str) -> Result<(String, String), CommandParsingError> {
 
 /// Potentially parse a command expression. If we recognize the
 /// command, an error should be raised if the command is misparsed. If
-/// we don't recognize the command, ignore it and return None.
+/// we don't recognize the command, return an error.
 pub fn parse_command(input: &str) -> Result<Box<dyn Command>, BotError> {
     match split_command(input) {
         Ok((cmd, cmd_input)) => match cmd.as_ref() {
@@ -130,7 +130,6 @@ pub fn parse_command(input: &str) -> Result<Box<dyn Command>, BotError> {
             "cthadv" | "cthARoll" => parse_cth_advancement_roll(&cmd_input),
             "chance" => chance_die(),
             "help" => help(&cmd_input),
-            // No recognized command, ignore this.
             _ => Err(CommandParsingError::UnrecognizedCommand(cmd).into()),
         },
         //All other errors passed up.
@@ -240,4 +239,42 @@ mod tests {
         assert!(split_command("roll 1d4").is_err());
         assert!(split_command("roll").is_err());
     }
+
+    #[test]
+    fn chance_die_is_not_malformed() {
+        assert!(parse_command("!chance").is_ok());
+    }
+
+    #[test]
+    fn roll_malformed_expression_test() {
+        assert!(parse_command("!roll 1d20asdlfkj").is_err());
+        assert!(parse_command("!roll 1d20asdlfkj   ").is_err());
+    }
+
+    #[test]
+    fn roll_dice_pool_malformed_expression_test() {
+        assert!(parse_command("!pool 8abc").is_err());
+        assert!(parse_command("!pool 8abc    ").is_err());
+    }
+
+    #[test]
+    fn pool_whitespace_test() {
+        parse_command("!pool ns3:8   ").expect("was error");
+        parse_command("   !pool ns3:8").expect("was error");
+        parse_command("   !pool ns3:8   ").expect("was error");
+    }
+
+    #[test]
+    fn help_whitespace_test() {
+        parse_command("!help stuff   ").expect("was error");
+        parse_command("   !help stuff").expect("was error");
+        parse_command("   !help stuff   ").expect("was error");
+    }
+
+    #[test]
+    fn roll_whitespace_test() {
+        parse_command("!roll 1d4 + 5d6 -3   ").expect("was error");
+        parse_command("!roll 1d4 + 5d6 -3   ").expect("was error");
+        parse_command("   !roll 1d4 + 5d6 -3   ").expect("was error");
+    }
 }