Implemented generic command error handling

Also:
- Removed emotes from some of the error messages.
- Changed how emotes were placed into the leaderboard slightly
- Moved around TODO strings into proper files and added more.
- Corrected main.py cog loading references.
- Improved ContestBot.reject with message references and used built-in
delete_after keyword argument.
- Minor docstring/light formatting

bot/bot.py

@@ -9,7 +9,7 @@ from discord.ext import commands
 from sqlalchemy.engine import Engine
 from sqlalchemy.orm import Session, sessionmaker
 
-from bot import constants
+from bot import constants, helpers
 from bot.models import Guild, Period, Submission
 
 logger = logging.getLogger(__file__)
@@ -64,6 +64,8 @@ class ContestBot(commands.Bot):
                             f'Guild {guild.name} ({guild.id}) was not inside database on ready. Bot was disconnected or did not add it properly...')
                     session.add(Guild(id=guild.id))
 
+        # TODO: Scan all messages on start for current period and check for new periods/updated vote counts.
+
     async def on_guild_join(self, guild: discord.Guild) -> None:
         """Handles adding or reactivating a Guild in the database."""
         logger.info(f'Added to new guild: {guild.name} ({guild.id})')
@@ -121,10 +123,7 @@ class ContestBot(commands.Bot):
         return await channel.fetch_message(message_id)
 
     @staticmethod
-    async def reject(message: discord.Message, warning: str, delete_delay: int = 1, warning_duration: int = 5) -> None:
+    async def reject(message: discord.Message, warning: str, delete_delay: int = 2, warning_duration: int = 5) -> None:
         """Send a warning message and delete the message, then the warning"""
-        if delete_delay < 0:
+        if delete_delay > 0: await message.delete(delay=delete_delay)
-            await message.delete(delay=delete_delay)
+        await message.channel.send(embed=helpers.error_embed(message=warning), delete_after=warning_duration, reference=message)
-        warning = await message.channel.send(warning)
-        if warning_duration < 0:
-            await warning.delete(delay=warning_duration)

bot/cogs/contest_commands.py

@@ -1,4 +1,3 @@
-import datetime
 import logging
 
 import discord
@@ -13,6 +12,8 @@ logger = logging.getLogger(__file__)
 logger.setLevel(constants.LOGGING_LEVEL)
 
 
+# TODO: Add command error handling to all commands
+
 class ContestCommandsCog(commands.Cog, name='Contest'):
     """
     Commands related to creating, advancing, and querying contests.
@@ -21,6 +22,64 @@ class ContestCommandsCog(commands.Cog, name='Contest'):
     def __init__(self, bot: ContestBot) -> None:
         self.bot = bot
 
+    @commands.Cog.listener()
+    async def on_command_error(self, ctx: Context, error: discord.ext.commands.CommandError):
+        """
+        The event triggered when an error is raised while invoking a command.
+
+        Taken and slightly edited from https://gist.github.com/EvieePy/7822af90858ef65012ea500bcecf1612
+
+        :param error: The context used for command invocation.
+        :param ctx: The exception raised.
+        """
+
+        # This prevents any commands with local handlers being handled here in on_command_error.
+        if hasattr(ctx.command, 'on_error'):
+            return
+
+        # This prevents any cogs with an overwritten cog_command_error being handled here.
+        cog = ctx.cog
+        if cog and cog._get_overridden_method(cog.cog_command_error) is not None:
+            return
+
+        ignored = (commands.CommandNotFound,)
+
+        # Allows us to check for original exceptions raised and sent to CommandInvokeError.
+        # If nothing is found. We keep the exception passed to on_command_error.
+        error = getattr(error, 'original', error)
+
+        # Anything in ignored will return and prevent anything happening.
+        if isinstance(error, ignored):
+            return
+
+        if isinstance(error, commands.UserInputError):
+            message = ''
+            if isinstance(error, commands.BadArgument):
+                if isinstance(error, commands.ChannelNotFound):
+                    message = 'Invalid channel - I couldn\'t find that channel.'
+                elif isinstance(error, commands.RoleNotFound):
+                    message = 'Invalid role - I couldn\'t find that role'
+                elif isinstance(error, commands.ChannelNotReadable):
+                    message = 'Invalid channel - I couldn\'t read the contents of that channel.'
+                else:
+                    message = 'Invalid argument. Please check you entered everything correctly.'
+            if isinstance(error, commands.ArgumentParsingError):
+                message = 'I couldn\'t read the contents of your arguments properly. Check that you\'ve entered everything properly.'
+            if message:
+                await ctx.send(embed=helpers.error_embed(message=message))
+                return
+
+        if isinstance(error, commands.DisabledCommand):
+            await ctx.send(embed=helpers.error_embed(message=f'`{ctx.command}` has been disabled.'))
+        elif isinstance(error, commands.NoPrivateMessage):
+            try:
+                await ctx.send(embed=helpers.error_embed(message=f'`{ctx.command}` can not be used in Private Messages.'))
+            except discord.HTTPException:
+                pass
+        else:
+            # All other Errors not returned come here. And we can just print the default TraceBack.
+            logger.warning(f'Ignoring exception in command {ctx.command}', exc_info=error)
+
     @commands.command()
     @commands.guild_only()
     @checks.privileged()
@@ -69,10 +128,11 @@ class ContestCommandsCog(commands.Cog, name='Contest'):
         """
         Advance the state of the current period pertaining to this Guild.
 
