PythonistaGuild / TwitchIO

An Async Bot/API wrapper for Twitch made in Python.
https://twitchio.dev
MIT License
785 stars 163 forks source link

Argument parsing errors not handled by event_command_error #422

Closed snazzyfox closed 11 months ago

snazzyfox commented 1 year ago

Description

When I use the commands extension, declare arguments, and receive an invalid command, the resulting exception is not handleable in the command itself, and also not caught by event_command_error. This makes it impossible to provide user feedback I cannot make use of anything other than strings for command arguments, as it's impossible to handle these errors gracefully.

Example

class Bot(commands.Bot):
    @commands.command()
    async def hello(self, ctx: commands.Context, target: twitchio.User):
        await ctx.send(f"Hello, {target.name}!")

    async def event_command_error(self, context: commands.Context, error: Exception) -> None:
        if isinstance(error, commands.errors.BadArgument):
            context.send(f'{context.author.mention} <- Error: {error.message}')

Desired behavior

User: !hello
Bot: Error: Missing required argument: target
User: !hello @invalid_user
Bot: Error: "invalid_user" is not a valid user
User: !hello @valid_user
Bot: Hello, valid_user!

Actual behavior

User: !hello

Bot: silence in chat; traceback:

"\twitchio\ext\commands\core.py", line 174, in parse_args
    raise MissingRequiredArgument(param)
twitchio.ext.commands.errors.MissingRequiredArgument: target: twitchio.user.User
User: !hello @invalid-user
"\twitchio\ext\commands\builtin_converter.py", line 89, in convert_User
    raise BadArgument(f"User '{arg}' was not found.")
twitchio.ext.commands.errors.BadArgument: User 'invalid-user' was not found.

Additional Context

The only workaround I've found for this issue is to make all arguments across the bot be a string with a default of empty string, and do all parsing/conversion within the event handler so that these errors can be caught. This is incredibly verbose and repetitive and in my opinion defeats the purpose of supporting conversion using types in the first place.

async def hello(self, ctx: commands.Context, target: str = ''):
    target_user = parse_user(target)  # may throw a BadArgument exception, which can now be caught properly

Although these errors can't be caught by event_command_error, they can be caught by event_error. However, this function only provides the exception object without a command context, and therefore it's impossible to know which channel the error is and which user it came from, so it's not possible to respond to the user.

In addition, if the user sends a message that uses the same prefix but is not a command defined in this bot (which happens frequently, as many channels use multiple off-the-shelf bots with the same prefix), the "command not found" error is caught by this handler. So I think it intuitively makes sense that since the lifecycle of a "command" has already started at the time of matching commands, argument parsing after a command is found would also be included here.

EvieePy commented 1 year ago

Howdy thanks for bringing this up. I have pushed a possible fix for this issue: https://github.com/PythonistaGuild/TwitchIO/commit/097201e5c92af1b489f4f8c619262e397d1a784b

You can install this from GitHub:

pip install -U git+https://github.com/PythonistaGuild/TwitchIO.git --force-reinstall

You can then test this in your error handler, as an example.

import sys
import traceback

async def event_command_error(self, context: commands.Context, error: Exception) -> None:
    if isinstance(error, commands.BadArgument):
        await context.send(f'{context.author.mention} <- Error: {error.message}')

    elif isinstance(error, commands.MissingRequiredArgument):
        await context.send(f'Missing required argument(s): {".".join(a.name for a in error.args)}')

    else:
        print(f"Ignoring exception in command: {error}:", file=sys.stderr)
        traceback.print_exception(type(error), error, error.__traceback__, file=sys.stderr)
snazzyfox commented 1 year ago

Oh wow, thanks for the amazingly fast response! I can confirm that the both of these errors can be caught in event_command_error in the updated commit :)