Merge pull request #227 from kyb3r/better-errors

Better error messages

Author: kyb3r

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+# v2.18.2
+
+### Changed
+
+Commands now have better error messages, instead of just sending the help message for a command when an argument fails to be converted to its specified object, the bot now says things like "User 'bob' not found" instead.
+
 # v2.18.1
 
 Un-deprecated the `OWNERS` config variable to support discord developer team accounts.

bot.py

@@ -784,11 +784,31 @@ async def on_error(self, event_method, *args, **kwargs):
         logger.error(error('Ignoring exception in {}'.format(event_method)))
         logger.error(error('Unexpected exception:'), exc_info=sys.exc_info())
 
-    async def on_command_error(self, context, exception):
-        if isinstance(exception, (commands.MissingRequiredArgument,
-                                  commands.UserInputError)):
-            await context.invoke(self.get_command('help'),
-                                 command=str(context.command))
+    async def on_command_error(self, ctx, exception):
+
+        def human_join(strings):
+            if len(strings) <= 2:
+                return ' or '.join(strings)
+            else:
+                return ', '.join(strings[:len(strings)-1]) + ' or ' + strings[-1]
+
+        if isinstance(exception, commands.BadUnionArgument):
+            msg = 'Could not find the specified ' + human_join([c.__name__ for c in exception.converters])
+            await ctx.trigger_typing()
+            await ctx.send(embed=discord.Embed(
+                color=discord.Color.red(), 
+                description=msg
+                ))
+
+        elif isinstance(exception, commands.BadArgument):
+            await ctx.trigger_typing()
+            await ctx.send(embed=discord.Embed(
+                color=discord.Color.red(), 
+                description=str(exception)
+                ))
+        elif isinstance(exception, commands.MissingRequiredArgument):
+            await ctx.invoke(self.get_command('help'),
+                                 command=str(ctx.command))
         elif isinstance(exception, commands.CommandNotFound):
             logger.warning(error('CommandNotFound: ' + str(exception)))
         elif isinstance(exception, commands.CheckFailure):

cogs/modmail.py

@@ -2,6 +2,7 @@
 import re
 from datetime import datetime
 from typing import Optional, Union
+from types import SimpleNamespace as param
 
 import discord
 from discord.ext import commands
@@ -420,7 +421,7 @@ async def logs(self, ctx, *, member: User = None):
         if not member:
             thread = ctx.thread
             if not thread:
-                raise commands.UserInputError
+                raise commands.MissingRequiredArgument(param(name='member'))
             user = thread.recipient
         else:
             user = member
@@ -689,15 +690,17 @@ async def block(self, ctx, user: Optional[User] = None, *,
             thread = ctx.thread
             if thread:
                 user = thread.recipient
+            elif after is None:
+                raise commands.MissingRequiredArgument(param(name='user'))
             else:
-                raise commands.UserInputError
+                raise commands.BadArgument(f'User "{after.arg}" not found')
 
         if after is not None:
             reason = after.arg
             if reason.startswith('System Message: '):
-                raise commands.UserInputError
+                raise commands.BadArgument('The reason cannot start with `System Message:`.')
             if re.search(r'%(.+?)%$', reason) is not None:
-                raise commands.UserInputError
+                raise commands.MissingRequiredArgument(param(name='reason'))
             if after.dt > after.now:
                 reason = f'{reason} %{after.dt.isoformat()}%'
 
@@ -749,12 +752,13 @@ async def unblock(self, ctx, *, user: User = None):
         use only.
         """
 
+
         if user is None:
             thread = ctx.thread
             if thread:
                 user = thread.recipient
             else:
-                raise commands.UserInputError
+                raise commands.MissingRequiredArgument(param(name='user'))
 
         mention = user.mention if hasattr(user, 'mention') else f'`{user.id}`'
 
@@ -792,14 +796,20 @@ async def unblock(self, ctx, *, user: User = None):
 
     @commands.command()
     @checks.has_permissions(PermissionLevel.SUPPORTER)
-    async def delete(self, ctx, message_id: int = None):
+    @checks.thread_only()
+    async def delete(self, ctx, message_id = None):
         """Delete a message that was sent using the reply command.
 
         Deletes the previous message, unless a message ID is provided, which in that case,
         deletes the message with that message ID.
         """
         thread = ctx.thread
 
+        try:
+            message_id = int(message_id)
+        except ValueError:
+            raise commands.BadArgument('An integer message ID needs to be specified.')
+
         linked_message_id = await self.find_linked_message(ctx, message_id)
 
         if linked_message_id is None:

cogs/utility.py

@@ -130,7 +130,7 @@ async def format_not_found(self, ctx, command):
 
         choices = set()
 
-        for name, c in self.bot.all_commands:
+        for name, c in self.bot.all_commands.items():
             if not c.hidden:
                 choices.add(name)
 
@@ -909,7 +909,7 @@ async def add_perms_command(self, ctx, command: str, *, user_or_role: Union[User
         elif user_or_role in {'everyone', 'all'}:
             value = -1
         else:
-            raise commands.UserInputError('Invalid user or role.')
+            raise commands.BadArgument(f'User or Role "{user_or_role}" not found')
 
         await self.bot.update_perms(self.bot.all_commands[command].name, value)
         embed = Embed(
@@ -937,7 +937,7 @@ async def add_perms_level(self, ctx, level: str, *, user_or_role: Union[User, Ro
         elif user_or_role in {'everyone', 'all'}:
             value = -1
         else:
-            raise commands.UserInputError('Invalid user or role.')
+            raise commands.BadArgument(f'User or Role "{user_or_role}" not found')
 
         await self.bot.update_perms(PermissionLevel[level.upper()], value)
         embed = Embed(
@@ -973,7 +973,7 @@ async def remove_perms_command(self, ctx, command: str, *, user_or_role: Union[U
         elif user_or_role in {'everyone', 'all'}:
             value = -1
         else:
-            raise commands.UserInputError('Invalid user or role.')
+            raise commands.BadArgument(f'User or Role "{user_or_role}" not found')
 
         await self.bot.update_perms(self.bot.all_commands[command].name, value, add=False)
         embed = Embed(
@@ -1001,7 +1001,7 @@ async def remove_perms_level(self, ctx, level: str, *, user_or_role: Union[User,
         elif user_or_role in {'everyone', 'all'}:
             value = -1
         else:
-            raise commands.UserInputError('Invalid user or role.')
+            raise commands.BadArgument(f'User or Role "{user_or_role}" not found')
 
         await self.bot.update_perms(PermissionLevel[level.upper()], value, add=False)
         embed = Embed(
@@ -1021,7 +1021,7 @@ async def get_perms(self, ctx, *, user_or_role: Union[User, Role, str]):
         elif user_or_role in {'everyone', 'all'}:
             value = -1
         else:
-            raise commands.UserInputError('Invalid user or role.')
+            raise commands.BadArgument(f'User or Role "{user_or_role}" not found')
 
         cmds = []
         levels = []