From 02c5c7e2b120c055b7db07d2ca9568d1ae25c399 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Wed, 2 Dec 2020 08:46:26 +0100 Subject: Fix leaderboard glitch caused by duplicate names We noticed that some entries on our leaderboard had an incorrect star count attached to their name. After a bit of digging, @HassanAbouelela discovered that this was caused by the use of the member's name as the key for the leaderboard dictionary: If different accounts used the same display name for the leaderboard, they'd be combined into one glitched score dict. The fix @HassanAbouelela wrote is to use the member id instead of the name as the key for the leaderboard. I've changed a few names here and there, but nothing major. This commit closes #536 --- bot/exts/christmas/advent_of_code/_helpers.py | 30 +++++++++++++++------------ 1 file changed, 17 insertions(+), 13 deletions(-) (limited to 'bot') diff --git a/bot/exts/christmas/advent_of_code/_helpers.py b/bot/exts/christmas/advent_of_code/_helpers.py index f4a20955..e7eeedb2 100644 --- a/bot/exts/christmas/advent_of_code/_helpers.py +++ b/bot/exts/christmas/advent_of_code/_helpers.py @@ -50,7 +50,7 @@ EST = pytz.timezone("EST") # Create namedtuple that combines a participant's name and their completion # time for a specific star. We're going to use this later to order the results # for each star to compute the rank score. -_StarResult = collections.namedtuple("StarResult", "name completion_time") +StarResult = collections.namedtuple("StarResult", "member_id completion_time") def leaderboard_sorting_function(entry: typing.Tuple[str, dict]) -> typing.Tuple[int, int]: @@ -61,7 +61,7 @@ def leaderboard_sorting_function(entry: typing.Tuple[str, dict]) -> typing.Tuple secondary on the number of stars someone has completed. """ result = entry[1] - return result["score"], result["star_2_count"] + result["star_1_count"] + return result["score"], result["star_2"] + result["star_1"] def _parse_raw_leaderboard_data(raw_leaderboard_data: dict) -> dict: @@ -90,30 +90,34 @@ def _parse_raw_leaderboard_data(raw_leaderboard_data: dict) -> dict: # star view. We need that per star view to compute rank scores per star. for member in raw_leaderboard_data.values(): name = member["name"] if member["name"] else f"Anonymous #{member['id']}" - leaderboard[name] = {"score": 0, "star_1_count": 0, "star_2_count": 0} + member_id = member['id'] + leaderboard[member_id] = {"name": name, "score": 0, "star_1": 0, "star_2": 0} # Iterate over all days for this participant for day, stars in member["completion_day_level"].items(): # Iterate over the complete stars for this day for this participant for star, data in stars.items(): # Record completion of this star for this individual - leaderboard[name][f"star_{star}_count"] += 1 + leaderboard[member_id][f"star_{star}"] += 1 # Record completion datetime for this participant for this day/star completion_time = datetime.datetime.fromtimestamp(int(data['get_star_ts'])) star_results[(day, star)].append( - _StarResult(name=name, completion_time=completion_time) + StarResult(member_id=member_id, completion_time=completion_time) ) # Now that we have a transposed dataset that holds the completion time of all # participants per star, we can compute the rank-based scores each participant # should get for that star. max_score = len(leaderboard) - for(day, _star), results in star_results.items(): + for (day, _star), results in star_results.items(): + # If this day should not count in the ranking, skip it. if day in AdventOfCode.ignored_days: continue - for rank, star_result in enumerate(sorted(results, key=operator.itemgetter(1))): - leaderboard[star_result.name]["score"] += max_score - rank + + sorted_result = sorted(results, key=operator.attrgetter('completion_time')) + for rank, star_result in enumerate(sorted_result): + leaderboard[star_result.member_id]["score"] += max_score - rank # Since dictionaries now retain insertion order, let's use that sorted_leaderboard = dict( @@ -133,16 +137,16 @@ def _parse_raw_leaderboard_data(raw_leaderboard_data: dict) -> dict: return {"daily_stats": daily_stats, "leaderboard": sorted_leaderboard} -def _format_leaderboard(leaderboard: typing.Dict[str, int]) -> str: +def _format_leaderboard(leaderboard: typing.Dict[str, dict]) -> str: """Format the leaderboard using the AOC_TABLE_TEMPLATE.""" leaderboard_lines = [HEADER] - for rank, (name, results) in enumerate(leaderboard.items(), start=1): + for rank, data in enumerate(leaderboard.values(), start=1): leaderboard_lines.append( AOC_TABLE_TEMPLATE.format( rank=rank, - name=name, - score=str(results["score"]), - stars=f"({results['star_1_count']}, {results['star_2_count']})" + name=data["name"], + score=str(data["score"]), + stars=f"({data['star_1']}, {data['star_2']})" ) ) -- cgit v1.2.3 From 4d2f89991432639431968d352bde2cd747b9432c Mon Sep 17 00:00:00 2001 From: Rohan Date: Wed, 2 Dec 2020 21:58:09 +0530 Subject: Modify error handler check for locally handled errors. Error handler now checks if the error has the attribute "handled" for locally handled errors. --- bot/exts/evergreen/error_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'bot') diff --git a/bot/exts/evergreen/error_handler.py b/bot/exts/evergreen/error_handler.py index 6e518435..b502dd4e 100644 --- a/bot/exts/evergreen/error_handler.py +++ b/bot/exts/evergreen/error_handler.py @@ -42,8 +42,8 @@ class CommandErrorHandler(commands.Cog): @commands.Cog.listener() async def on_command_error(self, ctx: commands.Context, error: commands.CommandError) -> None: """Activates when a command opens an error.""" - if hasattr(ctx.command, 'on_error'): - logging.debug("A command error occured but the command had it's own error handler.") + if hasattr(error, 'handled'): + logging.debug(f"Command {ctx.command} had its error already handled locally; ignoring.") return error = getattr(error, 'original', error) -- cgit v1.2.3 From cd24f9b4e70ebe654fa771238fca7e207e8cb0de Mon Sep 17 00:00:00 2001 From: Rohan Date: Fri, 4 Dec 2020 20:33:03 +0530 Subject: Check value of handled attribute on error in global error handler. --- bot/exts/evergreen/error_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bot') diff --git a/bot/exts/evergreen/error_handler.py b/bot/exts/evergreen/error_handler.py index b502dd4e..99af1519 100644 --- a/bot/exts/evergreen/error_handler.py +++ b/bot/exts/evergreen/error_handler.py @@ -42,7 +42,7 @@ class CommandErrorHandler(commands.Cog): @commands.Cog.listener() async def on_command_error(self, ctx: commands.Context, error: commands.CommandError) -> None: """Activates when a command opens an error.""" - if hasattr(error, 'handled'): + if getattr(error, 'handled', False): logging.debug(f"Command {ctx.command} had its error already handled locally; ignoring.") return -- cgit v1.2.3 From 1aad543a8d4b3c7317fe98937144591d85ad3330 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Sat, 5 Dec 2020 00:21:04 +0100 Subject: Support a fallback session cookie in constants To mitigate problems due to expiring session cookies, I'm currently in the process of adding support for a fallback cookie. Basically, my Advent of Code account is a member of *all* leaderboards, which means that my cookie can be used to fetch all leaderboards as well. As my session cookie should not expire until after the event, it should not give us any issues. However, to avoid issues with issuing too many requests from one session, we should still make sure to set individual session values regardless of the mitigation. --- bot/constants.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'bot') diff --git a/bot/constants.py b/bot/constants.py index e313e086..9e6db7a6 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -1,3 +1,4 @@ +import dataclasses import enum import logging from datetime import datetime @@ -29,11 +30,27 @@ __all__ = ( log = logging.getLogger(__name__) -class AdventOfCodeLeaderboard(NamedTuple): +@dataclasses.dataclass +class AdventOfCodeLeaderboard: id: str - session: str + _session: str join_code: str + # If we notice that the session for this board expired, we set + # this attribute to `True`. We will emit a Sentry error so we + # can handle it, but, in the meantime, we'll try using the + # fallback session to make sure the commands still work. + use_fallback_session: bool = False + + @property + def session(self) -> str: + """Return either the actual `session` cookie or the fallback cookie.""" + if self.use_fallback_session: + log.info(f"Returning fallback cookie for board `{self.id}`.") + return AdventOfCode.fallback_session + + return self._session + def _parse_aoc_leaderboard_env() -> Dict[str, AdventOfCodeLeaderboard]: """ @@ -61,6 +78,7 @@ class AdventOfCode: # Information for the several leaderboards we have leaderboards = _parse_aoc_leaderboard_env() staff_leaderboard_id = environ.get("AOC_STAFF_LEADERBOARD_ID", "") + fallback_session = environ.get("AOC_FALLBACK_SESSION", "") # Other Advent of Code constants ignored_days = [day for day in environ.get("AOC_IGNORED_DAYS", "").split(",")] -- cgit v1.2.3 From c21014abc7c74a72363daeac3714b9fa976247ce Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Sat, 5 Dec 2020 00:39:09 +0100 Subject: Mitigate session expiry by using fallback session Unfortunately, an expired session cookie wreaked havoc to our Advent of Code commands: All commands that relied on leaderboard data failed because we couldn't refresh our data and the cache had expired. To mitigate an expired session, I've added a fallback session feature that enables us to try again with a different session. While it will issue an error message to inform us to refresh the expired session cookie, it does mean that the functionality should continue to work in the mean time. The fallback session cookie is currently set to my session cookie, using an environment variable, `AOC_FALLBACK_SESSION`. It is important that the user connected to the session is a member of all boards and that it's a fresh session: We don't want our fallback to expire! At the same time, while a single fallback session works, the AoC website also does not like too many requests from a single user. That's why we'll still use a multi-session model under normal circumstances. To check for expired sessions, I've added a URL check: The Advent of Code website will silently redirect people with an expired session, issuing an 200: OK status as usual. The only way to really check for it is by comparing the final URL in the response object to the URL we set out to GET. I've added a custom exception to signal such an unexpected redirect. Finally, instead of having the commands just break, I've added an Exception signal that propagates back to the caller. The solution, with try-except, is a bit hacky and could benefit from an actual error handler, but I wanted to get things fixed first; polish can be added later. --- bot/exts/christmas/advent_of_code/_cog.py | 27 ++++++++--- bot/exts/christmas/advent_of_code/_helpers.py | 65 ++++++++++++++++++++++----- 2 files changed, 75 insertions(+), 17 deletions(-) (limited to 'bot') diff --git a/bot/exts/christmas/advent_of_code/_cog.py b/bot/exts/christmas/advent_of_code/_cog.py index 2a1a776b..0bcd9f42 100644 --- a/bot/exts/christmas/advent_of_code/_cog.py +++ b/bot/exts/christmas/advent_of_code/_cog.py @@ -221,7 +221,11 @@ class AdventOfCode(commands.Cog): if AocConfig.staff_leaderboard_id and any(r.id == Roles.helpers for r in author.roles): join_code = AocConfig.leaderboards[AocConfig.staff_leaderboard_id].join_code else: - join_code = await _helpers.get_public_join_code(author) + try: + join_code = await _helpers.get_public_join_code(author) + except _helpers.FetchingLeaderboardFailed: + await ctx.send(":x: Failed to get join code! Notified maintainers.") + return if not join_code: log.error(f"Failed to get a join code for user {author} ({author.id})") @@ -256,7 +260,12 @@ class AdventOfCode(commands.Cog): async def aoc_leaderboard(self, ctx: commands.Context) -> None: """Get the current top scorers of the Python Discord Leaderboard.""" async with ctx.typing(): - leaderboard = await _helpers.fetch_leaderboard() + try: + leaderboard = await _helpers.fetch_leaderboard() + except _helpers.FetchingLeaderboardFailed: + await ctx.send(":x: Unable to fetch leaderboard!") + return + number_of_participants = leaderboard["number_of_participants"] top_count = min(AocConfig.leaderboard_displayed_members, number_of_participants) @@ -291,7 +300,11 @@ class AdventOfCode(commands.Cog): @override_in_channel(AOC_WHITELIST) async def private_leaderboard_daily_stats(self, ctx: commands.Context) -> None: """Send an embed with daily completion statistics for the Python Discord leaderboard.""" - leaderboard = await _helpers.fetch_leaderboard() + try: + leaderboard = await _helpers.fetch_leaderboard() + except _helpers.FetchingLeaderboardFailed: + await ctx.send(":x: Can't fetch leaderboard for stats right now!") + return # The daily stats are serialized as JSON as they have to be cached in Redis daily_stats = json.loads(leaderboard["daily_stats"]) @@ -323,8 +336,12 @@ class AdventOfCode(commands.Cog): many requests to the Advent of Code server. """ async with ctx.typing(): - await _helpers.fetch_leaderboard(invalidate_cache=True) - await ctx.send("\N{OK Hand Sign} Refreshed leaderboard cache!") + try: + await _helpers.fetch_leaderboard(invalidate_cache=True) + except _helpers.FetchingLeaderboardFailed: + await ctx.send(":x: Something went wrong while trying to refresh the cache!") + else: + await ctx.send("\N{OK Hand Sign} Refreshed leaderboard cache!") def cog_unload(self) -> None: """Cancel season-related tasks on cog unload.""" diff --git a/bot/exts/christmas/advent_of_code/_helpers.py b/bot/exts/christmas/advent_of_code/_helpers.py index e7eeedb2..d883c09f 100644 --- a/bot/exts/christmas/advent_of_code/_helpers.py +++ b/bot/exts/christmas/advent_of_code/_helpers.py @@ -53,6 +53,18 @@ EST = pytz.timezone("EST") StarResult = collections.namedtuple("StarResult", "member_id completion_time") +class UnexpectedRedirect(aiohttp.ClientError): + """Raised when an unexpected redirect was detected.""" + + +class UnexpectedResponseStatus(aiohttp.ClientError): + """Raised when an unexpected redirect was detected.""" + + +class FetchingLeaderboardFailed(Exception): + """Raised when one or more leaderboards could not be fetched at all.""" + + def leaderboard_sorting_function(entry: typing.Tuple[str, dict]) -> typing.Tuple[int, int]: """ Provide a sorting value for our leaderboard. @@ -153,6 +165,23 @@ def _format_leaderboard(leaderboard: typing.Dict[str, dict]) -> str: return "\n".join(leaderboard_lines) +async def _leaderboard_request(url: str, board: int, cookies: dict) -> typing.Optional[dict]: + """Make a leaderboard request using the specified session cookie.""" + async with aiohttp.request("GET", url, headers=AOC_REQUEST_HEADER, cookies=cookies) as resp: + # The Advent of Code website redirects silently with a 200 response if a + # session cookie has expired, is invalid, or was not provided. + if str(resp.url) != url: + log.error(f"Fetching leaderboard `{board}` failed! Check the session cookie.") + raise UnexpectedRedirect(f"redirected unexpectedly to {resp.url} for board `{board}`") + + # Every status other than `200` is unexpected, not only 400+ + if not resp.status == 200: + log.error(f"Unexpected response `{resp.status}` while fetching leaderboard `{board}`") + raise UnexpectedResponseStatus(f"status `{resp.status}`") + + return await resp.json() + + async def _fetch_leaderboard_data() -> typing.Dict[str, typing.Any]: """Fetch data for all leaderboards and return a pooled result.""" year = AdventOfCode.year @@ -165,22 +194,34 @@ async def _fetch_leaderboard_data() -> typing.Dict[str, typing.Any]: participants = {} for leaderboard in AdventOfCode.leaderboards.values(): leaderboard_url = AOC_API_URL.format(year=year, leaderboard_id=leaderboard.id) - cookies = {"session": leaderboard.session} - # We don't need to create a session if we're going to throw it away after each request - async with aiohttp.request( - "GET", leaderboard_url, headers=AOC_REQUEST_HEADER, cookies=cookies - ) as resp: - if resp.status == 200: - raw_data = await resp.json() - - # Get the participants and store their current count + # Two attempts, one with the original session cookie and one with the fallback session + for attempt in range(1, 3): + log.info(f"Attempting to fetch leaderboard `{leaderboard.id}` ({attempt}/2)") + cookies = {"session": leaderboard.session} + try: + raw_data = await _leaderboard_request(leaderboard_url, leaderboard.id, cookies) + except UnexpectedRedirect: + if cookies["session"] == AdventOfCode.fallback_session: + log.error("It seems like the fallback cookie has expired!") + raise FetchingLeaderboardFailed from None + + # If we're here, it means that the original session did not + # work. Let's fall back to the fallback session. + leaderboard.use_fallback_session = True + continue + except aiohttp.ClientError: + # Don't retry, something unexpected is wrong and it may not be the session. + raise FetchingLeaderboardFailed from None + else: + # Get the participants and store their current count. board_participants = raw_data["members"] await _caches.leaderboard_counts.set(leaderboard.id, len(board_participants)) participants.update(board_participants) - else: - log.warning(f"Fetching data failed for leaderboard `{leaderboard.id}`") - resp.raise_for_status() + break + else: + log.error(f"reached 'unreachable' state while fetching board `{leaderboard.id}`.") + raise FetchingLeaderboardFailed log.info(f"Fetched leaderboard information for {len(participants)} participants") return participants -- cgit v1.2.3 From cd20acfee1e36351e561ba7c410dc8fac7725572 Mon Sep 17 00:00:00 2001 From: Rohan Date: Mon, 7 Dec 2020 01:16:29 +0530 Subject: Modify snakes_cog error handler. local error handler no longer checks for BadArgument error and the attribute handled will be set to True on the error if an OSError occurs. --- bot/exts/evergreen/snakes/_snakes_cog.py | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) (limited to 'bot') diff --git a/bot/exts/evergreen/snakes/_snakes_cog.py b/bot/exts/evergreen/snakes/_snakes_cog.py index 70bb0e73..2e88c146 100644 --- a/bot/exts/evergreen/snakes/_snakes_cog.py +++ b/bot/exts/evergreen/snakes/_snakes_cog.py @@ -15,7 +15,7 @@ import aiohttp import async_timeout from PIL import Image, ImageDraw, ImageFont from discord import Colour, Embed, File, Member, Message, Reaction -from discord.ext.commands import BadArgument, Bot, Cog, CommandError, Context, bot_has_permissions, group +from discord.ext.commands import Bot, Cog, CommandError, Context, bot_has_permissions, group from bot.constants import ERROR_REPLIES, Tokens from bot.exts.evergreen.snakes import _utils as utils @@ -1131,21 +1131,11 @@ class Snakes(Cog): @video_command.error async def command_error(self, ctx: Context, error: CommandError) -> None: """Local error handler for the Snake Cog.""" - embed = Embed() - embed.colour = Colour.red() - - if isinstance(error, BadArgument): - embed.description = str(error) - embed.title = random.choice(ERROR_REPLIES) - - elif isinstance(error, OSError): + if isinstance(error, OSError): + error.handled = True + embed = Embed() + embed.colour = Colour.red() log.error(f"snake_card encountered an OSError: {error} ({error.original})") embed.description = "Could not generate the snake card! Please try again." embed.title = random.choice(ERROR_REPLIES) - - else: - log.error(f"Unhandled tag command error: {error} ({error.original})") - return - - await ctx.send(embed=embed) - # endregion + await ctx.send(embed=embed) -- cgit v1.2.3 From 40fce53d6c01f0e753d70cb4b265d88f1107e104 Mon Sep 17 00:00:00 2001 From: Rohan Date: Wed, 9 Dec 2020 23:13:39 +0530 Subject: Check if error.original is an instance of OSError. Also, remove error handler for get_command and video_command. --- bot/exts/evergreen/snakes/_snakes_cog.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'bot') diff --git a/bot/exts/evergreen/snakes/_snakes_cog.py b/bot/exts/evergreen/snakes/_snakes_cog.py index 2e88c146..4fa4dcd1 100644 --- a/bot/exts/evergreen/snakes/_snakes_cog.py +++ b/bot/exts/evergreen/snakes/_snakes_cog.py @@ -1126,16 +1126,15 @@ class Snakes(Cog): # endregion # region: Error handlers - @get_command.error @card_command.error - @video_command.error async def command_error(self, ctx: Context, error: CommandError) -> None: """Local error handler for the Snake Cog.""" - if isinstance(error, OSError): + original_error = getattr(error, "original", None) + if isinstance(original_error, OSError): error.handled = True embed = Embed() embed.colour = Colour.red() - log.error(f"snake_card encountered an OSError: {error} ({error.original})") + log.error(f"snake_card encountered an OSError: {error} ({original_error})") embed.description = "Could not generate the snake card! Please try again." embed.title = random.choice(ERROR_REPLIES) await ctx.send(embed=embed) -- cgit v1.2.3 From d884c21e8ae2bd82370b11621230d7172cb098a3 Mon Sep 17 00:00:00 2001 From: janine9vn Date: Wed, 2 Dec 2020 09:28:32 -0500 Subject: Disallow .aoc commands in primary aoc channel Commands like `.aoc leaderboard` and `.aoc stats` proved to be spammy in the main advent of code channel. An aoc_commands channel has been added for aoc commands and this update prohibits aoc commands from being used in the primary aoc channel and adds the comands channel to the whitelist. This also specifically allows the less spammier commands: join, subscribe, unsubscribe, and countdown in the primary channel to foster discussion though. --- bot/constants.py | 1 + bot/exts/christmas/advent_of_code/_cog.py | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) (limited to 'bot') diff --git a/bot/constants.py b/bot/constants.py index 9e6db7a6..5dc42462 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -95,6 +95,7 @@ class Branding: class Channels(NamedTuple): admins = 365960823622991872 advent_of_code = int(environ.get("AOC_CHANNEL_ID", 782715290437943306)) + advent_of_code_commands = int(environ.get("AOC_COMMANDS_CHANNEL_ID", 783503267849437205)) announcements = int(environ.get("CHANNEL_ANNOUNCEMENTS", 354619224620138496)) big_brother_logs = 468507907357409333 bot = 267659945086812160 diff --git a/bot/exts/christmas/advent_of_code/_cog.py b/bot/exts/christmas/advent_of_code/_cog.py index 0bcd9f42..fad13f23 100644 --- a/bot/exts/christmas/advent_of_code/_cog.py +++ b/bot/exts/christmas/advent_of_code/_cog.py @@ -21,7 +21,7 @@ AOC_REQUEST_HEADER = {"user-agent": "PythonDiscord AoC Event Bot"} COUNTDOWN_STEP = 60 * 5 -AOC_WHITELIST = WHITELISTED_CHANNELS + (Channels.advent_of_code,) +AOC_WHITELIST = WHITELISTED_CHANNELS + (Channels.advent_of_code_commands,) async def countdown_status(bot: commands.Bot) -> None: @@ -139,7 +139,7 @@ class AdventOfCode(commands.Cog): aliases=("sub", "notifications", "notify", "notifs"), brief="Notifications for new days" ) - @override_in_channel(AOC_WHITELIST) + @override_in_channel(AOC_WHITELIST + (Channels.advent_of_code)) async def aoc_subscribe(self, ctx: commands.Context) -> None: """Assign the role for notifications about new days being ready.""" current_year = datetime.now().year @@ -160,7 +160,7 @@ class AdventOfCode(commands.Cog): @in_month(Month.DECEMBER) @adventofcode_group.command(name="unsubscribe", aliases=("unsub",), brief="Notifications for new days") - @override_in_channel(AOC_WHITELIST) + @override_in_channel(AOC_WHITELIST + Channels.advent_of_code) async def aoc_unsubscribe(self, ctx: commands.Context) -> None: """Remove the role for notifications about new days being ready.""" role = ctx.guild.get_role(AocConfig.role_id) @@ -172,7 +172,7 @@ class AdventOfCode(commands.Cog): await ctx.send("Hey, you don't even get any notifications about new Advent of Code tasks currently anyway.") @adventofcode_group.command(name="countdown", aliases=("count", "c"), brief="Return time left until next day") - @override_in_channel(AOC_WHITELIST) + @override_in_channel(AOC_WHITELIST + (Channels.advent_of_code)) async def aoc_countdown(self, ctx: commands.Context) -> None: """Return time left until next day.""" if not _helpers.is_in_advent(): @@ -207,7 +207,7 @@ class AdventOfCode(commands.Cog): await ctx.send("", embed=self.cached_about_aoc) @adventofcode_group.command(name="join", aliases=("j",), brief="Learn how to join the leaderboard (via DM)") - @override_in_channel(AOC_WHITELIST) + @override_in_channel(AOC_WHITELIST + (Channels.advent_of_code)) async def join_leaderboard(self, ctx: commands.Context) -> None: """DM the user the information for joining the Python Discord leaderboard.""" current_year = datetime.now().year -- cgit v1.2.3 From d222ee24ec4d90c4eacbe50348fb1e9b6f1a0fed Mon Sep 17 00:00:00 2001 From: janine9vn Date: Wed, 2 Dec 2020 11:15:03 -0500 Subject: Add cog-level error handler for Incorrect Channel If any of the "spammier" commands (stats, leaderboard) are used within the primary advent of code channel, rather than a non-specific embed we instead reply with the channel they should be using. This also adds a "AOC_WHITELIST_PLUS" constant that makes it easier to adjust what channels the non-spammier aoc commands can be used in. --- bot/exts/christmas/advent_of_code/_cog.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) (limited to 'bot') diff --git a/bot/exts/christmas/advent_of_code/_cog.py b/bot/exts/christmas/advent_of_code/_cog.py index fad13f23..30030e70 100644 --- a/bot/exts/christmas/advent_of_code/_cog.py +++ b/bot/exts/christmas/advent_of_code/_cog.py @@ -13,7 +13,7 @@ from bot.constants import ( AdventOfCode as AocConfig, Channels, Colours, Emojis, Month, Roles, WHITELISTED_CHANNELS, ) from bot.exts.christmas.advent_of_code import _helpers -from bot.utils.decorators import in_month, override_in_channel, with_role +from bot.utils.decorators import InChannelCheckFailure, in_month, override_in_channel, with_role log = logging.getLogger(__name__) @@ -23,6 +23,10 @@ COUNTDOWN_STEP = 60 * 5 AOC_WHITELIST = WHITELISTED_CHANNELS + (Channels.advent_of_code_commands,) +# Some commands can be run in the regular advent of code channel +# They aren't spammy and foster discussion +AOC_WHITELIST_PLUS = AOC_WHITELIST + (Channels.advent_of_code,) + async def countdown_status(bot: commands.Bot) -> None: """Set the playing status of the bot to the minutes & hours left until the next day's challenge.""" @@ -128,7 +132,7 @@ class AdventOfCode(commands.Cog): self.status_task = self.bot.loop.create_task(status_coro) @commands.group(name="adventofcode", aliases=("aoc",)) - @override_in_channel(AOC_WHITELIST) + @override_in_channel(AOC_WHITELIST_PLUS) async def adventofcode_group(self, ctx: commands.Context) -> None: """All of the Advent of Code commands.""" if not ctx.invoked_subcommand: @@ -139,7 +143,7 @@ class AdventOfCode(commands.Cog): aliases=("sub", "notifications", "notify", "notifs"), brief="Notifications for new days" ) - @override_in_channel(AOC_WHITELIST + (Channels.advent_of_code)) + @override_in_channel(AOC_WHITELIST_PLUS) async def aoc_subscribe(self, ctx: commands.Context) -> None: """Assign the role for notifications about new days being ready.""" current_year = datetime.now().year @@ -160,7 +164,7 @@ class AdventOfCode(commands.Cog): @in_month(Month.DECEMBER) @adventofcode_group.command(name="unsubscribe", aliases=("unsub",), brief="Notifications for new days") - @override_in_channel(AOC_WHITELIST + Channels.advent_of_code) + @override_in_channel(AOC_WHITELIST_PLUS) async def aoc_unsubscribe(self, ctx: commands.Context) -> None: """Remove the role for notifications about new days being ready.""" role = ctx.guild.get_role(AocConfig.role_id) @@ -172,7 +176,7 @@ class AdventOfCode(commands.Cog): await ctx.send("Hey, you don't even get any notifications about new Advent of Code tasks currently anyway.") @adventofcode_group.command(name="countdown", aliases=("count", "c"), brief="Return time left until next day") - @override_in_channel(AOC_WHITELIST + (Channels.advent_of_code)) + @override_in_channel(AOC_WHITELIST_PLUS) async def aoc_countdown(self, ctx: commands.Context) -> None: """Return time left until next day.""" if not _helpers.is_in_advent(): @@ -207,7 +211,7 @@ class AdventOfCode(commands.Cog): await ctx.send("", embed=self.cached_about_aoc) @adventofcode_group.command(name="join", aliases=("j",), brief="Learn how to join the leaderboard (via DM)") - @override_in_channel(AOC_WHITELIST + (Channels.advent_of_code)) + @override_in_channel(AOC_WHITELIST_PLUS) async def join_leaderboard(self, ctx: commands.Context) -> None: """DM the user the information for joining the Python Discord leaderboard.""" current_year = datetime.now().year @@ -366,3 +370,11 @@ class AdventOfCode(commands.Cog): about_embed.set_footer(text="Last Updated") return about_embed + + async def cog_command_error(self, ctx: commands.Context, error: Exception) -> None: + """Custom error handler if an advent of code command was posted in the wrong channel.""" + if isinstance(error, InChannelCheckFailure): + await ctx.send(f":x: Please use <#{Channels.advent_of_code_commands}> for aoc commands instead, please.") + ctx.command.on_error = True + else: + raise error -- cgit v1.2.3 From 15a511d5895c5c1f9ec68b968fa4e0ff55a416a3 Mon Sep 17 00:00:00 2001 From: janine9vn Date: Wed, 2 Dec 2020 11:21:27 -0500 Subject: Change custom error handler to match new style I'm a bit ahead of the game and changing the error handler to match the new style that Iceman will PR shortly. --- bot/exts/christmas/advent_of_code/_cog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bot') diff --git a/bot/exts/christmas/advent_of_code/_cog.py b/bot/exts/christmas/advent_of_code/_cog.py index 30030e70..0ad718b9 100644 --- a/bot/exts/christmas/advent_of_code/_cog.py +++ b/bot/exts/christmas/advent_of_code/_cog.py @@ -375,6 +375,6 @@ class AdventOfCode(commands.Cog): """Custom error handler if an advent of code command was posted in the wrong channel.""" if isinstance(error, InChannelCheckFailure): await ctx.send(f":x: Please use <#{Channels.advent_of_code_commands}> for aoc commands instead, please.") - ctx.command.on_error = True + error.handled = True else: raise error -- cgit v1.2.3 From b6fa6385c82e216d9ef5d2cefe1dd1efc470008f Mon Sep 17 00:00:00 2001 From: janine9vn Date: Fri, 11 Dec 2020 15:03:57 -0500 Subject: Remove re-raising the error Per Mark's comment, re-raising the error isn't necessary. --- bot/exts/christmas/advent_of_code/_cog.py | 2 -- 1 file changed, 2 deletions(-) (limited to 'bot') diff --git a/bot/exts/christmas/advent_of_code/_cog.py b/bot/exts/christmas/advent_of_code/_cog.py index 0ad718b9..b6462ab2 100644 --- a/bot/exts/christmas/advent_of_code/_cog.py +++ b/bot/exts/christmas/advent_of_code/_cog.py @@ -376,5 +376,3 @@ class AdventOfCode(commands.Cog): if isinstance(error, InChannelCheckFailure): await ctx.send(f":x: Please use <#{Channels.advent_of_code_commands}> for aoc commands instead, please.") error.handled = True - else: - raise error -- cgit v1.2.3 From f493699a4a076bebcd69404be57c3a32da5a22a1 Mon Sep 17 00:00:00 2001 From: janine9vn Date: Fri, 11 Dec 2020 17:11:41 -0500 Subject: Remove extra please Please -= 1 --- bot/exts/christmas/advent_of_code/_cog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bot') diff --git a/bot/exts/christmas/advent_of_code/_cog.py b/bot/exts/christmas/advent_of_code/_cog.py index b6462ab2..90d92fc1 100644 --- a/bot/exts/christmas/advent_of_code/_cog.py +++ b/bot/exts/christmas/advent_of_code/_cog.py @@ -374,5 +374,5 @@ class AdventOfCode(commands.Cog): async def cog_command_error(self, ctx: commands.Context, error: Exception) -> None: """Custom error handler if an advent of code command was posted in the wrong channel.""" if isinstance(error, InChannelCheckFailure): - await ctx.send(f":x: Please use <#{Channels.advent_of_code_commands}> for aoc commands instead, please.") + await ctx.send(f":x: Please use <#{Channels.advent_of_code_commands}> for aoc commands instead.") error.handled = True -- cgit v1.2.3 From 0ca1c0b55ea5d07694dc53d4f4856cbcddb35a8f Mon Sep 17 00:00:00 2001 From: janine9vn Date: Fri, 11 Dec 2020 19:30:23 -0500 Subject: Change Default AoC Commands Channel Changes the default value of the advent_of_code_commands constant to be the same channel ID as sir-lancebot-commands. If no AoC commands channel is set in the .env file, it'll re-direct people to sir-lancebot-commands instead. --- bot/constants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bot') diff --git a/bot/constants.py b/bot/constants.py index 5dc42462..a58801f7 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -95,7 +95,7 @@ class Branding: class Channels(NamedTuple): admins = 365960823622991872 advent_of_code = int(environ.get("AOC_CHANNEL_ID", 782715290437943306)) - advent_of_code_commands = int(environ.get("AOC_COMMANDS_CHANNEL_ID", 783503267849437205)) + advent_of_code_commands = int(environ.get("AOC_COMMANDS_CHANNEL_ID", 607247579608121354)) announcements = int(environ.get("CHANNEL_ANNOUNCEMENTS", 354619224620138496)) big_brother_logs = 468507907357409333 bot = 267659945086812160 -- cgit v1.2.3 From 8f119f66ec52b1ad55f2078289d35632d0c8127a Mon Sep 17 00:00:00 2001 From: janine9vn Date: Fri, 11 Dec 2020 19:43:43 -0500 Subject: Change AOC_WHITELIST names for clarity AOC_WHITELIST was changed to AOC_WHITELIST_RESTRICTED because it is clearer that commands with this parameter in the `@override_in_channel()` decorator will be restricted to the aoc commands channel and not be allowed in the main aoc channel. In the same vein, AOC_WHITELIST_PLUS was changed to AOC_WHITELIST. --- bot/exts/christmas/advent_of_code/_cog.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'bot') diff --git a/bot/exts/christmas/advent_of_code/_cog.py b/bot/exts/christmas/advent_of_code/_cog.py index 90d92fc1..0968dd26 100644 --- a/bot/exts/christmas/advent_of_code/_cog.py +++ b/bot/exts/christmas/advent_of_code/_cog.py @@ -21,11 +21,11 @@ AOC_REQUEST_HEADER = {"user-agent": "PythonDiscord AoC Event Bot"} COUNTDOWN_STEP = 60 * 5 -AOC_WHITELIST = WHITELISTED_CHANNELS + (Channels.advent_of_code_commands,) +AOC_WHITELIST_RESTRICTED = WHITELISTED_CHANNELS + (Channels.advent_of_code_commands,) # Some commands can be run in the regular advent of code channel # They aren't spammy and foster discussion -AOC_WHITELIST_PLUS = AOC_WHITELIST + (Channels.advent_of_code,) +AOC_WHITELIST = AOC_WHITELIST_RESTRICTED + (Channels.advent_of_code,) async def countdown_status(bot: commands.Bot) -> None: @@ -132,7 +132,7 @@ class AdventOfCode(commands.Cog): self.status_task = self.bot.loop.create_task(status_coro) @commands.group(name="adventofcode", aliases=("aoc",)) - @override_in_channel(AOC_WHITELIST_PLUS) + @override_in_channel(AOC_WHITELIST) async def adventofcode_group(self, ctx: commands.Context) -> None: """All of the Advent of Code commands.""" if not ctx.invoked_subcommand: @@ -143,7 +143,7 @@ class AdventOfCode(commands.Cog): aliases=("sub", "notifications", "notify", "notifs"), brief="Notifications for new days" ) - @override_in_channel(AOC_WHITELIST_PLUS) + @override_in_channel(AOC_WHITELIST) async def aoc_subscribe(self, ctx: commands.Context) -> None: """Assign the role for notifications about new days being ready.""" current_year = datetime.now().year @@ -164,7 +164,7 @@ class AdventOfCode(commands.Cog): @in_month(Month.DECEMBER) @adventofcode_group.command(name="unsubscribe", aliases=("unsub",), brief="Notifications for new days") - @override_in_channel(AOC_WHITELIST_PLUS) + @override_in_channel(AOC_WHITELIST) async def aoc_unsubscribe(self, ctx: commands.Context) -> None: """Remove the role for notifications about new days being ready.""" role = ctx.guild.get_role(AocConfig.role_id) @@ -176,7 +176,7 @@ class AdventOfCode(commands.Cog): await ctx.send("Hey, you don't even get any notifications about new Advent of Code tasks currently anyway.") @adventofcode_group.command(name="countdown", aliases=("count", "c"), brief="Return time left until next day") - @override_in_channel(AOC_WHITELIST_PLUS) + @override_in_channel(AOC_WHITELIST) async def aoc_countdown(self, ctx: commands.Context) -> None: """Return time left until next day.""" if not _helpers.is_in_advent(): @@ -211,7 +211,7 @@ class AdventOfCode(commands.Cog): await ctx.send("", embed=self.cached_about_aoc) @adventofcode_group.command(name="join", aliases=("j",), brief="Learn how to join the leaderboard (via DM)") - @override_in_channel(AOC_WHITELIST_PLUS) + @override_in_channel(AOC_WHITELIST) async def join_leaderboard(self, ctx: commands.Context) -> None: """DM the user the information for joining the Python Discord leaderboard.""" current_year = datetime.now().year @@ -260,7 +260,7 @@ class AdventOfCode(commands.Cog): aliases=("board", "lb"), brief="Get a snapshot of the PyDis private AoC leaderboard", ) - @override_in_channel(AOC_WHITELIST) + @override_in_channel(AOC_WHITELIST_RESTRICTED) async def aoc_leaderboard(self, ctx: commands.Context) -> None: """Get the current top scorers of the Python Discord Leaderboard.""" async with ctx.typing(): @@ -285,7 +285,7 @@ class AdventOfCode(commands.Cog): aliases=("globalboard", "gb"), brief="Get a link to the global leaderboard", ) - @override_in_channel(AOC_WHITELIST) + @override_in_channel(AOC_WHITELIST_RESTRICTED) async def aoc_global_leaderboard(self, ctx: commands.Context) -> None: """Get a link to the global Advent of Code leaderboard.""" url = self.global_leaderboard_url @@ -301,7 +301,7 @@ class AdventOfCode(commands.Cog): aliases=("dailystats", "ds"), brief="Get daily statistics for the Python Discord leaderboard" ) - @override_in_channel(AOC_WHITELIST) + @override_in_channel(AOC_WHITELIST_RESTRICTED) async def private_leaderboard_daily_stats(self, ctx: commands.Context) -> None: """Send an embed with daily completion statistics for the Python Discord leaderboard.""" try: -- cgit v1.2.3 From 0a91d49969600fa2d5f5b9e429ff693f3b94da72 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff Date: Sun, 13 Dec 2020 10:40:56 +0100 Subject: Add callback to log errors in AoC background tasks Currently, our Advent of Code background tasks fail without logging errors or printing error messages. This makes it difficult to debug the errors and means that they may fail silently. While we should ideally find the root cause that hides such errors, I've added a done_callback function in the meantime to help us debug the current issues with the Advent of Code Notification Task. --- bot/exts/christmas/advent_of_code/_cog.py | 6 ++++++ bot/exts/christmas/advent_of_code/_helpers.py | 11 +++++++++++ 2 files changed, 17 insertions(+) (limited to 'bot') diff --git a/bot/exts/christmas/advent_of_code/_cog.py b/bot/exts/christmas/advent_of_code/_cog.py index 0968dd26..0671203e 100644 --- a/bot/exts/christmas/advent_of_code/_cog.py +++ b/bot/exts/christmas/advent_of_code/_cog.py @@ -30,6 +30,7 @@ AOC_WHITELIST = AOC_WHITELIST_RESTRICTED + (Channels.advent_of_code,) async def countdown_status(bot: commands.Bot) -> None: """Set the playing status of the bot to the minutes & hours left until the next day's challenge.""" + log.info("Started `AoC Status Countdown` task") while _helpers.is_in_advent(): _, time_left = _helpers.time_left_to_aoc_midnight() @@ -62,6 +63,7 @@ async def day_countdown(bot: commands.Bot) -> None: Once we have calculated this we should then sleep that number and when the time is reached, ping the Advent of Code role notifying them that the new challenge is ready. """ + log.info("Started `Daily AoC Notification` task") while _helpers.is_in_advent(): tomorrow, time_left = _helpers.time_left_to_aoc_midnight() @@ -127,9 +129,13 @@ class AdventOfCode(commands.Cog): countdown_coro = day_countdown(self.bot) self.countdown_task = self.bot.loop.create_task(countdown_coro) + self.countdown_task.set_name("Daily AoC Notification") + self.countdown_task.add_done_callback(_helpers.background_task_callback) status_coro = countdown_status(self.bot) self.status_task = self.bot.loop.create_task(status_coro) + self.status_task.set_name("AoC Status Countdown") + self.status_task.add_done_callback(_helpers.background_task_callback) @commands.group(name="adventofcode", aliases=("aoc",)) @override_in_channel(AOC_WHITELIST) diff --git a/bot/exts/christmas/advent_of_code/_helpers.py b/bot/exts/christmas/advent_of_code/_helpers.py index d883c09f..da139e40 100644 --- a/bot/exts/christmas/advent_of_code/_helpers.py +++ b/bot/exts/christmas/advent_of_code/_helpers.py @@ -1,3 +1,4 @@ +import asyncio import collections import datetime import json @@ -407,3 +408,13 @@ def time_left_to_aoc_midnight() -> Tuple[datetime.datetime, datetime.timedelta]: # Calculate the timedelta between the current time and midnight return tomorrow, tomorrow - datetime.datetime.now(EST) + + +def background_task_callback(task: asyncio.Task) -> None: + """Check if the finished background task failed to make sure we log errors.""" + if task.cancelled(): + log.info(f"Background task `{task.get_name()}` was cancelled.") + elif exception := task.exception(): + log.error(f"Background task `{task.get_name()}` failed:", exc_info=exception) + else: + log.info(f"Background task `{task.get_name()}` exited normally.") -- cgit v1.2.3