From ed815a1cabde59edbea734ed7e10a15bf9a845ff Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 26 Nov 2022 14:09:02 +0400 Subject: Simplify Help Thread Warning Log The current warning log includes the thread name, which means the log message varies wildly between threads. This causes issues with sentry since the actual error message gets trimmed, and sentry fails to group issues from this log as they appear as different messages. Signed-off-by: Hassan Abouelela --- bot/exts/help_channels/_channel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/exts/help_channels/_channel.py b/bot/exts/help_channels/_channel.py index cc3d831b0..d4a742000 100644 --- a/bot/exts/help_channels/_channel.py +++ b/bot/exts/help_channels/_channel.py @@ -80,7 +80,7 @@ async def send_opened_post_dm(thread: discord.Thread) -> None: try: message = await thread.fetch_message(thread.id) except discord.HTTPException: - log.warning(f"Could not fetch message for thread {thread.name} ({thread.id})") + log.warning(f"Could not fetch message for thread {thread.id}") return formatted_message = textwrap.shorten(message.content, width=100, placeholder="...") -- cgit v1.2.3 From fcf7e1cc2cdef58b27cc7d418f22b7bc22b78f07 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 26 Nov 2022 14:11:27 +0400 Subject: Handle Discord API Not Being Ready When discord sends us the thread create event in help channels, it is not ready to perform other operations on the thread such as getting or pinning messages. This causes it to error out when we try to do these actions and claim that those channels don't exist. Instead, we sleep for a short time to try and wait for it to be ready. Signed-off-by: Hassan Abouelela --- bot/exts/help_channels/_channel.py | 7 ++++++- bot/exts/help_channels/_cog.py | 3 +-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/bot/exts/help_channels/_channel.py b/bot/exts/help_channels/_channel.py index d4a742000..3dc9e81ef 100644 --- a/bot/exts/help_channels/_channel.py +++ b/bot/exts/help_channels/_channel.py @@ -1,5 +1,5 @@ """Contains all logic to handle changes to posts in the help forum.""" - +import asyncio import textwrap import discord @@ -109,6 +109,11 @@ async def help_thread_opened(opened_thread: discord.Thread, *, reopen: bool = Fa await _close_help_thread(opened_thread, _stats.ClosingReason.CLEANUP) return + # Discord sends the open event long before the thread is ready for actions in the API. + # This causes actions such as fetching the message, pinning message, etc to fail. + # We sleep here to try and delay our code enough so the thread is ready in the API. + await asyncio.sleep(2) + await send_opened_post_dm(opened_thread) if opened_thread.starter_message: diff --git a/bot/exts/help_channels/_cog.py b/bot/exts/help_channels/_cog.py index 50f8416fc..bb2f43c5a 100644 --- a/bot/exts/help_channels/_cog.py +++ b/bot/exts/help_channels/_cog.py @@ -119,9 +119,8 @@ class HelpForum(commands.Cog): if thread.parent_id != self.help_forum_channel_id: return - await _channel.help_thread_opened(thread) - await self.post_with_disallowed_title_check(thread) + await _channel.help_thread_opened(thread) @commands.Cog.listener() async def on_thread_update(self, before: discord.Thread, after: discord.Thread) -> None: -- cgit v1.2.3 From 4779d4240ae43fd48699ac80c0779665604abebb Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 26 Nov 2022 14:21:58 +0400 Subject: Don't Remove Cooldown Role From Non-Existing Users We try to remove the cooldown role from users before checking if the user is still in the server, which can cause an error since the thread object will just contain `None` as the user. Signed-off-by: Hassan Abouelela --- bot/exts/help_channels/_channel.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bot/exts/help_channels/_channel.py b/bot/exts/help_channels/_channel.py index 3dc9e81ef..a41fcd63f 100644 --- a/bot/exts/help_channels/_channel.py +++ b/bot/exts/help_channels/_channel.py @@ -52,6 +52,15 @@ async def _close_help_thread(closed_thread: discord.Thread, closed_on: _stats.Cl poster = closed_thread.owner cooldown_role = closed_thread.guild.get_role(constants.Roles.help_cooldown) + + if poster is None: + # We can't include the owner ID/name here since the thread only contains None + log.info( + f"Failed to remove cooldown role for owner of thread ({closed_thread.id}). " + f"The user is likely no longer on the server." + ) + return + await members.handle_role_change(poster, poster.remove_roles, cooldown_role) -- cgit v1.2.3 From 81a68784559d231afcc055c2d34cb71314d5bd61 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 26 Nov 2022 14:28:18 +0400 Subject: Catch Failure In Pining Help Starter Message The old method for detecting deleted opener messages does not seem to work, probably because the message is fetched from a cache or similar. Instead we simply try/except pinning the message and pass if the pinning failed. Signed-off-by: Hassan Abouelela --- bot/exts/help_channels/_channel.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/bot/exts/help_channels/_channel.py b/bot/exts/help_channels/_channel.py index a41fcd63f..04c2cc454 100644 --- a/bot/exts/help_channels/_channel.py +++ b/bot/exts/help_channels/_channel.py @@ -125,9 +125,14 @@ async def help_thread_opened(opened_thread: discord.Thread, *, reopen: bool = Fa await send_opened_post_dm(opened_thread) - if opened_thread.starter_message: - # To cover the case where the user deletes their starter message before code execution reaches this line. + try: await opened_thread.starter_message.pin() + except discord.HTTPException as e: + if e.code == 10008: + # The message was not found, most likely deleted + pass + else: + raise e await send_opened_post_message(opened_thread) -- cgit v1.2.3 From 555ed4e9195aae7b5af742c59125833ce8b01b4c Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 26 Nov 2022 14:40:04 +0400 Subject: Check If Thread Is Closed In wait_for_deletion The wait_for_deletion utility would try to remove reactions from a message after the timeout expires, which would normally be fine. In threads however, they can be closed while waiting for the timeout to expire. In such a case, the bot will try to remove the reactions after the channel has been closed and fail. A special exception was added for this case to do nothing, since this is only a QoL feature. Signed-off-by: Hassan Abouelela --- bot/utils/messages.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bot/utils/messages.py b/bot/utils/messages.py index a5ed84351..cc7e6dccb 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -98,7 +98,14 @@ async def wait_for_deletion( try: await bot.instance.wait_for('reaction_add', check=check, timeout=timeout) except asyncio.TimeoutError: - await message.clear_reactions() + try: + await message.clear_reactions() + except discord.HTTPException as e: + if isinstance(message.channel, discord.Thread): + # Threads might not be accessible by the time we try to remove the reaction. + pass + else: + raise e else: await message.delete() except discord.NotFound: -- cgit v1.2.3 From dddf9734f898474ebd841537a49fb8d6905da346 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 26 Nov 2022 15:16:54 +0400 Subject: Check If Thread Is Closed In Pagination Similar to 555ed4e9, the pagination utility needs to catch when it's trying to act on an archived thread. Signed-off-by: Hassan Abouelela --- bot/pagination.py | 93 +++++++++++++++++++++++-------------------------------- 1 file changed, 38 insertions(+), 55 deletions(-) diff --git a/bot/pagination.py b/bot/pagination.py index 10bef1c9f..5b96d8fbb 100644 --- a/bot/pagination.py +++ b/bot/pagination.py @@ -301,42 +301,30 @@ class LinePaginator(Paginator): if str(reaction.emoji) == DELETE_EMOJI: log.debug("Got delete reaction") return await message.delete() - - if reaction.emoji == FIRST_EMOJI: - await message.remove_reaction(reaction.emoji, user) - current_page = 0 - - log.debug(f"Got first page reaction - changing to page 1/{len(paginator.pages)}") - - embed.description = paginator.pages[current_page] - if footer_text: - embed.set_footer(text=f"{footer_text} (Page {current_page + 1}/{len(paginator.pages)})") - else: - embed.set_footer(text=f"Page {current_page + 1}/{len(paginator.pages)}") - await message.edit(embed=embed) - - if reaction.emoji == LAST_EMOJI: - await message.remove_reaction(reaction.emoji, user) - current_page = len(paginator.pages) - 1 - - log.debug(f"Got last page reaction - changing to page {current_page + 1}/{len(paginator.pages)}") - - embed.description = paginator.pages[current_page] - if footer_text: - embed.set_footer(text=f"{footer_text} (Page {current_page + 1}/{len(paginator.pages)})") - else: - embed.set_footer(text=f"Page {current_page + 1}/{len(paginator.pages)}") - await message.edit(embed=embed) - - if reaction.emoji == LEFT_EMOJI: + elif reaction.emoji in PAGINATION_EMOJI: + total_pages = len(paginator.pages) await message.remove_reaction(reaction.emoji, user) - if current_page <= 0: - log.debug("Got previous page reaction, but we're on the first page - ignoring") - continue - - current_page -= 1 - log.debug(f"Got previous page reaction - changing to page {current_page + 1}/{len(paginator.pages)}") + if reaction.emoji == FIRST_EMOJI: + current_page = 0 + log.debug(f"Got first page reaction - changing to page 1/{total_pages}") + elif reaction.emoji == LAST_EMOJI: + current_page = len(paginator.pages) - 1 + log.debug(f"Got last page reaction - changing to page {current_page + 1}/{total_pages}") + elif reaction.emoji == LEFT_EMOJI: + if current_page <= 0: + log.debug("Got previous page reaction, but we're on the first page - ignoring") + continue + + current_page -= 1 + log.debug(f"Got previous page reaction - changing to page {current_page + 1}/{total_pages}") + elif reaction.emoji == RIGHT_EMOJI: + if current_page >= len(paginator.pages) - 1: + log.debug("Got next page reaction, but we're on the last page - ignoring") + continue + + current_page += 1 + log.debug(f"Got next page reaction - changing to page {current_page + 1}/{total_pages}") embed.description = paginator.pages[current_page] @@ -345,27 +333,22 @@ class LinePaginator(Paginator): else: embed.set_footer(text=f"Page {current_page + 1}/{len(paginator.pages)}") - await message.edit(embed=embed) - - if reaction.emoji == RIGHT_EMOJI: - await message.remove_reaction(reaction.emoji, user) - - if current_page >= len(paginator.pages) - 1: - log.debug("Got next page reaction, but we're on the last page - ignoring") - continue - - current_page += 1 - log.debug(f"Got next page reaction - changing to page {current_page + 1}/{len(paginator.pages)}") - - embed.description = paginator.pages[current_page] - - if footer_text: - embed.set_footer(text=f"{footer_text} (Page {current_page + 1}/{len(paginator.pages)})") - else: - embed.set_footer(text=f"Page {current_page + 1}/{len(paginator.pages)}") - - await message.edit(embed=embed) + try: + await message.edit(embed=embed) + except discord.HTTPException as e: + if e.code == 50083: + # Trying to act on an archived thread, just ignore and abort + break + else: + raise e log.debug("Ending pagination and clearing reactions.") with suppress(discord.NotFound): - await message.clear_reactions() + try: + await message.clear_reactions() + except discord.HTTPException as e: + if e.code == 50083: + # Trying to act on an archived thread, just ignore + pass + else: + raise e -- cgit v1.2.3 From 4ca04f60c122f534700e2a350bcbafd2471b96e4 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 26 Nov 2022 15:20:14 +0400 Subject: Handle Images As Starter Messages In the case of an image or other media as the starter message, the formatted message in the help DM will be empty, which is invalid for the embed. We populate the field with some more useful text in this case. Signed-off-by: Hassan Abouelela --- bot/exts/help_channels/_channel.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bot/exts/help_channels/_channel.py b/bot/exts/help_channels/_channel.py index 04c2cc454..b9a79a476 100644 --- a/bot/exts/help_channels/_channel.py +++ b/bot/exts/help_channels/_channel.py @@ -92,7 +92,11 @@ async def send_opened_post_dm(thread: discord.Thread) -> None: log.warning(f"Could not fetch message for thread {thread.id}") return - formatted_message = textwrap.shorten(message.content, width=100, placeholder="...") + formatted_message = textwrap.shorten(message.content, width=100, placeholder="...").strip() + if formatted_message is None: + # This most likely means the initial message is only an image or similar + formatted_message = "No text content." + embed.add_field(name="Your message", value=formatted_message, inline=False) embed.add_field( name="Conversation", -- cgit v1.2.3 From 8cfa7ed5b39b85e432420c5749c0fe1569e2d2cb Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 26 Nov 2022 16:12:42 +0400 Subject: Refactor Thread Error Catching Co-authored-by: Boris Muratov <8bee278@gmail.com> Co-authored-by: Amrou Bellalouna <48383734+shtlrs@users.noreply.github.com> Signed-off-by: Hassan Abouelela --- bot/exts/help_channels/_channel.py | 6 ++---- bot/pagination.py | 13 ++++++++----- bot/utils/messages.py | 15 +++++++-------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/bot/exts/help_channels/_channel.py b/bot/exts/help_channels/_channel.py index b9a79a476..5fc39b623 100644 --- a/bot/exts/help_channels/_channel.py +++ b/bot/exts/help_channels/_channel.py @@ -132,10 +132,8 @@ async def help_thread_opened(opened_thread: discord.Thread, *, reopen: bool = Fa try: await opened_thread.starter_message.pin() except discord.HTTPException as e: - if e.code == 10008: - # The message was not found, most likely deleted - pass - else: + # Suppress if the message was not found, most likely deleted + if e.code != 10008: raise e await send_opened_post_message(opened_thread) diff --git a/bot/pagination.py b/bot/pagination.py index 5b96d8fbb..0ef5808cc 100644 --- a/bot/pagination.py +++ b/bot/pagination.py @@ -303,7 +303,12 @@ class LinePaginator(Paginator): return await message.delete() elif reaction.emoji in PAGINATION_EMOJI: total_pages = len(paginator.pages) - await message.remove_reaction(reaction.emoji, user) + try: + await message.remove_reaction(reaction.emoji, user) + except discord.HTTPException as e: + # Suppress if trying to act on an archived thread. + if e.code != 50083: + raise e if reaction.emoji == FIRST_EMOJI: current_page = 0 @@ -347,8 +352,6 @@ class LinePaginator(Paginator): try: await message.clear_reactions() except discord.HTTPException as e: - if e.code == 50083: - # Trying to act on an archived thread, just ignore - pass - else: + # Suppress if trying to act on an archived thread. + if e.code != 50083: raise e diff --git a/bot/utils/messages.py b/bot/utils/messages.py index cc7e6dccb..8a968f659 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -98,19 +98,18 @@ async def wait_for_deletion( try: await bot.instance.wait_for('reaction_add', check=check, timeout=timeout) except asyncio.TimeoutError: - try: - await message.clear_reactions() - except discord.HTTPException as e: - if isinstance(message.channel, discord.Thread): - # Threads might not be accessible by the time we try to remove the reaction. - pass - else: - raise e + await message.clear_reactions() else: await message.delete() + except discord.NotFound: log.trace(f"wait_for_deletion: message {message.id} deleted prematurely.") + except discord.HTTPException: + if not isinstance(message.channel, discord.Thread): + # Threads might not be accessible by the time the timeout expires + raise + async def send_attachments( message: discord.Message, -- cgit v1.2.3