feat: Proper error handling

- Bump version to 0.8.0
- Added discord error struct
- All errors now implement std::error::Error
- Implemented error handler instead of relying on default
- Fixed bug where you could send answer on a completed quest

Cargo.lock

@@ -1599,7 +1599,7 @@ dependencies = [
 
 [[package]]
 name = "squad-quest"
-version = "0.7.0"
+version = "0.8.0"
 dependencies = [
  "serde",
  "toml",
@@ -1607,7 +1607,7 @@ dependencies = [
 
 [[package]]
 name = "squad-quest-cli"
-version = "0.7.0"
+version = "0.8.0"
 dependencies = [
  "chrono",
  "clap",
@@ -1618,7 +1618,7 @@ dependencies = [
 
 [[package]]
 name = "squad-quest-discord"
-version = "0.7.0"
+version = "0.8.0"
 dependencies = [
  "clap",
  "dotenvy",

Cargo.toml

@@ -2,7 +2,7 @@
 members = ["cli", "discord"]
 
 [workspace.package]
-version = "0.7.0"
+version = "0.8.0"
 edition = "2024"
 repository = "https://2ndbeam.ru/git/2ndbeam/squad-quest"
 license = "MIT"

cli/Cargo.toml

@@ -9,5 +9,5 @@ license.workspace = true
 chrono = "0.4.42"
 clap = { version = "4.5.53", features = ["derive"] }
 serde = { version = "1.0.228", features = ["derive"] }
-squad-quest = { version = "0.7.0", path = ".." }
+squad-quest = { version = "0.8.0", path = ".." }
 toml = "0.9.8"

discord/Cargo.toml

@@ -10,6 +10,6 @@ clap = { version = "4.5.53", features = ["derive"] }
 dotenvy = "0.15.7"
 poise = "0.6.1"
 serde = "1.0.228"
-squad-quest = { version = "0.7.0", path = ".." }
+squad-quest = { version = "0.8.0", path = ".." }
 tokio = { version = "1.48.0", features = ["rt-multi-thread"] }
 toml = "0.9.8"

discord/src/commands/answer.rs

@@ -21,11 +21,15 @@ pub async fn answer(
     #[description = "Attachment answer to the quest"]
     file3: Option<Attachment>,
 ) -> Result<(), Error> {
+    let mut account = fetch_or_init_account(&ctx.data().config, ctx.author().id.to_string());
+
+    if let Some(_) = account.quests_completed.iter().find(|qid| **qid == quest_id) {
+        return Err(Error::QuestIsCompleted(quest_id));
+    }
+
     let quests = ctx.data().config.load_quests();
     let Some(quest) = quests.iter().find(|q| q.id == quest_id) else {
-        let reply_string = format!("Quest #{quest_id} not found.");
+        return Err(Error::QuestNotFound(quest_id));
-        ctx.reply(reply_string).await?;
-        return Ok(());
     };
     
     let mut files: Vec<Attachment> = Vec::new();
@@ -36,9 +40,7 @@ pub async fn answer(
     }
 
     if text.is_none() && files.len() == 0 {
-        let reply_string = "Please specify text or at least one attachment.".to_string();
+        return Err(Error::NoContent);
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
 
     let text_ans = match text {
@@ -90,7 +92,7 @@ pub async fn answer(
     let reply_string = "Your answer has been posted.".to_string();
     ctx.reply(reply_string).await?;
 
-    while let Some(press) = ComponentInteractionCollector::new(ctx)
+    if let Some(press) = ComponentInteractionCollector::new(ctx)
         .filter(move |press| press.data.custom_id.starts_with(&ctx_id.to_string()))
         .await
     {
@@ -108,7 +110,6 @@ pub async fn answer(
         let content: String;
         if is_approved {
             let mut no_errors = true;
-            let mut account = fetch_or_init_account(&ctx.data().config, ctx.author().id.to_string());
             if let Err(error) = quest.complete_for_account(&mut account) {
                 eprintln!("{error}");
                 no_errors = false;
@@ -137,9 +138,6 @@ pub async fn answer(
         };
         let dm_builder = CreateMessage::new().content(content);
         ctx.author().dm(ctx, dm_builder).await?;
-
-
-        return Ok(());
     }
 
     Ok(())

discord/src/commands/init.rs

@@ -3,7 +3,7 @@ use std::path::Path;
 use poise::serenity_prelude::{ChannelId};
 use squad_quest::SquadObject;
 
-use crate::{commands::ERROR_MSG, Context, Error};
+use crate::{Context, Error};
 
 
 #[poise::command(
@@ -19,8 +19,8 @@ pub async fn init(
     #[description = "Channel to post answers to check"]
     answers_channel: ChannelId,
 ) -> Result<(), Error> {
-    let reply_string = {
     let dc = ctx.data().discord.clone();
+    {
         let mut guard = dc.lock().expect("shouldn't be locked");
         let guild = ctx.guild_id().unwrap();
         guard.quests_channel = quests_channel;
@@ -28,14 +28,9 @@ pub async fn init(
         guard.guild = guild;
     
         let path = &ctx.data().config.full_impl_path().unwrap();
-        match guard.save(path.parent().unwrap_or(Path::new("")).to_owned()) {
+        guard.save(path.parent().unwrap_or(Path::new("")).to_owned())?
-            Ok(_) => "Settings updated.".to_string(),
-            Err(error) => {
-                eprintln!("{error}");
-                ERROR_MSG.to_string()
-            },
-        }
     };
+    let reply_string = "Settings updated.".to_string();
     ctx.reply(reply_string).await?;
 
     Ok(())

discord/src/commands/mod.rs

@@ -1,6 +1,7 @@
-//use poise::{CreateReply, serenity_prelude as serenity};
+use std::error::Error as StdError;
+use poise::CreateReply;
 
-use crate::{Context, Error};
+use crate::{Context, Data, Error};
 
 pub mod quest;
 pub mod init;
@@ -8,10 +9,30 @@ pub mod answer;
 pub mod social;
 pub mod account;
 
-pub const ERROR_MSG: &str = "Server error :(";
-
 #[poise::command(prefix_command)]
 pub async fn register(ctx: Context<'_>) -> Result<(), Error> {
     poise::builtins::register_application_commands_buttons(ctx).await?;
     Ok(())
 }
+
+pub async fn error_handler(error: poise::FrameworkError<'_, Data, Error>) {
+    eprintln!("ERROR:");
+    print_error_recursively(&error);
+    if let Some(ctx) = error.ctx() {
+        let response = match error {
+            poise::FrameworkError::Command { error, .. } => format!("Internal server error: {error}"),
+            _ => format!("Internal server error: {error}"),
+        };
+        if let Err(error) = ctx.send(CreateReply::default().content(response).ephemeral(true)).await {
+            eprintln!("Couldn't send error message: {error}");
+        }
+    }
+}
+
+fn print_error_recursively(error: &impl StdError) {
+    eprintln!("{error}");
+    if let Some(source) = error.source() {
+        eprintln!("source:");
+        print_error_recursively(&source);
+    }
+}

discord/src/commands/quest.rs

@@ -3,12 +3,12 @@ use std::{future, path::Path, str::FromStr};
 use poise::serenity_prelude::{CreateMessage, EditMessage, Message, futures::StreamExt};
 use squad_quest::{SquadObject, quest::{Quest, QuestDifficulty}};
 use toml::value::Date;
-use crate::{Context, Error, commands::ERROR_MSG};
+use crate::{Context, Error};
 
-async fn find_quest_message(ctx: Context<'_>, id: u16) -> Option<Message>{
+async fn find_quest_message(ctx: Context<'_>, id: u16) -> Result<Option<Message>, Error>{
-    let _ = ctx.defer().await;
+    ctx.defer().await?;
-    let channel = {
     let dc = ctx.data().discord.clone();
+    let channel = {
         let guard = dc.lock().expect("shouldn't be locked");
         guard.quests_channel
     };
@@ -21,7 +21,7 @@ async fn find_quest_message(ctx: Context<'_>, id: u16) -> Option<Message>{
             future::ready(m.content.contains(&format!("#{id}")))
         })
         .collect::<Vec<_>>().await;
-    messages.first().cloned()
+    Ok(messages.first().cloned())
 }
 
 fn make_quest_message_content(quest: &Quest) -> String {
@@ -163,13 +163,8 @@ pub async fn create(
     
     let path = conf.full_quests_path();
     
-    let reply_string = match quest.save(path) {
+    quest.save(path)?;
-        Ok(_) => format!("Created quest #{}", quest.id),
+    let reply_string = format!("Created quest #{}", quest.id);
-        Err(error) => {
-            eprintln!("{error}");
-            format!("{ERROR_MSG}")
-        },
-    };
 
     ctx.reply(reply_string).await?;
 
@@ -207,9 +202,7 @@ pub async fn update(
     let conf = &ctx.data().config;
     let quests = conf.load_quests();
     let Some(quest) = quests.iter().find(|q| q.id == id) else {
-        let reply_string = format!("Quest #{id} not found");
+        return Err(Error::QuestNotFound(id));
-        ctx.reply(reply_string).await?;
-        return Ok(());
     };
 
     let difficulty = match difficulty {
@@ -248,8 +241,8 @@ pub async fn update(
         let content = make_quest_message_content(&new_quest);
         let builder = EditMessage::new().content(content);
         
-        let message = find_quest_message(ctx, id);
+        let message = find_quest_message(ctx, id).await?;
-        if let Some(mut message) = message.await {
+        if let Some(mut message) = message {
             message.edit(ctx, builder).await?;
         } else {
             let reply_string = format!("Quest #{id} is public, but its message was not found in the quest channel",
@@ -259,13 +252,8 @@ pub async fn update(
     }
     
     let path = conf.full_quests_path();
-    let reply_string = match new_quest.save(path) {
+    new_quest.save(path)?;
-        Err(error) => {
+    let reply_string = format!("Updated quest #{id}");
-            eprintln!("{error}");
-            ERROR_MSG.to_string()
-        },
-        Ok(_) => format!("Updated quest #{id}"),
-    };
     ctx.reply(reply_string).await?;
 
     Ok(())
@@ -285,15 +273,11 @@ pub async fn publish(
     let mut quests = ctx.data().config.load_quests();
     
     let Some(quest) = quests.iter_mut().find(|q| q.id == id) else {
-        let reply_string = format!("Quest #{id} not found");
+        return Err(Error::QuestNotFound(id));
-        ctx.reply(reply_string).await?;
-        return Ok(());
     };
 
     if quest.public {
-        let reply_string = format!("Quest #{id} is already public");
+        return Err(Error::QuestIsPublic(id));
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
     
     quest.public = true;
@@ -311,27 +295,15 @@ pub async fn publish(
 
     let message = channel.send_message(ctx, builder).await?;
 
-    let result = {
+    {
         let mut guard = dc.lock().expect("shouldn't be locked");
         guard.quests_messages.push(message.id);
         let path = ctx.data().config.full_impl_path().unwrap();
-        guard.save(path.parent().unwrap_or(Path::new("")).to_owned())
+        guard.save(path.parent().unwrap_or(Path::new("")).to_owned())?
     };
 
-    if let Err(error) = result {
-        eprintln!("{error}");
-        let reply_string = ERROR_MSG.to_string();
-        ctx.reply(reply_string).await?;
-        return Ok(())
-    }
-
     let quests_path = ctx.data().config.full_quests_path();
-    if let Err(error) = quest.save(quests_path) {
+    quest.save(quests_path)?;
-        eprintln!("{error}");
-        let reply_string = ERROR_MSG.to_string();
-        ctx.reply(reply_string).await?;
-        return Ok(())
-    }
 
     let reply_string = format!("Published quest #{id}");
     ctx.reply(reply_string).await?;
@@ -349,18 +321,13 @@ pub async fn delete(
     ctx: Context<'_>,
     id: u16,
 ) -> Result<(), Error> {
-    if let Some(msg) = find_quest_message(ctx, id).await {
+    if let Some(msg) = find_quest_message(ctx, id).await? {
         msg.delete(ctx).await?;
     }
 
     let mut path = ctx.data().config.full_quests_path();
     path.push(format!("{id}.toml"));
-    if let Err(error) = Quest::delete(path) {
+    Quest::delete(path)?;
-        eprintln!("{error}");
-        let reply_string = ERROR_MSG.to_string();
-        ctx.reply(reply_string).await?;
-        return Ok(());
-    }
 
     let mut accounts = ctx.data().config.load_accounts();
     let accounts_path = ctx.data().config.full_accounts_path();
@@ -368,9 +335,7 @@ pub async fn delete(
     for account in accounts.iter_mut().filter(|a| a.quests_completed.contains(&id)) {
         let index = account.quests_completed.iter().position(|qid| *qid == id).expect("We just filtered it");
         account.quests_completed.remove(index);
-        if let Err(error) = account.save(accounts_path.clone()) {
+        account.save(accounts_path.clone())?;
-            eprintln!("{error}");
-        }
     }
 
     let reply_string = format!("Successfully deleted quest #{id}");

discord/src/commands/social.rs

@@ -28,21 +28,15 @@ pub async fn msg (
 
 ) -> Result<(), Error> {
     if channel.is_none() && user.is_none() {
-        let reply_string = "Please specify channel **or** user".to_string();
+        return Err(Error::NoChannelOrUser);
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
 
     if channel.is_some() && user.is_some() {
-        let reply_string = "Please specify **only** channel or **only** user, not both".to_string();
+        return Err(Error::BothChannelAndUser);
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
 
     if content.is_none() && file.is_none() {
-        let reply_string = "Please specify content and/or file".to_string();
+        return Err(Error::NoContent);
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
 
     let mut builder = CreateMessage::new();
@@ -53,16 +47,8 @@ pub async fn msg (
 
     if let Some(file) = file {
         ctx.defer().await?;
-        let attachment = CreateAttachment::url(ctx, &file.url).await;
+        let attachment = CreateAttachment::url(ctx, &file.url).await?;
-        match attachment {
+        builder = builder.add_file(attachment);
-            Ok(attachment) => { builder = builder.add_file(attachment); },
-            Err(error) => {
-                eprintln!("{error}");
-                let reply_string = "Couldn't copy attachment, aborting";
-                ctx.reply(reply_string).await?;
-                return Ok(());
-            },
-        }
     }
 
     let reply_string = if let Some(channel) = channel {
@@ -104,21 +90,15 @@ pub async fn edit (
     file: Option<Attachment>,
 ) -> Result<(), Error> {
     if channel.is_none() && user.is_none() {
-        let reply_string = "Please specify channel **or** user".to_string();
+        return Err(Error::NoChannelOrUser);
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
 
     if channel.is_some() && user.is_some() {
-        let reply_string = "Please specify **only** channel or **only** user, not both".to_string();
+        return Err(Error::BothChannelAndUser);
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
 
     if content.is_none() && file.is_none() {
-        let reply_string = "Please specify content and/or file".to_string();
+        return Err(Error::NoContent);
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
     
     let mut builder = EditMessage::new();
@@ -129,16 +109,8 @@ pub async fn edit (
     
     if let Some(file) = file {
         ctx.defer().await?;
-        let attachment = CreateAttachment::url(ctx, &file.url).await;
+        let attachment = CreateAttachment::url(ctx, &file.url).await?;
-        match attachment {
+        builder = builder.new_attachment(attachment);
-            Ok(attachment) => { builder = builder.new_attachment(attachment); },
-            Err(error) => {
-                eprintln!("{error}");
-                let reply_string = "Couldn't copy attachment, aborting";
-                ctx.reply(reply_string).await?;
-                return Ok(());
-            },
-        }
     }
 
     if let Some(channel) = channel {
@@ -171,15 +143,11 @@ pub async fn undo(
 ) -> Result<(), Error> {
 
     if channel.is_none() && user.is_none() {
-        let reply_string = "Please specify channel **or** user".to_string();
+        return Err(Error::NoChannelOrUser);
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
 
     if channel.is_some() && user.is_some() {
-        let reply_string = "Please specify **only** channel or **only** user, not both".to_string();
+        return Err(Error::BothChannelAndUser);
-        ctx.reply(reply_string).await?;
-        return Ok(());
     }
 
     if let Some(channel) = channel {

discord/src/config.rs

@@ -4,7 +4,7 @@ use poise::serenity_prelude::{ChannelId, GuildId, MessageId};
 use serde::{Serialize, Deserialize};
 use squad_quest::{SquadObject, config::Config, error::Error};
 
-#[derive(Serialize, Deserialize, Default, Clone)]
+#[derive(Serialize, Deserialize, Default, Clone, Debug)]
 pub struct DiscordConfig {
     pub guild: GuildId,
     pub quests_channel: ChannelId,

discord/src/error.rs

@@ -0,0 +1,60 @@
+use std::fmt::Display;
+
+use poise::serenity_prelude as serenity;
+
+#[non_exhaustive]
+#[derive(Debug)]
+pub enum Error {
+    QuestNotFound(u16),
+    QuestIsPublic(u16),
+    QuestIsCompleted(u16),
+    NoContent,
+    NoChannelOrUser,
+    BothChannelAndUser,
+    SerenityError(serenity::Error),
+    SquadQuestError(squad_quest::error::Error),
+}
+
+impl From<serenity::Error> for Error {
+    fn from(value: serenity::Error) -> Self {
+        Self::SerenityError(value)
+    }
+}
+
+impl From<squad_quest::error::Error> for Error {
+    fn from(value: squad_quest::error::Error) -> Self {
+        Self::SquadQuestError(value)
+    }
+}
+
+impl Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::QuestNotFound(u16) => write!(f, "quest #{u16} not found"),
+            Self::QuestIsPublic(u16) => write!(f, "quest #{u16} is already public"),
+            Self::QuestIsCompleted(u16) => write!(f, "quest #{u16} is already completed for this user"),
+            Self::NoContent => write!(f, "no text or attachment was specified"),
+            Self::NoChannelOrUser => write!(f, "no channel or user was specified"),
+            Self::BothChannelAndUser => write!(f, "both channel and user was specified"),
+            Self::SerenityError(_) => write!(f, "discord interaction error"),
+            Self::SquadQuestError(_) => write!(f, "internal logic error"),
+        }
+    }
+}
+
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            Self::QuestNotFound(_) |
+            Self::QuestIsPublic(_) |
+            Self::QuestIsCompleted(_) |
+            Self::NoContent |
+            Self::NoChannelOrUser |
+            Self::BothChannelAndUser => None,
+            Self::SerenityError(error) => Some(error),
+            Self::SquadQuestError(error) => Some(error),
+        }
+    }
+}
+
+

discord/src/main.rs

@@ -5,18 +5,19 @@ use dotenvy::dotenv;
 use poise::serenity_prelude as serenity;
 use squad_quest::config::Config;
 
-use crate::config::{ConfigImpl, DiscordConfig};
+use crate::{commands::error_handler, config::{ConfigImpl, DiscordConfig}, error::Error};
 
 mod commands;
 mod cli;
 mod config;
 mod account;
+mod error;
 
+#[derive(Debug)]
 struct Data {
     pub config: Config,
     pub discord: Arc<Mutex<DiscordConfig>>,
 }
-type Error = Box<dyn std::error::Error + Send + Sync>;
 type Context<'a> = poise::Context<'a, Data, Error>;
 
 #[tokio::main]
@@ -35,6 +36,7 @@ async fn main() {
 
     let framework = poise::Framework::builder()
         .options(poise::FrameworkOptions {
+            on_error: |err| Box::pin(error_handler(err)),
             commands: vec![
                 commands::quest::quest(),
                 commands::register(),

src/config/mod.rs

@@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize};
 use crate::{SquadObject, account::Account, error::Error, quest::Quest};
 
 /// Struct for containing paths to other (de-)serializable things
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
 #[serde(default)]
 pub struct Config {
     /// Path to config directory

src/error.rs

@@ -23,14 +23,26 @@ impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::IsNotAFile(path) => write!(f, "{path:?} is not a file"),
-            Self::IoError(error) => write!(f, "{error}"),
+            Self::IoError(_) => write!(f, "io error"),
-            Self::TomlSerializeError(error) => write!(f, "{error}"),
+            Self::TomlSerializeError(_) => write!(f, "TOML serialization error"),
-            Self::TomlDeserializeError(error) => write!(f, "{error}"),
+            Self::TomlDeserializeError(_) => write!(f, "TOML deserialization error"),
             Self::IsNotImplemented => write!(f, "implementation not found"),
         }
     }
 }
 
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            Self::IsNotAFile(_) |
+            Self::IsNotImplemented => None,
+            Self::IoError(error) => Some(error),
+            Self::TomlSerializeError(error) => Some(error),
+            Self::TomlDeserializeError(error) => Some(error),
+        }
+    }
+}
+
 /// Error related to quest logic
 #[derive(Debug)]
 #[non_exhaustive]
@@ -47,6 +59,8 @@ impl fmt::Display for QuestError {
     }
 }
 
+impl std::error::Error for QuestError {}
+
 /// Error related to map logic
 #[derive(Debug)]
 #[non_exhaustive]
@@ -68,3 +82,5 @@ impl fmt::Display for MapError {
         }
     }
 }
+
+impl std::error::Error for MapError {}