pcparadise / discordbot

GNU General Public License v3.0
3 stars 5 forks source link

Handle command exceptions #16

Closed Ganoodles closed 1 year ago

Ganoodles commented 3 years ago

We need to create an exception handler for when a command goes haywire.

awsomearvinder commented 3 years ago

I'm actually not sure it's nessecary https://discordpy.readthedocs.io/en/stable/api.html#discord.on_error https://discordpy.readthedocs.io/en/stable/ext/commands/api.html?highlight=cog#discord.ext.commands.Bot.on_command_error This documentation implies that it prints the error information to stderr - and that's exactly how I'd want it to handle errors. It won't take down the bot, although it might take down the cog or the command, which frankly, I'm okay with since it's clearly a bug that should be fixed.

lukadd16 commented 3 years ago

I'm actually not sure it's nessecary https://discordpy.readthedocs.io/en/stable/api.html#discord.on_error https://discordpy.readthedocs.io/en/stable/ext/commands/api.html?highlight=cog#discord.ext.commands.Bot.on_command_error This documentation implies that it prints the error information to stderr - and that's exactly how I'd want it to handle errors. It won't take down the bot, although it might take down the cog or the command, which frankly, I'm okay with since it's clearly a bug that should be fixed.

@awsomearvinder I can agree with keeping on_error output to stderr and/or even an external .log file, but you fail to consider situations when on_command_error is called as a result of a non-code issue.

A user passing arguments of one type to a command that is expecting those arguments to be of a different type would throw BadArgument, but this error isn't indicative of a bug or issue with the code. This is in fact exactly an error that we would want to catch and handle properly. And by handle properly I mean actually sending a message or embed to the context that the command was called from to tell the user why the command failed to execute (I know personally, I'd be pretty infuriated if I didn't get any feedback from an application as to why it failed to perform the action I requested, and I'm sure you would be too).

Another case where on_command_error is called for a non-code issue is if you assigned a cooldown to a particular command and a user tripped the rate limit for that command (commands.CommandOnCooldown would be thrown). In this situation it would be helpful to inform the user how long they must wait until they are no longer being rate limited.

Examples of other errors you should ideally be handling include NotOwner (member tried to run a command that only bot owners can use), MissingPermissions (when user doesn't have the required permissions to run this command), BotMissingPermissions (when the bot doesn't have the required permissions to run this command), and the list goes on...

I suggest you take a look at the entire list of commands-related Exceptions that the library may throw to get a better idea of what you need to catch and what you might be able to ignore.

awsomearvinder commented 3 years ago

Fair point on all of those - for the log file though, I prefer logging to stderr by default since you can just capture the input and send it to a file pretty trivially in a cli. It also gets rid of the annoying file handling that can come with logging, just passing it off to the user, shell and whatever other tools they use to deal with it.

Both options are valid, I just prefer the stderr route. Otherwise yeah, we should handle those errors you mentioned and probably others.