From fa55efc264e54f773952c5062063b2ce5397f806 Mon Sep 17 00:00:00 2001 From: borketh Date: Tue, 17 Sep 2024 17:53:12 -0400 Subject: [PATCH 1/7] fix: potential fix for user info failure + better logging --- fred/cogs/levelling.py | 12 +++++++--- fred/cogs/webhooklistener.py | 2 +- fred/fred_commands/__init__.py | 2 +- fred/fred_commands/dbcommands.py | 1 + fred/libraries/common.py | 38 ++++++++++++++++++++------------ 5 files changed, 36 insertions(+), 19 deletions(-) diff --git a/fred/cogs/levelling.py b/fred/cogs/levelling.py index b875584..719b455 100644 --- a/fred/cogs/levelling.py +++ b/fred/cogs/levelling.py @@ -4,7 +4,6 @@ from datetime import * from nextcord import DMChannel, Message, Guild -from nextcord.ext.commands import MemberNotFound from .. import config from ..libraries import common @@ -20,7 +19,7 @@ def __init__(self, user_id: int, guild: Guild): self.member = guild.get_member(user_id) if self.member is None: logger.warning(f"Unable to retrieve information about user {user_id}") - raise MemberNotFound(f"Unable to retrieve information about user {user_id}") + # raise MemberNotFound(f"Unable to retrieve information about user {user_id}") logger.info(f"Found member id {self.member}") @@ -50,7 +49,14 @@ def xp_count(self, value: float): self._xp_count = value self.DB_user.xp_count = value + async def try_resolve_member(self): + if (member := await self.guild.fetch_member(self.user_id)) is not None: + self.member = member + else: + logger.warning(f"Still unable to resolve user {self.user_id}") + async def validate_role(self): + await self.try_resolve_member() if not self.member: logger.info( "Could not validate someone's level role because they aren't in the main guild", @@ -69,7 +75,7 @@ async def validate_role(self): self.DB_user.rank_role_id = role_id # if not self.guild.get_channel(config.Misc.fetch("mod_channel")).permissions_for(self.member).send_messages: - if not common.permission_check(self.member, level=6): + if not await common.permission_check(self.member, level=6): for member_role in self.member.roles: if config.RankRoles.fetch_by_role(member_role.id) is not None: # i.e. member_role is a rank role logpayload["role_id"] = member_role.id diff --git a/fred/cogs/webhooklistener.py b/fred/cogs/webhooklistener.py index ea87790..9c4a268 100644 --- a/fred/cogs/webhooklistener.py +++ b/fred/cogs/webhooklistener.py @@ -25,7 +25,7 @@ def runServer(self, bot): try: server = HTTPServerV6((ip, port), MakeGithookHandler(bot)) server.serve_forever() - except PermissionError as pe: + except PermissionError: logger.error(f"Cannot handle githooks! Permission denied to listen to {ip=} {port=}.") diff --git a/fred/fred_commands/__init__.py b/fred/fred_commands/__init__.py index 577965b..5f3b7f5 100644 --- a/fred/fred_commands/__init__.py +++ b/fred/fred_commands/__init__.py @@ -26,7 +26,7 @@ class Commands(BotCmds, ChannelCmds, CommandCmds, CrashCmds, DialogflowCmds, EXP @BaseCmds.listener() async def on_command_error(self, ctx: commands.Context, error): # We get an error about commands being found when using "runtime" commands, so we have to ignore that - self.logger.error(f"Caught {error!r}, {dir(error)}") + self.logger.error(f"Caught {error!r}") if isinstance(error, commands.CommandNotFound): command = ctx.message.content.lower().lstrip(self.bot.command_prefix).split(" ")[0] if config.Commands.fetch(command) is not None: diff --git a/fred/fred_commands/dbcommands.py b/fred/fred_commands/dbcommands.py index 8390a5e..26c6de7 100644 --- a/fred/fred_commands/dbcommands.py +++ b/fred/fred_commands/dbcommands.py @@ -101,6 +101,7 @@ async def modify_command(self, ctx: commands.Context, command_name: str.lower, * results[0].attachment = attachment.url if attachment else None await self.bot.reply_to_msg(ctx.message, f"Command '{command_name}' modified!") + self.logger.info(f"Command {command_name} modified. New response: '{new_response}'") @staticmethod def _valid_aliases(target: str, aliases: list[str]) -> dict[str, list[str | tuple[str, str]]]: diff --git a/fred/libraries/common.py b/fred/libraries/common.py index 982ab77..f6e25e3 100644 --- a/fred/libraries/common.py +++ b/fred/libraries/common.py @@ -44,61 +44,71 @@ def is_bot_author(user_id: int) -> bool: async def l4_only(ctx: Context) -> bool: logger.info("Checking if someone is a T3", extra=user_info(ctx.author)) - return is_bot_author(ctx.author.id) or permission_check(ctx, level=4) + return is_bot_author(ctx.author.id) or await permission_check(ctx, level=4) async def mod_only(ctx: Context) -> bool: logger.info("Checking if someone is a Moderator", extra=user_info(ctx.author)) - return is_bot_author(ctx.author.id) or permission_check(ctx, level=6) + return is_bot_author(ctx.author.id) or await permission_check(ctx, level=6) @singledispatch -def permission_check(_, *, level: int) -> bool: +async def permission_check(_ctx_or_member, *, level: int) -> bool: pass @permission_check.register -def _permission_check_ctx(ctx: Context, *, level: int) -> bool: +async def _permission_check_ctx(ctx: Context, *, level: int) -> bool: main_guild_id = config.Misc.fetch("main_guild_id") - main_guild = ctx.bot.get_guild(main_guild_id) + main_guild = await ctx.bot.fetch_guild(main_guild_id) if main_guild is None: raise LookupError(f"Unable to retrieve the guild {main_guild_id}. Is this the guild you meant?") - if (main_guild_member := main_guild.get_member(ctx.author.id)) is None: + if (main_guild_member := await main_guild.fetch_member(ctx.author.id)) is None: logger.warning( "Checked permissions for someone but they weren't in the main guild", extra=user_info(ctx.author) ) return False - return _permission_check_member(main_guild_member, level=level) + return await _permission_check_member(main_guild_member, threshold_level=level) @permission_check.register -def _permission_check_member(member: Member, *, level: int) -> bool: +async def _permission_check_member(member: Member, *, threshold_level: int) -> bool: """Checks permissions for a member assuming they are in the main guild.""" logpayload = user_info(member) - logpayload["level"] = level + logpayload["level"] = threshold_level if member.guild.id != config.Misc.fetch("main_guild_id"): logger.warning("Checked permissions for a member of the wrong guild", extra=logpayload) return False - logger.info("Checking permissions for someone", extra=logpayload) - perms = config.PermissionRoles.fetch_by_lvl(level) + logger.info(f"Checking permissions for {member.display_name} ({member.id})", extra=logpayload) + perms = config.PermissionRoles.fetch_by_lvl(threshold_level) user_roles = [role.id for role in member.roles] if ( # it shouldn't be possible to request a level above the defined levels but check anyway role := next( - (permission for permission in perms if permission.perm_lvl >= level and permission.role_id in user_roles), + ( + permission + for permission in perms + if permission.perm_lvl >= threshold_level and permission.role_id in user_roles + ), False, ) # checks for the first occurring, if any ): - logger.info(f"A permission check was positive with level {role.perm_lvl}", extra=logpayload) + logger.info( + f"Permission granted for {member.display_name} (lvl {role.perm_lvl}, threshold {threshold_level})", + extra=logpayload, + ) return True # user has a role that is above the requested level - logger.info(f"A permission check was negative with level less than required ({level})", extra=logpayload) + logger.info( + f"Permission denied for {member.display_name} (lvl {role.perm_lvl}, threshold {threshold_level})", + extra=logpayload, + ) return False From 84847ac47ba3429156fc57de44852601365e07c8 Mon Sep 17 00:00:00 2001 From: borketh Date: Tue, 17 Sep 2024 17:53:48 -0400 Subject: [PATCH 2/7] fix: don't propagate reply if it would be to the bot itself --- fred/fred.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/fred/fred.py b/fred/fred.py index 640ae0f..bf850d0 100644 --- a/fred/fred.py +++ b/fred/fred.py @@ -203,7 +203,14 @@ async def reply_to_msg( self.logger.info("Replying to a message", extra=common.message_info(message)) # use this line if you're trying to debug discord throwing code 400s # self.logger.debug(jsonpickle.dumps(dict(content=content, **kwargs), indent=2)) - reference = (message.reference if propagate_reply else None) or message + if propagate_reply and message.reference is not None: + reference = message.reference + if (referenced_message := message.reference.cached_message) is not None: + if referenced_message.author == self.user: + reference = message + else: + reference = message + if self.owo and content is not None: content = common.owoize(content) if isinstance(reference, nextcord.MessageReference): From 908db8a1b61422bbb461e1554017ec91e564f884 Mon Sep 17 00:00:00 2001 From: borketh Date: Tue, 17 Sep 2024 17:54:22 -0400 Subject: [PATCH 3/7] fix: handle attachment urls properly --- fred/fred_commands/__init__.py | 6 +++++- fred/fred_commands/dbcommands.py | 8 +++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/fred/fred_commands/__init__.py b/fred/fred_commands/__init__.py index 5f3b7f5..21e1673 100644 --- a/fred/fred_commands/__init__.py +++ b/fred/fred_commands/__init__.py @@ -5,9 +5,12 @@ import io import logging import re +from os.path import split +from urllib.parse import urlparse import nextcord from algoliasearch.search_client import SearchClient +from nextcord import Attachment from nextcord.ext.commands.view import StringView from ._baseclass import BaseCmds, common, config, commands @@ -86,7 +89,8 @@ async def on_message(self, message: nextcord.Message): if (attachment := command["attachment"]) is not None: async with self.bot.web_session.get(attachment) as resp: buff = io.BytesIO(await resp.read()) - attachment = nextcord.File(filename=attachment.split("/")[-1], fp=buff) + _, filename = split(urlparse(attachment).path) + attachment = nextcord.File(filename=filename, fp=buff) args = [] view = StringView(message.content.lstrip(prefix)) view.get_word() # command name diff --git a/fred/fred_commands/dbcommands.py b/fred/fred_commands/dbcommands.py index 26c6de7..66a2120 100644 --- a/fred/fred_commands/dbcommands.py +++ b/fred/fred_commands/dbcommands.py @@ -27,7 +27,7 @@ async def add_command(self, ctx: commands.Context, command_name: str.lower, *, r await self.bot.reply_to_msg(ctx.message, "This command name is reserved") return - attachment = ctx.message.attachments[0].url if ctx.message.attachments else None + attachment = ctx.message.attachments[0] if ctx.message.attachments else None if not response and not attachment: response, attachment = await self.bot.reply_question(ctx.message, "What should the response be?") @@ -38,9 +38,10 @@ async def add_command(self, ctx: commands.Context, command_name: str.lower, *, r if not await self.bot.reply_yes_or_no(ctx.message, msg): return - config.Commands(name=command_name, content=response, attachment=attachment) + config.Commands(name=command_name, content=response, attachment=attachment.url) await self.bot.reply_to_msg(ctx.message, f"Command '{command_name}' added!") + self.logger.info("Command {command_name} added with response '{response}'") @BaseCmds.remove.command(name="command") async def remove_command(self, ctx: commands.Context, command_name: str.lower): @@ -60,6 +61,7 @@ async def remove_command(self, ctx: commands.Context, command_name: str.lower): config.Commands.deleteBy(name=command_name) await self.bot.reply_to_msg(ctx.message, "Command removed!") + self.logger.info(f"Command {command_name} removed!") @BaseCmds.modify.command(name="command") async def modify_command(self, ctx: commands.Context, command_name: str.lower, *, new_response: str = None): @@ -98,7 +100,7 @@ async def modify_command(self, ctx: commands.Context, command_name: str.lower, * # this just works, don't touch it. trying to use config.Commands.fetch makes a duplicate command. results[0].content = new_response - results[0].attachment = attachment.url if attachment else None + results[0].attachment = attachment or attachment.url await self.bot.reply_to_msg(ctx.message, f"Command '{command_name}' modified!") self.logger.info(f"Command {command_name} modified. New response: '{new_response}'") From 86fae966f8dfc538d30ccc763aae399db0bf2928 Mon Sep 17 00:00:00 2001 From: borketh Date: Tue, 17 Sep 2024 17:55:58 -0400 Subject: [PATCH 4/7] chore: version bump (2.22.5) --- fred/fred.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fred/fred.py b/fred/fred.py index bf850d0..62df7e5 100644 --- a/fred/fred.py +++ b/fred/fred.py @@ -18,7 +18,7 @@ from .fred_commands import Commands, FredHelpEmbed from .libraries import createembed, common -__version__ = "2.22.4" +__version__ = "2.22.5" class Bot(commands.Bot): diff --git a/pyproject.toml b/pyproject.toml index ab1062c..ea8c240 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "fred" -version = "2.22.4" +version = "2.22.5" description = "A Discord bot for the Satisfactory Modding Discord " authors = ["Feyko ", "Mircea Roata ", "Borketh "] license = "MIT License" From b6b4e898a89965c5af3546a0ddde1dcf7e5a04bb Mon Sep 17 00:00:00 2001 From: borketh Date: Tue, 17 Sep 2024 18:04:06 -0400 Subject: [PATCH 5/7] fix: don't query discord if we already have the member info --- fred/cogs/levelling.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/fred/cogs/levelling.py b/fred/cogs/levelling.py index 719b455..718343c 100644 --- a/fred/cogs/levelling.py +++ b/fred/cogs/levelling.py @@ -50,10 +50,11 @@ def xp_count(self, value: float): self.DB_user.xp_count = value async def try_resolve_member(self): - if (member := await self.guild.fetch_member(self.user_id)) is not None: - self.member = member - else: - logger.warning(f"Still unable to resolve user {self.user_id}") + if self.member is None: + if (member := await self.guild.fetch_member(self.user_id)) is not None: + self.member = member + else: + logger.warning(f"Still unable to resolve user {self.user_id}") async def validate_role(self): await self.try_resolve_member() @@ -86,6 +87,7 @@ async def validate_role(self): await self.member.add_roles(role) async def validate_level(self): + await self.try_resolve_member() if not self.member: logger.info( "Could not validate someone's level because they aren't in the main guild", From a5d54ddfcd222e5baae1a53a71f885f0cd00e61c Mon Sep 17 00:00:00 2001 From: borketh Date: Tue, 17 Sep 2024 18:17:54 -0400 Subject: [PATCH 6/7] chore: amend comment for robb but with proper spacing that won't fuck with python --- fred/cogs/levelling.py | 1 + 1 file changed, 1 insertion(+) diff --git a/fred/cogs/levelling.py b/fred/cogs/levelling.py index 718343c..5c84f52 100644 --- a/fred/cogs/levelling.py +++ b/fred/cogs/levelling.py @@ -19,6 +19,7 @@ def __init__(self, user_id: int, guild: Guild): self.member = guild.get_member(user_id) if self.member is None: logger.warning(f"Unable to retrieve information about user {user_id}") + # Silencing error about this, Laterâ„¢ problem -Borketh # raise MemberNotFound(f"Unable to retrieve information about user {user_id}") logger.info(f"Found member id {self.member}") From af791b1a177c210841f963f1457510aa6b26d7cd Mon Sep 17 00:00:00 2001 From: borketh Date: Tue, 17 Sep 2024 18:28:30 -0400 Subject: [PATCH 7/7] fix: utility function for guaranteed member lookup assuming of course the member didn't leave in the split second between execution --- fred/cogs/levelling.py | 2 +- fred/fred_commands/dbcommands.py | 2 +- fred/libraries/common.py | 8 ++++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/fred/cogs/levelling.py b/fred/cogs/levelling.py index 5c84f52..0fa3907 100644 --- a/fred/cogs/levelling.py +++ b/fred/cogs/levelling.py @@ -52,7 +52,7 @@ def xp_count(self, value: float): async def try_resolve_member(self): if self.member is None: - if (member := await self.guild.fetch_member(self.user_id)) is not None: + if (member := await common.get_guild_member(self.guild, self.user_id)) is not None: self.member = member else: logger.warning(f"Still unable to resolve user {self.user_id}") diff --git a/fred/fred_commands/dbcommands.py b/fred/fred_commands/dbcommands.py index 66a2120..cc57140 100644 --- a/fred/fred_commands/dbcommands.py +++ b/fred/fred_commands/dbcommands.py @@ -100,7 +100,7 @@ async def modify_command(self, ctx: commands.Context, command_name: str.lower, * # this just works, don't touch it. trying to use config.Commands.fetch makes a duplicate command. results[0].content = new_response - results[0].attachment = attachment or attachment.url + results[0].attachment = attachment and attachment.url await self.bot.reply_to_msg(ctx.message, f"Command '{command_name}' modified!") self.logger.info(f"Command {command_name} modified. New response: '{new_response}'") diff --git a/fred/libraries/common.py b/fred/libraries/common.py index f6e25e3..dd2955c 100644 --- a/fred/libraries/common.py +++ b/fred/libraries/common.py @@ -5,7 +5,7 @@ from functools import lru_cache, singledispatch from typing import TYPE_CHECKING, Optional -from nextcord import User, Message, Member +from nextcord import User, Message, Member, Guild from nextcord.ext import commands from nextcord.ext.commands import Context @@ -37,6 +37,10 @@ def __init__(self, bot: Bot): self.logger.debug("Cog loaded.") +async def get_guild_member(guild: Guild, user_id: int) -> Member: + return guild.get_member(user_id) or await guild.fetch_member(user_id) + + def is_bot_author(user_id: int) -> bool: logger.info("Checking if someone is the author", extra={"user_id": user_id}) return user_id == 227473074616795137 @@ -65,7 +69,7 @@ async def _permission_check_ctx(ctx: Context, *, level: int) -> bool: if main_guild is None: raise LookupError(f"Unable to retrieve the guild {main_guild_id}. Is this the guild you meant?") - if (main_guild_member := await main_guild.fetch_member(ctx.author.id)) is None: + if (main_guild_member := await get_guild_member(main_guild, ctx.author.id)) is None: logger.warning( "Checked permissions for someone but they weren't in the main guild", extra=user_info(ctx.author) )