From 99812c5d7c6a74806f6595927e0cc05449e1a10b Mon Sep 17 00:00:00 2001 From: 2ndbeam <2ndbeam@disroot.org> Date: Fri, 12 Dec 2025 16:52:31 +0300 Subject: [PATCH] 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 | 6 +-- Cargo.toml | 2 +- cli/Cargo.toml | 2 +- discord/Cargo.toml | 2 +- discord/src/commands/answer.rs | 20 ++++----- discord/src/commands/init.rs | 15 +++---- discord/src/commands/mod.rs | 29 +++++++++++-- discord/src/commands/quest.rs | 77 ++++++++++------------------------ discord/src/commands/social.rs | 56 ++++++------------------- discord/src/config.rs | 2 +- discord/src/error.rs | 60 ++++++++++++++++++++++++++ discord/src/main.rs | 6 ++- src/config/mod.rs | 2 +- src/error.rs | 22 ++++++++-- 14 files changed, 163 insertions(+), 138 deletions(-) create mode 100644 discord/src/error.rs diff --git a/Cargo.lock b/Cargo.lock index 7d0148e..5a8e429 100644 --- a/Cargo.lock +++ b/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", diff --git a/Cargo.toml b/Cargo.toml index 95bbcce..8e47446 100644 --- a/Cargo.toml +++ b/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" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index b448666..1027d59 100644 --- a/cli/Cargo.toml +++ b/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" diff --git a/discord/Cargo.toml b/discord/Cargo.toml index ed6de15..5b754a9 100644 --- a/discord/Cargo.toml +++ b/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" diff --git a/discord/src/commands/answer.rs b/discord/src/commands/answer.rs index f2b0816..32b76ab 100644 --- a/discord/src/commands/answer.rs +++ b/discord/src/commands/answer.rs @@ -21,11 +21,15 @@ pub async fn answer( #[description = "Attachment answer to the quest"] file3: Option, ) -> 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."); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::QuestNotFound(quest_id)); }; let mut files: Vec = 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(); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::NoContent); } 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(()) diff --git a/discord/src/commands/init.rs b/discord/src/commands/init.rs index 6b16bb5..e642921 100644 --- a/discord/src/commands/init.rs +++ b/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 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()) { - Ok(_) => "Settings updated.".to_string(), - Err(error) => { - eprintln!("{error}"); - ERROR_MSG.to_string() - }, - } + guard.save(path.parent().unwrap_or(Path::new("")).to_owned())? }; + let reply_string = "Settings updated.".to_string(); ctx.reply(reply_string).await?; Ok(()) diff --git a/discord/src/commands/mod.rs b/discord/src/commands/mod.rs index 7e988f8..5f57941 100644 --- a/discord/src/commands/mod.rs +++ b/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); + } +} diff --git a/discord/src/commands/quest.rs b/discord/src/commands/quest.rs index 7b97a13..b7e4896 100644 --- a/discord/src/commands/quest.rs +++ b/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{ - let _ = ctx.defer().await; +async fn find_quest_message(ctx: Context<'_>, id: u16) -> Result, Error>{ + ctx.defer().await?; + let dc = ctx.data().discord.clone(); let channel = { - let dc = ctx.data().discord.clone(); 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{ future::ready(m.content.contains(&format!("#{id}"))) }) .collect::>().await; - messages.first().cloned() + Ok(messages.first().cloned()) } fn make_quest_message_content(quest: &Quest) -> String { @@ -162,14 +162,9 @@ pub async fn create( }; let path = conf.full_quests_path(); - - let reply_string = match quest.save(path) { - Ok(_) => format!("Created quest #{}", quest.id), - Err(error) => { - eprintln!("{error}"); - format!("{ERROR_MSG}") - }, - }; + + quest.save(path)?; + let reply_string = format!("Created quest #{}", quest.id); 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"); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::QuestNotFound(id)); }; 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); - if let Some(mut message) = message.await { + let message = find_quest_message(ctx, id).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) { - Err(error) => { - eprintln!("{error}"); - ERROR_MSG.to_string() - }, - Ok(_) => format!("Updated quest #{id}"), - }; + new_quest.save(path)?; + let reply_string = 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"); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::QuestNotFound(id)); }; if quest.public { - let reply_string = format!("Quest #{id} is already public"); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::QuestIsPublic(id)); } 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) { - eprintln!("{error}"); - let reply_string = ERROR_MSG.to_string(); - ctx.reply(reply_string).await?; - return Ok(()) - } + quest.save(quests_path)?; 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) { - eprintln!("{error}"); - let reply_string = ERROR_MSG.to_string(); - ctx.reply(reply_string).await?; - return Ok(()); - } + Quest::delete(path)?; 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()) { - eprintln!("{error}"); - } + account.save(accounts_path.clone())?; } let reply_string = format!("Successfully deleted quest #{id}"); diff --git a/discord/src/commands/social.rs b/discord/src/commands/social.rs index 9b3578b..118b593 100644 --- a/discord/src/commands/social.rs +++ b/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(); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::NoChannelOrUser); } if channel.is_some() && user.is_some() { - let reply_string = "Please specify **only** channel or **only** user, not both".to_string(); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::BothChannelAndUser); } if content.is_none() && file.is_none() { - let reply_string = "Please specify content and/or file".to_string(); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::NoContent); } 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; - match 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 attachment = CreateAttachment::url(ctx, &file.url).await?; + builder = builder.add_file(attachment); } let reply_string = if let Some(channel) = channel { @@ -104,21 +90,15 @@ pub async fn edit ( file: Option, ) -> Result<(), Error> { if channel.is_none() && user.is_none() { - let reply_string = "Please specify channel **or** user".to_string(); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::NoChannelOrUser); } if channel.is_some() && user.is_some() { - let reply_string = "Please specify **only** channel or **only** user, not both".to_string(); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::BothChannelAndUser); } if content.is_none() && file.is_none() { - let reply_string = "Please specify content and/or file".to_string(); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::NoContent); } 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; - match 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(()); - }, - } + let attachment = CreateAttachment::url(ctx, &file.url).await?; + builder = builder.new_attachment(attachment); } 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(); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::NoChannelOrUser); } if channel.is_some() && user.is_some() { - let reply_string = "Please specify **only** channel or **only** user, not both".to_string(); - ctx.reply(reply_string).await?; - return Ok(()); + return Err(Error::BothChannelAndUser); } if let Some(channel) = channel { diff --git a/discord/src/config.rs b/discord/src/config.rs index 6bcdf7b..fee6498 100644 --- a/discord/src/config.rs +++ b/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, diff --git a/discord/src/error.rs b/discord/src/error.rs new file mode 100644 index 0000000..feee407 --- /dev/null +++ b/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 for Error { + fn from(value: serenity::Error) -> Self { + Self::SerenityError(value) + } +} + +impl From 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), + } + } +} + + diff --git a/discord/src/main.rs b/discord/src/main.rs index 391fb16..7fa2d26 100644 --- a/discord/src/main.rs +++ b/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>, } -type Error = Box; 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(), diff --git a/src/config/mod.rs b/src/config/mod.rs index 9544042..07aa1f1 100644 --- a/src/config/mod.rs +++ b/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 diff --git a/src/error.rs b/src/error.rs index abb475c..1cc4654 100644 --- a/src/error.rs +++ b/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::TomlSerializeError(error) => write!(f, "{error}"), - Self::TomlDeserializeError(error) => write!(f, "{error}"), + Self::IoError(_) => write!(f, "io error"), + Self::TomlSerializeError(_) => write!(f, "TOML serialization 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 {}