🐛 Incorrect None check in react_roles - fetch_message never returns None

[anthropic-code-agent]

Description

In react_roles/react_roles.py, there is incorrect error handling that checks if fetch_message() returns None, but this method never returns None - it raises exceptions instead.

Location

File: react_roles/react_roles.py
Lines: 55-56

Current Code

message = await channel.fetch_message(message_id)
if not message:  # ❌ fetch_message never returns None!
    log.error(f"Message {message_id} not found, removing from config...")
    # cleanup code

The Problem

Discord.py's fetch_message() has two possible outcomes:

1. Success: Returns a discord.Message object (never None)
2. Failure: Raises discord.NotFound exception

The current code assumes it can return None, which is incorrect. This means:

• The if not message: check never triggers
• If a message is deleted, fetch_message() will raise discord.NotFound instead
• The cleanup code is never reached
• The exception propagates up, potentially crashing the bot

From Discord.py Documentation

async def fetch_message(message_id: int) -> Message:
    """Retrieves a single Message from the destination.
    
    Raises
    ------
    discord.NotFound
        The specified message was not found.
    discord.Forbidden
        You do not have the permissions required to get a message.
    discord.HTTPException
        Retrieving the message failed.
    """

Notice there's no mention of returning None - it always returns Message or raises an exception.

Impact

• Incorrect error handling: Deleted messages cause exceptions instead of cleanup
• Config pollution: Deleted messages stay in the config forever
• Potential bot crash: Unhandled discord.NotFound can propagate
• Misleading code: Future developers might copy this incorrect pattern

Fix Required

Replace the None check with proper exception handling:

try:
    message = await channel.fetch_message(message_id)
except discord.NotFound:
    log.error(f"Message {message_id} not found, removing from config...")
    async with self.config.channel(channel).watching() as watching:
        if str(message_id) in watching:
            del watching[str(message_id)]
    continue  # Skip to next message
except discord.Forbidden:
    log.error(f"No permission to fetch message {message_id}")
    continue
except discord.HTTPException as e:
    log.error(f"Failed to fetch message {message_id}: {e}")
    continue

Similar Issues to Check

Search for other instances of this pattern:

grep -n "fetch_message" *.py | grep "if not"

Also check:

• fetch_user() - also raises exceptions, never returns None
• fetch_channel() - same behavior
• fetch_guild() - same behavior
• get_message() (cache lookup) - this one DOES return None

Priority

MEDIUM - Causes incorrect behavior and config pollution, but doesn't always crash.

Testing

After fixing:

1. Set up a reaction role on a message
2. Delete the message
3. Restart the bot (to reload reaction role listeners)
4. Verify the deleted message is cleaned from config
5. Check logs for proper error messages (no crashes)

Additional Note

For clarity, use get_message() when you want to check the cache without fetching:

# Check cache (returns None if not cached)
message = channel.get_message(message_id)
if not message:
    # Try fetching
    try:
        message = await channel.fetch_message(message_id)
    except discord.NotFound:
        # Handle missing message

Related

This is part of a comprehensive code review. See other issues for additional improvements.

References

• Discord.py TextChannel.fetch_message docs
• Discord.py exceptions