From 6457601a2b326980d28e699236a9a823299e1f98 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 22 Sep 2021 22:19:47 -0400 Subject: [PATCH 01/14] feat: add initial error handler --- modmail/extensions/utils/error_handler.py | 145 ++++++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 modmail/extensions/utils/error_handler.py diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py new file mode 100644 index 00000000..003c690c --- /dev/null +++ b/modmail/extensions/utils/error_handler.py @@ -0,0 +1,145 @@ +import logging +import typing + +import discord +import discord.errors +from discord.ext import commands + +from modmail.bot import ModmailBot +from modmail.log import ModmailLogger +from modmail.utils.cogs import ExtMetadata, ModmailCog + + +logger: ModmailLogger = logging.getLogger(__name__) + +EXT_METADATA = ExtMetadata() + +ERROR_COLOUR = discord.Colour.red() + + +class ErrorHandler(ModmailCog, name="Error Handler"): + """Handles all errors across the bot.""" + + def __init__(self, bot: ModmailBot): + self.bot = bot + + @commands.command() + async def cmd(self, ctx: commands.Context, error: str = None, *args) -> None: + """.""" + if error is not None: + raise getattr(commands, error)(*args) + else: + await ctx.send("oop") + + @staticmethod + def error_embed(message: str, title: str = None) -> discord.Embed: + """Create an error embed with an error colour and reason and return it.""" + return discord.Embed(message, colour=ERROR_COLOUR, title=title or "Error occured") + + async def handle_user_input_error( + self, ctx: commands.Context, error: commands.UserInputError + ) -> discord.Embed: + """Handling deferred from main error handler to handle UserInputErrors.""" + embed = None + if isinstance(error, commands.BadArgument): + embed = self.error_embed(str(error), title="Bad argument") + elif isinstance(error, commands.MissingRequiredArgument): + embed = self.error_embed(str(error), title="Missing required argument") + elif isinstance(error, commands.TooManyArguments): + embed = self.error_embed(str(error), title="Too many arguments") + elif isinstance(error, commands.BadUnionArgument): + # TODO: complete + msg = "" + embed = self.error_embed(msg, title="Bad union argument") + elif isinstance(error, commands.BadLiteralArgument): + # TODO: complete + msg = "" + embed = self.error_embed(msg, title="Bad literal argument") + elif isinstance(error, commands.ArgumentParsingError): + embed = self.error_embed(str(error), title="Argument parsing error") + if isinstance(error, commands.UnexpectedQuoteError): + ... + elif isinstance(error, commands.InvalidEndOfQuotedStringError): + ... + elif isinstance(error, commands.ExpectedClosingQuoteError): + ... + else: + ... + else: + ... + return embed + + async def handle_check_failure(self, ctx: commands.Context, error: commands.CheckFailure) -> ...: + """Handle CheckFailures seperately given that there are many of them.""" + ... + + @ModmailCog.listener() + async def on_command_error(self, ctx: commands.Context, error: commands.CommandError) -> None: + """Activates when a command raises an error.""" + if getattr(error, "handled", False): + logging.debug(f"Command {ctx.command} had its error already handled locally; ignoring.") + return + + if not isinstance(error, commands.CommandError): + logger.error("What in the world...") + return + logger.trace(error) + if isinstance(error, commands.CommandNotFound): + # ignore every time the user inputs a message that starts with our prefix but isn't a command + # this will be modified in the future to support prefilled commands + return + + embed: typing.Optional[discord.Embed] = None + should_respond = True + + if isinstance(error, commands.UserInputError): + embed = await self.handle_user_input_error(ctx, error) + elif isinstance(error, commands.CheckFailure): + ... + elif isinstance(error, commands.ConversionError): + s = object() + error.converter.convert.__annotations__.get("return", s) + embed = error + + elif isinstance(error, commands.DisabledCommand): + logger.debug("") + if ctx.command.hidden: + should_respond = False + else: + msg = f"Command `{ctx.invoked_with}` is disabled." + if reason := ctx.command.extras.get("disabled_reason", None): + msg += f"\nReason: {reason}" + embed = self.error_embed(msg, title="Command disabled") + + elif isinstance(error, commands.CommandInvokeError): + # generic error + logger.error(f"Error occured in {ctx.command}.", exc_info=error.original) + # todo: this should log somewhere else since this is a bot bug. + embed = self.error_embed( + "Oops! Something went wrong internally in the command you were trying to execute. " + "Please report this error and what you were trying to do to the developer." + ) + elif isinstance(error, commands.CommandOnCooldown): + ... + elif isinstance(error, commands.MaxConcurrencyReached): + ... + else: + logger.error("An error was made that was unhandlable.") + + # TODO: this has a fundamental problem with any BotMissingPermissions error + # if the issue is the bot does not have permissions to send embeds or send messages... + # yeah, problematic. + + if not should_respond: + logger.debug("Not responding to error since should_respond is falsey.") + return + + if embed is not None: + await ctx.send(embeds=[embed]) + else: + await ctx.send("Uhm. Something happened. IDK what.") + + +def setup(bot: ModmailBot) -> None: + """Add the error handler to the bot.""" + bot.add_cog(ErrorHandler(bot)) From 716e1213c3e9ceeb0b4780d70d22fd1b51a5a823 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Thu, 23 Sep 2021 04:45:23 -0400 Subject: [PATCH 02/14] add some CheckFailure handling support --- modmail/extensions/utils/error_handler.py | 52 ++++++++++++++++------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index 003c690c..fae6e344 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -1,4 +1,5 @@ import logging +import re import typing import discord @@ -17,12 +18,23 @@ ERROR_COLOUR = discord.Colour.red() +def _raise(e: BaseException, *arg, **kwargs) -> typing.NoReturn: + raise e + + class ErrorHandler(ModmailCog, name="Error Handler"): """Handles all errors across the bot.""" def __init__(self, bot: ModmailBot): self.bot = bot + @commands.check(lambda _: _raise(commands.NotOwner("oy"))) + @commands.command() + async def error(self, ctx: commands.Context) -> None: + """.""" + await ctx.send(5) + + @commands.has_permissions(administrator=False) @commands.command() async def cmd(self, ctx: commands.Context, error: str = None, *args) -> None: """.""" @@ -41,22 +53,17 @@ async def handle_user_input_error( ) -> discord.Embed: """Handling deferred from main error handler to handle UserInputErrors.""" embed = None - if isinstance(error, commands.BadArgument): - embed = self.error_embed(str(error), title="Bad argument") - elif isinstance(error, commands.MissingRequiredArgument): - embed = self.error_embed(str(error), title="Missing required argument") - elif isinstance(error, commands.TooManyArguments): - embed = self.error_embed(str(error), title="Too many arguments") - elif isinstance(error, commands.BadUnionArgument): + title = "User Input Error" + if isinstance(error, commands.BadUnionArgument): # TODO: complete msg = "" - embed = self.error_embed(msg, title="Bad union argument") + embed = self.error_embed(msg, title="Bad Union Argument") elif isinstance(error, commands.BadLiteralArgument): # TODO: complete msg = "" - embed = self.error_embed(msg, title="Bad literal argument") + embed = self.error_embed(msg, title="Bad Literal Argument") elif isinstance(error, commands.ArgumentParsingError): - embed = self.error_embed(str(error), title="Argument parsing error") + embed = self.error_embed(str(error), title="Argument Parsing Error") if isinstance(error, commands.UnexpectedQuoteError): ... elif isinstance(error, commands.InvalidEndOfQuotedStringError): @@ -66,12 +73,27 @@ async def handle_user_input_error( else: ... else: - ... - return embed + title = re.sub(r"([A-Z])", r" \1", error.__class__.__name__) + return embed or self.error_embed(str(error), title=title) - async def handle_check_failure(self, ctx: commands.Context, error: commands.CheckFailure) -> ...: + async def handle_check_failure( + self, ctx: commands.Context, error: commands.CheckFailure + ) -> discord.Embed: """Handle CheckFailures seperately given that there are many of them.""" - ... + title = "Check Failure" + embed = None + if isinstance(error, commands.CheckAnyFailure): + ... + elif isinstance(error, commands.PrivateMessageOnly): + title = "DMs Only" + elif isinstance(error, commands.NoPrivateMessage): + title = "Server Only" + elif isinstance(error, commands.NSFWChannelRequired): + title = "NSFW Channel Required" + else: + title = re.sub(r"([A-Z])", r" \1", error.__class__.__name__) + embed = self.error_embed(str(error), title=title) + return embed @ModmailCog.listener() async def on_command_error(self, ctx: commands.Context, error: commands.CommandError) -> None: @@ -95,7 +117,7 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE if isinstance(error, commands.UserInputError): embed = await self.handle_user_input_error(ctx, error) elif isinstance(error, commands.CheckFailure): - ... + embed = await self.handle_check_failure(ctx, error) elif isinstance(error, commands.ConversionError): s = object() error.converter.convert.__annotations__.get("return", s) From cfac97c8e0891d7b406c846075af3658aedf3fa1 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Thu, 23 Sep 2021 15:25:33 -0400 Subject: [PATCH 03/14] fix: implement remaining UserInputErrors --- modmail/extensions/utils/error_handler.py | 71 ++++++++--------------- 1 file changed, 24 insertions(+), 47 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index fae6e344..5f8619e9 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -18,81 +18,58 @@ ERROR_COLOUR = discord.Colour.red() -def _raise(e: BaseException, *arg, **kwargs) -> typing.NoReturn: - raise e - - class ErrorHandler(ModmailCog, name="Error Handler"): """Handles all errors across the bot.""" def __init__(self, bot: ModmailBot): self.bot = bot - @commands.check(lambda _: _raise(commands.NotOwner("oy"))) - @commands.command() - async def error(self, ctx: commands.Context) -> None: - """.""" - await ctx.send(5) - - @commands.has_permissions(administrator=False) - @commands.command() - async def cmd(self, ctx: commands.Context, error: str = None, *args) -> None: - """.""" - if error is not None: - raise getattr(commands, error)(*args) - else: - await ctx.send("oop") - @staticmethod def error_embed(message: str, title: str = None) -> discord.Embed: """Create an error embed with an error colour and reason and return it.""" - return discord.Embed(message, colour=ERROR_COLOUR, title=title or "Error occured") + return discord.Embed(message, colour=ERROR_COLOUR, title=title or "Error Occured") + + @staticmethod + def get_title_from_name(error: typing.Union[Exception, str]) -> str: + """ + Return a message dervived from the exception class name. + + Eg NSFWChannelRequired returns NSFW Channel Required + """ + if not isinstance(error, str): + error = error.__class__.__name__ + return re.sub(r"(?<=[a-z])([A-Z])(?=[a-z])", r" \1", error) async def handle_user_input_error( self, ctx: commands.Context, error: commands.UserInputError ) -> discord.Embed: """Handling deferred from main error handler to handle UserInputErrors.""" embed = None + msg = None title = "User Input Error" if isinstance(error, commands.BadUnionArgument): - # TODO: complete - msg = "" - embed = self.error_embed(msg, title="Bad Union Argument") - elif isinstance(error, commands.BadLiteralArgument): - # TODO: complete - msg = "" - embed = self.error_embed(msg, title="Bad Literal Argument") - elif isinstance(error, commands.ArgumentParsingError): - embed = self.error_embed(str(error), title="Argument Parsing Error") - if isinstance(error, commands.UnexpectedQuoteError): - ... - elif isinstance(error, commands.InvalidEndOfQuotedStringError): - ... - elif isinstance(error, commands.ExpectedClosingQuoteError): - ... - else: - ... + msg = self.get_title_from_name(str(error)) + title = self.get_title_from_name(error) else: - title = re.sub(r"([A-Z])", r" \1", error.__class__.__name__) - return embed or self.error_embed(str(error), title=title) + title = self.get_title_from_name(error) + return embed or self.error_embed(msg or str(error), title=title) async def handle_check_failure( self, ctx: commands.Context, error: commands.CheckFailure ) -> discord.Embed: """Handle CheckFailures seperately given that there are many of them.""" title = "Check Failure" - embed = None + msg = None if isinstance(error, commands.CheckAnyFailure): - ... + title = self.get_title_from_name(error.checks[-1]) + msg = str(error) elif isinstance(error, commands.PrivateMessageOnly): title = "DMs Only" elif isinstance(error, commands.NoPrivateMessage): title = "Server Only" - elif isinstance(error, commands.NSFWChannelRequired): - title = "NSFW Channel Required" else: - title = re.sub(r"([A-Z])", r" \1", error.__class__.__name__) - embed = self.error_embed(str(error), title=title) + title = self.get_title_from_name(error) + embed = self.error_embed(msg or str(error), title=title) return embed @ModmailCog.listener() @@ -135,11 +112,11 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE elif isinstance(error, commands.CommandInvokeError): # generic error - logger.error(f"Error occured in {ctx.command}.", exc_info=error.original) + logger.error(f'Error occured in command "{ctx.command}".', exc_info=error.original) # todo: this should log somewhere else since this is a bot bug. embed = self.error_embed( "Oops! Something went wrong internally in the command you were trying to execute. " - "Please report this error and what you were trying to do to the developer." + "Please report this error and what you were trying to do to the developers." ) elif isinstance(error, commands.CommandOnCooldown): ... From 3e64f9efe30fe0b3737f90553339a11a08603939 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Thu, 23 Sep 2021 16:53:40 -0400 Subject: [PATCH 04/14] fix: title regex --- modmail/extensions/utils/error_handler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index 5f8619e9..58110f43 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -17,6 +17,8 @@ ERROR_COLOUR = discord.Colour.red() +ERROR_TITLE_REGEX = re.compile(r"(?<=[a-zA-Z])([A-Z])(?=[a-z])") + class ErrorHandler(ModmailCog, name="Error Handler"): """Handles all errors across the bot.""" @@ -38,7 +40,7 @@ def get_title_from_name(error: typing.Union[Exception, str]) -> str: """ if not isinstance(error, str): error = error.__class__.__name__ - return re.sub(r"(?<=[a-z])([A-Z])(?=[a-z])", r" \1", error) + return re.sub(ERROR_TITLE_REGEX, r" \1", error) async def handle_user_input_error( self, ctx: commands.Context, error: commands.UserInputError From 59e700ed4a70fccca402be950895e0feec78bfb7 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Fri, 24 Sep 2021 18:36:52 -0400 Subject: [PATCH 05/14] fix: add concurrency errors and cooldown error support --- modmail/extensions/utils/error_handler.py | 39 +++++++++++++---------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index 58110f43..448398d5 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -42,10 +42,21 @@ def get_title_from_name(error: typing.Union[Exception, str]) -> str: error = error.__class__.__name__ return re.sub(ERROR_TITLE_REGEX, r" \1", error) + @staticmethod + def _reset_command_cooldown(ctx: commands.Context) -> bool: + if return_value := ctx.command.is_on_cooldown(ctx): + ctx.command.reset_cooldown(ctx) + return return_value + async def handle_user_input_error( - self, ctx: commands.Context, error: commands.UserInputError + self, + ctx: commands.Context, + error: commands.UserInputError, + reset_cooldown: bool = True, ) -> discord.Embed: """Handling deferred from main error handler to handle UserInputErrors.""" + if reset_cooldown: + self._reset_command_cooldown(ctx) embed = None msg = None title = "User Input Error" @@ -90,6 +101,8 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE # this will be modified in the future to support prefilled commands return + title = None + msg = None embed: typing.Optional[discord.Embed] = None should_respond = True @@ -98,10 +111,10 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE elif isinstance(error, commands.CheckFailure): embed = await self.handle_check_failure(ctx, error) elif isinstance(error, commands.ConversionError): - s = object() - error.converter.convert.__annotations__.get("return", s) - embed = error - + # s = object() + # error.converter.convert.__annotations__.get("return", s) + # embed = error + ... elif isinstance(error, commands.DisabledCommand): logger.debug("") if ctx.command.hidden: @@ -110,7 +123,7 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE msg = f"Command `{ctx.invoked_with}` is disabled." if reason := ctx.command.extras.get("disabled_reason", None): msg += f"\nReason: {reason}" - embed = self.error_embed(msg, title="Command disabled") + embed = self.error_embed(msg, title="Command Disabled") elif isinstance(error, commands.CommandInvokeError): # generic error @@ -120,12 +133,6 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE "Oops! Something went wrong internally in the command you were trying to execute. " "Please report this error and what you were trying to do to the developers." ) - elif isinstance(error, commands.CommandOnCooldown): - ... - elif isinstance(error, commands.MaxConcurrencyReached): - ... - else: - logger.error("An error was made that was unhandlable.") # TODO: this has a fundamental problem with any BotMissingPermissions error # if the issue is the bot does not have permissions to send embeds or send messages... @@ -135,10 +142,10 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE logger.debug("Not responding to error since should_respond is falsey.") return - if embed is not None: - await ctx.send(embeds=[embed]) - else: - await ctx.send("Uhm. Something happened. IDK what.") + if embed is None: + embed = self.error_embed(msg or str(error), title=title or self.get_title_from_name(error)) + + await ctx.send(embeds=[embed]) def setup(bot: ModmailBot) -> None: From 09ee9de6e239ebfeb335181e5d8b88c71b7e6094 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Fri, 24 Sep 2021 19:27:32 -0400 Subject: [PATCH 06/14] fix: don't log CommandNotFound errors unless in a dev mode --- modmail/extensions/utils/error_handler.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index 448398d5..02f2026a 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -8,7 +8,8 @@ from modmail.bot import ModmailBot from modmail.log import ModmailLogger -from modmail.utils.cogs import ExtMetadata, ModmailCog +from modmail.utils.cogs import BotModes, ExtMetadata, ModmailCog +from modmail.utils.extensions import BOT_MODE logger: ModmailLogger = logging.getLogger(__name__) @@ -19,6 +20,8 @@ ERROR_TITLE_REGEX = re.compile(r"(?<=[a-zA-Z])([A-Z])(?=[a-z])") +ANY_DEV_MODE = BOT_MODE & (BotModes.DEVELOP.value + BotModes.PLUGIN_DEV.value) + class ErrorHandler(ModmailCog, name="Error Handler"): """Handles all errors across the bot.""" @@ -92,15 +95,15 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE logging.debug(f"Command {ctx.command} had its error already handled locally; ignoring.") return - if not isinstance(error, commands.CommandError): - logger.error("What in the world...") - return - logger.trace(error) if isinstance(error, commands.CommandNotFound): # ignore every time the user inputs a message that starts with our prefix but isn't a command # this will be modified in the future to support prefilled commands + if ANY_DEV_MODE: + logger.trace(error) return + logger.trace(error) + title = None msg = None embed: typing.Optional[discord.Embed] = None From 5062b49c7e54daf1e3e2670460da542a8a7be30f Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sat, 25 Sep 2021 12:44:05 -0400 Subject: [PATCH 07/14] fix: handle bot missing permissions --- modmail/extensions/utils/error_handler.py | 41 ++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index 02f2026a..edcc2f7f 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -70,9 +70,38 @@ async def handle_user_input_error( title = self.get_title_from_name(error) return embed or self.error_embed(msg or str(error), title=title) + async def handle_bot_missing_perms( + self, ctx: commands.Context, error: commands.BotMissingPermissions + ) -> bool: + """Handles bot missing perms by dming the user if they have a permission which may be to fix this.""" + embed = self.error_embed(str(error)) + try: + await ctx.send(embeds=[embed]) + except discord.Forbidden: + logger.error(f"Unable to send an error message to {ctx.channel!s}") + if ANY_DEV_MODE: + # non-general permissions + perms = discord.Permissions( + administrator=True, + manage_threads=True, + manage_roles=True, + manage_channels=True, + ) + if perms.value & ctx.channel.permissions_for(ctx.author).value: + logger.info( + f"Attempting to dm {ctx.author} since they have a permission which may be able " + "to give the bot send message permissions." + ) + try: + await ctx.author.send(embeds=[embed]) + except discord.Forbidden: + logger.notice("Also encountered an error when trying to reply in dms.") + return False + return True + async def handle_check_failure( self, ctx: commands.Context, error: commands.CheckFailure - ) -> discord.Embed: + ) -> typing.Optional[discord.Embed]: """Handle CheckFailures seperately given that there are many of them.""" title = "Check Failure" msg = None @@ -83,6 +112,10 @@ async def handle_check_failure( title = "DMs Only" elif isinstance(error, commands.NoPrivateMessage): title = "Server Only" + elif isinstance(error, commands.BotMissingPermissions): + # defer handling BotMissingPermissions to a method, since this can be problematic + await self.handle_bot_missing_perms(ctx, error) + return None else: title = self.get_title_from_name(error) embed = self.error_embed(msg or str(error), title=title) @@ -113,6 +146,9 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE embed = await self.handle_user_input_error(ctx, error) elif isinstance(error, commands.CheckFailure): embed = await self.handle_check_failure(ctx, error) + # handle_check_failure may send its own error if its a BotMissingPermissions error. + if embed is None: + should_respond = False elif isinstance(error, commands.ConversionError): # s = object() # error.converter.convert.__annotations__.get("return", s) @@ -136,6 +172,9 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE "Oops! Something went wrong internally in the command you were trying to execute. " "Please report this error and what you were trying to do to the developers." ) + if isinstance(error.original, discord.Forbidden): + await self.handle_bot_missing_perms(ctx, error.original) + return # TODO: this has a fundamental problem with any BotMissingPermissions error # if the issue is the bot does not have permissions to send embeds or send messages... From 34ab6065e945799660a033b81f7efcc3d9cec27e Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sat, 25 Sep 2021 12:46:03 -0400 Subject: [PATCH 08/14] changes: add error handler changelog entry --- docs/changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.md b/docs/changelog.md index 14225c04..bf3b6581 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Running the bot after configuring the env vars is now as simple as `docker-compose up` - Automatic docker image creation: `ghcr.io/discord-modmail/modmail` (#19) - Dockerfile support for all supported hosting providers. (#58) +- Errors no longer happen silently and notify the user when they make a mistake. (#77) ### Changed From 5e84f72268f52f30346465fb64464cb4285e7282 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sat, 25 Sep 2021 13:19:03 -0400 Subject: [PATCH 09/14] fix typos --- modmail/extensions/utils/error_handler.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index edcc2f7f..7c858226 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -32,7 +32,7 @@ def __init__(self, bot: ModmailBot): @staticmethod def error_embed(message: str, title: str = None) -> discord.Embed: """Create an error embed with an error colour and reason and return it.""" - return discord.Embed(message, colour=ERROR_COLOUR, title=title or "Error Occured") + return discord.Embed(message, colour=ERROR_COLOUR, title=title or "Error Occurred") @staticmethod def get_title_from_name(error: typing.Union[Exception, str]) -> str: @@ -73,7 +73,7 @@ async def handle_user_input_error( async def handle_bot_missing_perms( self, ctx: commands.Context, error: commands.BotMissingPermissions ) -> bool: - """Handles bot missing perms by dming the user if they have a permission which may be to fix this.""" + """Handles bot missing permissing by dming the user if they have a permission which may be able to fix this.""" # noqa: E501 embed = self.error_embed(str(error)) try: await ctx.send(embeds=[embed]) @@ -166,7 +166,7 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE elif isinstance(error, commands.CommandInvokeError): # generic error - logger.error(f'Error occured in command "{ctx.command}".', exc_info=error.original) + logger.error(f'Error occurred in command "{ctx.command}".', exc_info=error.original) # todo: this should log somewhere else since this is a bot bug. embed = self.error_embed( "Oops! Something went wrong internally in the command you were trying to execute. " From c00d2364d6c533d6217cf265fbf6f81ef0fa438c Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 26 Sep 2021 01:00:50 -0400 Subject: [PATCH 10/14] fix: remove unused variables, commented out code, fix method call --- modmail/extensions/utils/error_handler.py | 42 ++++++++++------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index 7c858226..24d1534e 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -30,9 +30,9 @@ def __init__(self, bot: ModmailBot): self.bot = bot @staticmethod - def error_embed(message: str, title: str = None) -> discord.Embed: + def error_embed(title: str, message: str) -> discord.Embed: """Create an error embed with an error colour and reason and return it.""" - return discord.Embed(message, colour=ERROR_COLOUR, title=title or "Error Occurred") + return discord.Embed(title=title, description=message, colour=ERROR_COLOUR) @staticmethod def get_title_from_name(error: typing.Union[Exception, str]) -> str: @@ -60,25 +60,22 @@ async def handle_user_input_error( """Handling deferred from main error handler to handle UserInputErrors.""" if reset_cooldown: self._reset_command_cooldown(ctx) - embed = None msg = None - title = "User Input Error" if isinstance(error, commands.BadUnionArgument): msg = self.get_title_from_name(str(error)) - title = self.get_title_from_name(error) - else: - title = self.get_title_from_name(error) - return embed or self.error_embed(msg or str(error), title=title) + title = self.get_title_from_name(error) + return self.error_embed(title, msg or str(error)) async def handle_bot_missing_perms( self, ctx: commands.Context, error: commands.BotMissingPermissions ) -> bool: """Handles bot missing permissing by dming the user if they have a permission which may be able to fix this.""" # noqa: E501 - embed = self.error_embed(str(error)) + embed = self.error_embed("Permissions Failure", str(error)) try: await ctx.send(embeds=[embed]) except discord.Forbidden: logger.error(f"Unable to send an error message to {ctx.channel!s}") + if ANY_DEV_MODE: # non-general permissions perms = discord.Permissions( @@ -104,10 +101,8 @@ async def handle_check_failure( ) -> typing.Optional[discord.Embed]: """Handle CheckFailures seperately given that there are many of them.""" title = "Check Failure" - msg = None if isinstance(error, commands.CheckAnyFailure): title = self.get_title_from_name(error.checks[-1]) - msg = str(error) elif isinstance(error, commands.PrivateMessageOnly): title = "DMs Only" elif isinstance(error, commands.NoPrivateMessage): @@ -118,14 +113,14 @@ async def handle_check_failure( return None else: title = self.get_title_from_name(error) - embed = self.error_embed(msg or str(error), title=title) + embed = self.error_embed(title, str(error)) return embed @ModmailCog.listener() async def on_command_error(self, ctx: commands.Context, error: commands.CommandError) -> None: """Activates when a command raises an error.""" if getattr(error, "handled", False): - logging.debug(f"Command {ctx.command} had its error already handled locally; ignoring.") + logging.debug(f"Command {ctx.command} had its error already handled locally, ignoring.") return if isinstance(error, commands.CommandNotFound): @@ -137,8 +132,6 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE logger.trace(error) - title = None - msg = None embed: typing.Optional[discord.Embed] = None should_respond = True @@ -150,10 +143,7 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE if embed is None: should_respond = False elif isinstance(error, commands.ConversionError): - # s = object() - # error.converter.convert.__annotations__.get("return", s) - # embed = error - ... + pass elif isinstance(error, commands.DisabledCommand): logger.debug("") if ctx.command.hidden: @@ -162,30 +152,34 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE msg = f"Command `{ctx.invoked_with}` is disabled." if reason := ctx.command.extras.get("disabled_reason", None): msg += f"\nReason: {reason}" - embed = self.error_embed(msg, title="Command Disabled") + embed = self.error_embed("Command Disabled", msg or str(error)) elif isinstance(error, commands.CommandInvokeError): # generic error logger.error(f'Error occurred in command "{ctx.command}".', exc_info=error.original) # todo: this should log somewhere else since this is a bot bug. embed = self.error_embed( + "Error Occurred", "Oops! Something went wrong internally in the command you were trying to execute. " - "Please report this error and what you were trying to do to the developers." + "Please report this error and what you were trying to do to the developers.", ) if isinstance(error.original, discord.Forbidden): await self.handle_bot_missing_perms(ctx, error.original) - return + should_respond = False # TODO: this has a fundamental problem with any BotMissingPermissions error # if the issue is the bot does not have permissions to send embeds or send messages... # yeah, problematic. if not should_respond: - logger.debug("Not responding to error since should_respond is falsey.") + logger.debug( + "Not responding to error since should_respond is falsey because either " + "the embed has already been sent or belongs to a hidden command and thus should be hidden." + ) return if embed is None: - embed = self.error_embed(msg or str(error), title=title or self.get_title_from_name(error)) + embed = self.error_embed(self.get_title_from_name(error), str(error)) await ctx.send(embeds=[embed]) From 7f199961a15842651bc7c60ea3c1b7a8bf1320c4 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 26 Sep 2021 01:27:58 -0400 Subject: [PATCH 11/14] fix: don't attempt to send a message if we aren't able to send messages --- modmail/extensions/utils/error_handler.py | 75 ++++++++++++++--------- 1 file changed, 46 insertions(+), 29 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index 24d1534e..ef902192 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -71,29 +71,44 @@ async def handle_bot_missing_perms( ) -> bool: """Handles bot missing permissing by dming the user if they have a permission which may be able to fix this.""" # noqa: E501 embed = self.error_embed("Permissions Failure", str(error)) - try: + bot_perms = ctx.channel.permissions_for(ctx.me) + responded = None + if bot_perms >= discord.Permissions(send_messages=True, embed_links=True): await ctx.send(embeds=[embed]) - except discord.Forbidden: - logger.error(f"Unable to send an error message to {ctx.channel!s}") - - if ANY_DEV_MODE: - # non-general permissions - perms = discord.Permissions( - administrator=True, - manage_threads=True, - manage_roles=True, - manage_channels=True, + responded = True + elif bot_perms >= discord.Permissions(send_messages=True): + # make a message as similar to the embed, using as few permissions as possible + # this is the only place we send a standard message instead of an embed, so no helper methods + await ctx.send( + "**Permissions Failure**\n\n" + "I am missing the permissions required to properly execute your command." + ) + # intentionally not setting responded to True, since we want to attempt to dm the user + logger.warning( + f"Missing partial required permissions for {ctx.channel}. " + "I am able to send messages, but not embeds." + ) + else: + logger.error(f"Unable to send an error message to channel {ctx.channel}") + + if responded is not True and ANY_DEV_MODE: + # non-general permissions + perms = discord.Permissions( + administrator=True, + manage_threads=True, + manage_roles=True, + manage_channels=True, + ) + if perms.value & ctx.channel.permissions_for(ctx.author).value: + logger.info( + f"Attempting to dm {ctx.author} since they have a permission which may be able " + "to give the bot send message permissions." ) - if perms.value & ctx.channel.permissions_for(ctx.author).value: - logger.info( - f"Attempting to dm {ctx.author} since they have a permission which may be able " - "to give the bot send message permissions." - ) - try: - await ctx.author.send(embeds=[embed]) - except discord.Forbidden: - logger.notice("Also encountered an error when trying to reply in dms.") - return False + try: + await ctx.author.send(embeds=[embed]) + except discord.Forbidden: + logger.notice("Also encountered an error when trying to reply in dms.") + return False return True async def handle_check_failure( @@ -155,17 +170,19 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE embed = self.error_embed("Command Disabled", msg or str(error)) elif isinstance(error, commands.CommandInvokeError): - # generic error - logger.error(f'Error occurred in command "{ctx.command}".', exc_info=error.original) - # todo: this should log somewhere else since this is a bot bug. - embed = self.error_embed( - "Error Occurred", - "Oops! Something went wrong internally in the command you were trying to execute. " - "Please report this error and what you were trying to do to the developers.", - ) if isinstance(error.original, discord.Forbidden): + logger.warn(f"Permissions error occurred in {ctx.command}.") await self.handle_bot_missing_perms(ctx, error.original) should_respond = False + else: + # generic error + logger.error(f'Error occurred in command "{ctx.command}".', exc_info=error.original) + # todo: this should log somewhere else since this is a bot bug. + embed = self.error_embed( + "Error Occurred", + "Oops! Something went wrong internally in the command you were trying to execute. " + "Please report this error and what you were trying to do to the developers.", + ) # TODO: this has a fundamental problem with any BotMissingPermissions error # if the issue is the bot does not have permissions to send embeds or send messages... From 00751ca47ceaa4f83ff73f28b4775a38832be7e9 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 26 Sep 2021 01:48:22 -0400 Subject: [PATCH 12/14] minor: rename variable, remove unused or --- modmail/extensions/utils/error_handler.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index ef902192..31994ea4 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -72,10 +72,10 @@ async def handle_bot_missing_perms( """Handles bot missing permissing by dming the user if they have a permission which may be able to fix this.""" # noqa: E501 embed = self.error_embed("Permissions Failure", str(error)) bot_perms = ctx.channel.permissions_for(ctx.me) - responded = None + not_responded = True if bot_perms >= discord.Permissions(send_messages=True, embed_links=True): await ctx.send(embeds=[embed]) - responded = True + not_responded = False elif bot_perms >= discord.Permissions(send_messages=True): # make a message as similar to the embed, using as few permissions as possible # this is the only place we send a standard message instead of an embed, so no helper methods @@ -91,7 +91,7 @@ async def handle_bot_missing_perms( else: logger.error(f"Unable to send an error message to channel {ctx.channel}") - if responded is not True and ANY_DEV_MODE: + if not_responded and ANY_DEV_MODE: # non-general permissions perms = discord.Permissions( administrator=True, @@ -167,7 +167,7 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE msg = f"Command `{ctx.invoked_with}` is disabled." if reason := ctx.command.extras.get("disabled_reason", None): msg += f"\nReason: {reason}" - embed = self.error_embed("Command Disabled", msg or str(error)) + embed = self.error_embed("Command Disabled", msg) elif isinstance(error, commands.CommandInvokeError): if isinstance(error.original, discord.Forbidden): From 34e769692ad780498039b226bd1eeaab63985a6e Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 26 Sep 2021 01:50:29 -0400 Subject: [PATCH 13/14] fix: display different error for plugin specific errors --- modmail/extensions/utils/error_handler.py | 26 +++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index 31994ea4..f487e221 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -175,14 +175,28 @@ async def on_command_error(self, ctx: commands.Context, error: commands.CommandE await self.handle_bot_missing_perms(ctx, error.original) should_respond = False else: + # todo: this should properly handle plugin errors and note that they are not bot bugs + # todo: this should log somewhere else since this is a bot bug. # generic error logger.error(f'Error occurred in command "{ctx.command}".', exc_info=error.original) - # todo: this should log somewhere else since this is a bot bug. - embed = self.error_embed( - "Error Occurred", - "Oops! Something went wrong internally in the command you were trying to execute. " - "Please report this error and what you were trying to do to the developers.", - ) + if ctx.command.cog.__module__.startswith("modmail.plugins"): + # plugin msg + title = "Plugin Internal Error Occurred" + msg = ( + "Something went wrong internally in the plugin contributed command you were trying " + "to execute. Please report this error and what you were trying to do to the " + "respective plugin developers.\n\n**PLEASE NOTE**: Modmail developers will not help " + "you with this issue and will refer you to the plugin developers." + ) + else: + # built in command msg + title = "Internal Error" + msg = ( + "Something went wrong internally in the command you were trying to execute. " + "Please report this error and what you were trying to do to the bot developers." + ) + logger.debug(ctx.command.callback.__module__) + embed = self.error_embed(title, msg) # TODO: this has a fundamental problem with any BotMissingPermissions error # if the issue is the bot does not have permissions to send embeds or send messages... From 144b00875bc999e92ebdf30ba4a6a6a11ca90c8e Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 29 Sep 2021 03:00:43 -0400 Subject: [PATCH 14/14] chore: make requested comment changes --- modmail/extensions/utils/error_handler.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index f487e221..02b059f2 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -78,7 +78,8 @@ async def handle_bot_missing_perms( not_responded = False elif bot_perms >= discord.Permissions(send_messages=True): # make a message as similar to the embed, using as few permissions as possible - # this is the only place we send a standard message instead of an embed, so no helper methods + # this is the only place we send a standard message instead of an embed + # so no helper methods are necessary await ctx.send( "**Permissions Failure**\n\n" "I am missing the permissions required to properly execute your command." @@ -123,7 +124,9 @@ async def handle_check_failure( elif isinstance(error, commands.NoPrivateMessage): title = "Server Only" elif isinstance(error, commands.BotMissingPermissions): - # defer handling BotMissingPermissions to a method, since this can be problematic + # defer handling BotMissingPermissions to a method + # the error could be that the bot is unable to send messages, which would cause + # the error handling to fail await self.handle_bot_missing_perms(ctx, error) return None else: