[BUG] ModmailBot.on_command_error does not respect cog's cog_command_error error handler.

[Jerrie-Aries]

Bot Version

v4.0.0-dev16

How are you hosting Modmail?

Other

Error Logs

None

Screenshots

No response

Additional Information

[PLUGIN] Both cog's cog_command_error and ModmailBot.on_command_error handlers catch the same exception.

Reproduce:
Create a plugin with a error handler.

import discord

from discord.ext import commands

from core import checks
from core.models import PermissionLevel


class MyPlugin(commands.Cog):
    def __init__(self, bot):
        self.bot = bot

    async def cog_command_error(self, ctx, error):
        if isinstance(error, TypeError):
            embed = discord.Embed(color=self.bot.error_color, description=str(error))
            await ctx.send(embed=embed)
        return
    
    @commands.command(name="raise")
    @checks.has_permissions(PermissionLevel.OWNER)
    async def raise_error(self, ctx):
        raise TypeError("Error raised.")

Expected result:
The ModmailBot.on_command_error handler should not be called since the error has already been dealt with within the cog/plugin.

Actual result:
Both cog_command_error and ModmailBot.on_command_error handlers catch the exception.
And since ModmailBot.on_command_error doesn't handle TypeError, the error is raised and traceback is printed on console.

Error:

2022-07-06 16:10:49 __main__[1547] - ERROR: Unexpected exception:
Traceback (most recent call last):
  File "/Users/Projects/modmail/.venv/lib/python3.9/site-packages/discord/ext/commands/core.py", line 200, in wrapped
    ret = await coro(*args, **kwargs)
  File "/Users/Projects/modmail/plugins/@local/moderation/moderation.py", line 309, in raise_error
    raise TypeError("Error raised.")
TypeError: Error raised.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/Projects/modmail/.venv/lib/python3.9/site-packages/discord/ext/commands/bot.py", line 1330, in invoke
    await ctx.command.invoke(ctx)
  File "/Users/Projects/modmail/.venv/lib/python3.9/site-packages/discord/ext/commands/core.py", line 995, in invoke
    await injected(*ctx.args, **ctx.kwargs)  # type: ignore
  File "/Users/Projects/modmail/.venv/lib/python3.9/site-packages/discord/ext/commands/core.py", line 209, in wrapped
    raise CommandInvokeError(exc) from exc
discord.ext.commands.errors.CommandInvokeError: Command raised an exception: TypeError: Error raised.

[sebkuip]

This is due to how discord.py handles errors. The only thing I can imagine is that the bot checks if an error is not part of a plugin before processing it. You can check out how discord.py internally dispatches error events here

[Jerrie-Aries]

I have a couple of suggestions.

Suggested solutions:
Add an if... block in the ModmailBot.on_command_error to check whether the context has already had a error handler or something.
And I could only think of these two.

Solution 1:
Example in bot.py:

class ModmailBot(commands.Bot):
    ...

    async def on_command_error(self, context, exception):
        # this is to deal with command specific handlers, e.g. `@kick.error`, etc
        command = context.command
        if command and command.has_error_handler():
            return
        
        # this is to deal with the `async def cog_command_error` if it's even defined in the cog
        cog = context.cog
        if cog and cog.has_error_handler():
            return
        
        # the rest of the code

This is the default implemantation of commands.Bot.on_command_error in discord.py library, can be found here.
But there is a gotcha, if the plugin has a custom error handler this handler will always return early. In this case, plugin developers would have to deal with other errors (the ones that should have been dealt with in on_command_error, e.g. BadArgument) as well.
Otherwise the bot would just eat the errors silently.

Solution 2:
Same with Solution 1 except with an addition of allowing the ModmailBot.on_command_error be called from the cog in case the error is not handled by cog or the plugin developers do not want to handle some type of errors, e.g. BadArgument.
And personally I prefer this solution since it allows plugin developers to implement their own error handlers in their plugins and at the same time still be able to use the default one in bot.py.
Example in bot.py:

class ModmailBot(commands.Bot):
    ...
    
    # add a new parameter `unhandled_by_cog` which default to `False`
    async def on_command_error(self, context, exception, unhandled_by_cog=False):
        if not unhandled_by_cog:
            command = context.command
            if command and command.has_error_handler():
                return

            cog = context.cog
            if cog and cog.has_error_handler():
                return
        
        # the rest of the code

Example in myplugin.py

class MyPlugin(commands.Cog):
    def __init__(self, bot):
        self.bot = bot

    async def cog_command_error(self, ctx, error):
        handled = False
        if isinstance(error, TypeError):
            await ctx.send(str(error))
            handled = True
        elif isinstance(error, MyCustomException):
            # do things
            handled = True
        # some other checks
        
        
        # the error is not handled here so let the `ModmailBot.on_command_error` do its thing
        if not handled:
            await self.bot.on_command_error(ctx, error, unhandled_by_cog=True)

Reference:

• Red-DiscordBot, in redbot/core/evets.py #L200-208.