Delete automated infraction messages after a period of time#3339
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that automated infraction messages sent by the bot are cleaned up after a configurable delay, persisting deletion schedules across restarts via Redis.
- Adds an
AUTOMATED_TIDY_UP_HOURSconstant and Redis-backed cache for message deletion times. - Introduces a separate
tidy_up_scheduleralongside existing expiration scheduling. - Implements
_delete_infraction_messagefor safe removal and cache cleanup, and integrates scheduling incog_loadandapply_infraction.
Comments suppressed due to low confidence (1)
bot/exts/moderation/infraction/_scheduler.py:330
- Add tests to cover the scheduling and execution of automated message deletions, including verifying that entries are persisted to and removed from Redis as expected across restarts.
self.tidy_up_scheduler.schedule_at(
| channel = await get_or_fetch_channel(self.bot, channel_id) | ||
| if channel is None: | ||
| log.warning(f"Channel {channel_id} not found for infraction message deletion.") | ||
| return |
There was a problem hiding this comment.
Should we also delete it from Redis in this case? On one hand, it's nice to be able to try later if there is a transient issue with fetching the channel. On the other hand, it's wasteful to keep trying if the channel has been deleted.
There was a problem hiding this comment.
Consider setting a TTL in Redis for these records. That way if a channel truly is gone, stale records won't pile up.
|
How does this interact with threads that get closed? Can the bot still delete messages? |
|
Bot should always have permissions to delete messages anywhere in the server. I will handle discord.Forbidden though and ensure we log it out (but still clear the record, this should all be best effort). |
jb3
force-pushed
the
jb3/infraction-message-tidy-up
branch
from
8bef06b to
b50ba64
Compare
jb3
force-pushed
the
jb3/infraction-message-tidy-up
branch
from
b50ba64 to
c4e62e4
Compare
| except discord.NotFound: | ||
| log.warning(f"Channel or message {message_id} not found in channel {channel_id}.") | ||
| except discord.Forbidden: | ||
| log.warning(f"Bot lacks permissions to delete message {message_id} in channel {channel_id}.") |
There was a problem hiding this comment.
Unsure if this should always be a warning, since trying to delete messages in closed threads will also raise Forbidden iirc. No point in sending that specific case to sentry
There was a problem hiding this comment.
Ah I hadn't realised that manage messages does not override this. I'll drop it to an info log.
This PR automatically deletes messages generated by the bot as part of dealing out automated infractions (e.g. spam filters). These have a habit of filling up channels which are frequently abused but have low traffic otherwise (some channels appear as just a wall of automated infractions).
As of now, the timer is set to 8 hours, though we can increase this if we feel we need the longer persistence (though obviously the full record and log is still preserved in the #mod-alerts channel).
Deletion times are persisted to Redis to ensure we try to delete stuff after the bot restarts.
Closes #2317.