marcelropos / HM-DiscordBot

A Discordbot by and for first semester students of HM.
8 stars 5 forks source link

Command subjects #197

Open schitcrafter opened 9 months ago

schitcrafter commented 9 months ago

Implements #180 #181 #182 #183 #184

TODO:

schitcrafter commented 7 months ago

I've just tested it manually, seems to work well. We should figure out how best to handle errors when for example the user has no study group role - I'd like to avoid replying to the user on discord in every function / when anything fails, as we'd need the context in every function, so it would be nice if we could have either some generic error (like the anyhow and miette) or create a custom error using thiserror, bubbling up errors using normal Result's and replying with them.

Errors thrown by a command function can be caught using an on_error handler (this can do much more, like catch command panics, btw). This means that we can use either of the options - I've tried out miette, and it works beautifully. We need to add the dependency, add a simple error handler (~40 lines of code, because I've added a lot of information), and whenever we want to return an error (that the user should see), we simply do return Err(miette::miette!("Some error here - supports fmt like arguments: {x}"));

maxwai commented 7 months ago

I've just tested it manually, seems to work well. We should figure out how best to handle errors when for example the user has no study group role - I'd like to avoid replying to the user on discord in every function / when anything fails, as we'd need the context in every function, so it would be nice if we could have either some generic error (like the anyhow and miette) or create a custom error using thiserror, bubbling up errors using normal Result's and replying with them.

Errors thrown by a command function can be caught using an on_error handler (this can do much more, like catch command panics, btw). This means that we can use either of the options - I've tried out miette, and it works beautifully. We need to add the dependency, add a simple error handler (~40 lines of code, because I've added a lot of information), and whenever we want to return an error (that the user should see), we simply do return Err(miette::miette!("Some error here - supports fmt like arguments: {x}"));

I think the best would be to define our own errors (in an error module) and then handle them in on_error function (put the actual handling in the error module and just call the handler in on_error, keeping it as short as possible)

Edit: for inspiration one can look at how it was done in the python version: https://github.com/marcelropos/HM-DiscordBot/tree/master/core/error

schitcrafter commented 7 months ago

I'm gonna guess that this is done so that individual errors can report their own information in a customizable way, with discord-specific ways of formatting?

Also, I think the best way to implement this (with the above presumption) is to use thiserror to get a std::error::Error implementation for a custom error type (which will be the main error type for this project), and then create a method on the error type which spits out a CreateReply. That way, we can use the easy formatting options from thiserror for most of the simple errors, while being able to match on the more complex errors and generate a discord-specific reply.

maxwai commented 7 months ago

I'm gonna guess that this is done so that individual errors can report their own information in a customizable way, with discord-specific ways of formatting?

Yes exactly.

to use thiserror to get a std::error::Error implementation for a custom error type, and then create a method on the error type which spits out a CreateReply

Yes sound goods

schitcrafter commented 7 months ago

Alright - is it okay if I put this in the scope of this pr? Would be good if I had error handling, and with thiserror, I don't think it's that much work

maxwai commented 7 months ago

Yes put it in here