-        :param ctx:
+        :param ctx: The context used for command invocation.
         :param duration: If given, the advance command will be repeated once more after the duration (in seconds) has passed.
         :param pingback: Whether or not the user should be pinged back when the duration is passed.
         """
+        # TODO: Implement pingback argument
         # TODO: Ensure that permissions for this command are being correctly tested for.
         if duration is not None: assert duration >= 0, "If specified, duration must be more than or equal to zero."
 
@@ -129,6 +189,12 @@ class ContestCommandsCog(commands.Cog, name='Contest'):
 
     @advance.error
     async def advance_error(self, error: errors.CommandError, ctx: Context) -> None:
+        """
+        `advance` command error handling.
+
+        :param error: The error raised while attempting to invoke the command
+        :param ctx: The context used for command invocation.
+        """
         if isinstance(error, errors.MissingPermissions):
             await ctx.send(embed=helpers.error_embed(
                     message='Check that the bot can actually modify roles, add reactions, see messages and send messages within this channel.'))
@@ -195,22 +261,18 @@ class ContestCommandsCog(commands.Cog, name='Contest'):
                     message = self.bot.get_message(guild.submission_channel, submission.id)
 
                     emote = ''
-                    if i == 1: emote = ':trophy: '
+                    if i == 1: emote = ':trophy:'
-                    elif i == 2: emote = ':second_place: '
+                    elif i == 2: emote = ':second_place:'
-                    elif i == 3: emote = ':third_place: '
+                    elif i == 3: emote = ':third_place:'
 
-                    description += f'`{str(i).zfill(2)}` {emote}<@{submission.user}> [Jump]({message.jump_url})\n'
+                    description += f'`{str(i).zfill(2)}` {emote + " " if emote else ""}<@{submission.user}> [Jump]({message.jump_url})\n'
 
                 if not description:
                     description = 'No one has submitted anything yet.'
 
-                embed = discord.Embed(title='Leaderboard',
+                embed = helpers.general_embed(title='Leaderboard', message=description, timestamp=True)
-                                      color=constants.GENERAL_COLOR,
-                                      description=description,
-                                      timestamp=datetime.datetime.utcnow())
                 embed.set_footer(text='Contest is still in progress...' if guild.current_period.active else 'Contest has finished.')
 
-                # embed.add_field(name="🤔", value="some of these properties have certain limits...", inline=True)
                 await ctx.send(embed=embed)
 
 

bot/cogs/contest_events.py

@@ -12,11 +12,7 @@ logger = logging.getLogger(__file__)
 logger.setLevel(constants.LOGGING_LEVEL)
 
 
-# TODO: Add command error handling to all commands
-# TODO: Use embeds in all bot responses
 # TODO: Look into migrating from literals to i18n-ish representation of all messages & formatting
-# TODO: Contest names
-# TODO: Refactor Period into Contest (major)
 
 
 class ContestEventsCog(commands.Cog):
@@ -38,15 +34,12 @@ class ContestEventsCog(commands.Cog):
 
                 # Ensure that the submission contains at least one attachment
                 if len(attachments) == 0:
-                    await self.bot.reject(message, f':no_entry_sign: {message.author.mention} '
+                    await self.bot.reject(message, f'Each submission must contain exactly one image.')
-                                                   f'Each submission must contain exactly one image.')
                 # Ensure the image contains no more than one attachment
                 elif len(attachments) > 1:
-                    await self.bot.reject(message, f':no_entry_sign: {message.author.mention} '
+                    await self.bot.reject(message, f'Each submission must contain exactly one image.')
-                                                   f'Each submission must contain exactly one image.')
                 elif guild.current_period is None:
-                    await self.bot.reject(message, f':no_entry_sign: {message.author.mention} A period has not been started. '
+                    await self.bot.reject(message, f'A period has not been started. Submissions should not be allowed at this moment.')
-                                                   f'Submissions should not be allowed at this moment.')
                 elif guild.current_period != PeriodStates.SUBMISSIONS:
                     logger.warning(f'Valid submission was sent outside of Submissions in'
                                    f' {channel.id}/{message.id}. Permissions error? Removing.')
@@ -55,9 +48,9 @@ class ContestEventsCog(commands.Cog):
                     attachment = attachments[0]
                     # TODO: Add helper for displaying error/warning messages
                     if attachment.is_spoiler():
-                        await self.bot.reject(message, ':no_entry_sign: Attachment must not make use of a spoiler.')
+                        await self.bot.reject(message, 'Attachment must not make use of a spoiler.')
                     elif attachment.width is None:
-                        await self.bot.reject(message, ':no_entry_sign: Attachment must be a image or video.')
+                        await self.bot.reject(message, 'Attachment must be a image or video.')
                     else:
                         last_submission: Submission = session.query(Submission).filter_by(period=guild.current_period,
                                                                                           user=message.author.id).first()
@@ -157,6 +150,8 @@ class ContestEventsCog(commands.Cog):
         except ValueError:
             pass
 
+        # TODO: Only update the votecount during the VOTING period!
+
         with self.bot.get_session() as session:
             guild: Guild = session.query(Guild).get(payload.guild_id)
             if payload.channel_id == guild.submission_channel and helpers.is_upvote(payload.emoji):

bot/models.py

@@ -23,9 +23,8 @@ logger.setLevel(constants.LOGGING_LEVEL)
 
 Base = declarative_base()
 
-
+# TODO: Contest names
-# TODO: Setup and test basic automatic migration.
+# TODO: Refactor Period into Contest (major)
-
 
 class PeriodStates(enum.Enum):
     """

main.py

@@ -25,7 +25,8 @@ if __name__ == "__main__":
                             logging.StreamHandler()
                         ])
 
-    initial_extensions = ['bot.cogs.contest']
+    initial_extensions = ['bot.cogs.contest_commands',
+                          'bot.cogs.contest_events']
 
     engine = load_db()
     bot = ContestBot(engine, description='A assistant for the Photography Lounge\'s monday contests')