From 4471c213862866b3b760d3dc69f4bfd7987fecef Mon Sep 17 00:00:00 2001 From: Rosie Val Date: Tue, 28 Jul 2026 23:50:10 +0100 Subject: [PATCH] Fix RubyLogging crashes: __slots__ attribute injection and dead config key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _handle_message_delete_with_cache tried to stash cached attachment data as message._cached_attachments, but discord.Message uses __slots__ and rejects arbitrary attribute assignment — this crashed on_message_delete every time a deleted message had cached attachments (42 times in 48h in production). Fixed by looking the cache up by message id in self._attachment_cache instead (same data, same spot _build_embed already checked, just accessed legally), deferring eviction until after it's actually used. Also removed two dead `accent = await config.accent_color()` calls — accent_color was never a registered config key, so both crashed unconditionally whenever the attachment-preservation feature actually ran, and the fetched value was never even used afterward. Co-Authored-By: Claude Sonnet 5 --- rubylogging/rubylogging.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/rubylogging/rubylogging.py b/rubylogging/rubylogging.py index 6d137e1..d9c52d6 100644 --- a/rubylogging/rubylogging.py +++ b/rubylogging/rubylogging.py @@ -299,14 +299,11 @@ class RubyLogging(commands.Cog): if not message.guild: return - # Check if there are cached attachments - cached = self._attachment_cache.pop(message.id, None) - - # If we have cached attachments, pass them through in the event data - # so the preview can be shown in the main delete log - if cached: - # Add cached data to message object temporarily - message._cached_attachments = cached # type: ignore + # Peek (don't evict yet) — discord.Message uses __slots__, so cached + # data can't be stashed as an extra attribute on the message object. + # _build_embed() looks this up from self._attachment_cache by message + # id instead, so the entry needs to still be there while it runs. + cached = self._attachment_cache.get(message.id) # Process the normal message_delete event (this will show the preview if cached) await self._process_event("message_delete", "messages", message) @@ -317,6 +314,9 @@ class RubyLogging(commands.Cog): if await config.log_attachments(): await self._log_deleted_attachments(message, cached) + # Now safe to evict — nothing below this point needs it. + self._attachment_cache.pop(message.id, None) + async def _handle_bulk_delete_with_cache(self, messages: List[discord.Message]) -> None: """Handle bulk message deletion with cached attachments.""" if not messages: @@ -372,7 +372,6 @@ class RubyLogging(commands.Cog): reupload = await config.reupload_attachments() # Build embed - accent = await config.accent_color() color_coded = await config.color_coded() include_timestamp = await config.include_timestamp() @@ -510,7 +509,6 @@ class RubyLogging(commands.Cog): if not channel or not isinstance(channel, discord.TextChannel): return - accent = await config.accent_color() color_coded = await config.color_coded() color = discord.Color.red() if color_coded else discord.Color.greyple() @@ -740,9 +738,11 @@ class RubyLogging(commands.Cog): if args and isinstance(args[0], discord.Message): msg = args[0] - # First check for cached attachments (for deleted messages) - if hasattr(msg, '_cached_attachments'): - cached = msg._cached_attachments + # First check for cached attachments (for deleted messages) — looked + # up by message id, since discord.Message uses __slots__ and can't + # hold an extra attribute stashed onto the object itself. + cached = self._attachment_cache.get(msg.id) + if cached: for att_data in cached: if att_data.get('is_image'): # Use proxy URL which lasts longer