Skip to content
This repository was archived by the owner on Apr 6, 2026. It is now read-only.

fix(moderation): improve autodeleter resilience and DB connection stability#308

Merged
karaktaka merged 3 commits into
mainfrom
fix/autodeleter-rate-limits
Mar 10, 2026
Merged

fix(moderation): improve autodeleter resilience and DB connection stability#308
karaktaka merged 3 commits into
mainfrom
fix/autodeleter-rate-limits

Conversation

@karaktaka
Copy link
Copy Markdown
Contributor

@karaktaka karaktaka commented Mar 10, 2026

Summary

  • Autodeleter bulk delete: Recent messages (< 14 days) are now bulk-deleted via channel.delete_messages() — up to 100 messages per API call instead of one at a time. This eliminates the 40062 per-resource rate limit errors that were hitting every 5-minute tick.
  • Old message cap: Messages older than 14 days (individual-delete only, no bulk option) are capped at 50 per tick with a 1 s sleep between each, keeping each loop iteration well under 1 minute. Any remainder is picked up on the next tick.
  • Per-channel error isolation: Errors on one channel no longer abort the whole tick. A 429 on any channel logs a warning and skips remaining channels for that run.
  • Guild None guard: Fixed a latent crash if the bot has left a guild that still has an AutoDelete config.
  • DB connection pre-ping: Added pool_pre_ping=True to the SQLAlchemy engine so stale connections after a DB restart are transparently recycled before use, preventing the cascade of OperationalError failures that would otherwise follow an AdminShutdown.

Test plan

  • Autodeleter loop runs without 429 errors on channels with message backlogs
  • Messages older than 14 days are individually deleted with 1 s pacing
  • A channel with KeepMessages=N correctly preserves the newest N candidates
  • Pinned messages are skipped unless DeletePinnedMessage=True
  • A channel error (e.g. missing permissions) does not abort other channels in the same tick
  • After a DB restart, the bot reconnects cleanly without cascading errors

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved database connection reliability by proactively verifying connections before reuse.
  • New Features

    • Enhanced moderation auto-deletion: per-channel cleanup with controlled batching, separate handling of recent vs. older messages, optional exclusion of pinned items, limited individual deletions per run, safer thread removal, and graceful handling of rate limits and per-channel errors for more robust cleanup.

…bility

Rewrite the autodeleter background loop to eliminate Discord API rate
limiting (error 40062). Recent messages (< 14 days) are now bulk-deleted
via channel.delete_messages() — up to 100 per API call instead of one at
a time. Older messages are individually deleted with a 1 s sleep and
capped at 50 per tick so each loop iteration stays under ~1 minute.
Per-channel errors no longer abort the whole tick; a 429 from any channel
logs a warning and skips remaining channels for that run.

Also add pool_pre_ping=True to the SQLAlchemy engine so stale connections
from a DB restart are transparently recycled before use.

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 10, 2026

Warning

Rate limit exceeded

@karaktaka has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 58 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 07c44899-acda-4918-9afb-73c8889db467

📥 Commits

Reviewing files that changed from the base of the PR and between e19d946 and ee9dbcf.

📒 Files selected for processing (2)
  • NerdyPy/modules/moderation.py
  • tests/modules/test_moderation_loops.py
📝 Walkthrough

Walkthrough

Engine creation now uses pool_pre_ping=True. The moderation autodelete flow was refactored: per-channel cleanup extracted to _cleanup_channel, with message partitioning (bulk vs. old), batch deletion, thread deletion, HTTP 429 handling, and per-channel error isolation.

Changes

Cohort / File(s) Summary
Engine Configuration
NerdyPy/bot.py
Added pool_pre_ping=True to SQLAlchemy Engine creation to pre-validate pooled connections.
Auto-deletion Workflow Refactor
NerdyPy/modules/moderation.py
Introduced async def _cleanup_channel(...). Added imports, new constants (_BULK_MAX_AGE, _BULK_BATCH_SIZE, _MAX_INDIVIDUAL_PER_RUN), moved per-channel cleanup into the new method, partitioned deletions into bulk (<=14 days) and individual-old (>=14 days), added batch deletion with delays, thread deletions, HTTP 429 handling to stop remaining channels in a run, and per-channel error logging/isolation.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibble through channels, tidy and spry,
Bulk hops in batches, old threads say goodbye,
Rate limits slow me, I wait with a twitch,
One channel at a time, sorted without a hitch,
Hooray — clean and neat, a rabbit's little fix!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the two main changes in the changeset: improving autodeleter resilience (per-channel error isolation, rate limit handling, message batching) and DB connection stability (pool_pre_ping=True).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/autodeleter-rate-limits

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@NerdyPy/modules/moderation.py`:
- Around line 175-183: The inner except blocks that catch HTTPException around
m.thread.delete() currently swallow all HTTP errors; update both occurrences
(the loop over bulk where you call m.thread.delete() and the second similar
thread-delete block) to detect rate-limit responses and re-raise them: in each
except HTTPException as e, if getattr(e, "status", None) == 429 or (hasattr(e,
"response") and e.response is not None and e.response.status == 429) then raise
e, otherwise continue to swallow/ignore; keep the existing asyncio.sleep and
non-429 handling unchanged so rate-limit errors propagate to the outer handler
that enforces stopping.
- Around line 152-158: The fetch window logic currently grabs the oldest
fetch_limit messages and then preserves the newest message_limit within that
truncated slice; change the history fetch to retrieve newest-first (use
channel.history(..., oldest_first=False) or remove oldest_first=True) so
candidates contains newest messages first, then compute to_delete as
candidates[message_limit:] if message_limit > 0 else candidates to preserve the
actual newest messages in the channel; additionally, in the thread deletion
except blocks around thread.delete() (and the analogous individual-message
thread deletion) re-raise or propagate rate-limit errors instead of swallowing
them by checking the caught HTTPException (or discord.HTTPException) for a 429
status (e.g., if getattr(e, "status", None) == 429: raise) and only suppress
non-429 errors.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 87da4abe-4295-4f54-941d-18ffd6ec6eec

📥 Commits

Reviewing files that changed from the base of the PR and between db57180 and 5e07342.

📒 Files selected for processing (2)
  • NerdyPy/bot.py
  • NerdyPy/modules/moderation.py

Comment thread NerdyPy/modules/moderation.py Outdated
Comment thread NerdyPy/modules/moderation.py
…hread deletes

Switch history fetch to newest-first so keep_messages correctly preserves
the actual newest N messages in the channel, not just the newest within a
truncated oldest-first window (which would silently delete messages that
should have been kept on channels with large backlogs).

Re-raise 429 HTTPExceptions from thread.delete() calls in both the bulk
and individual deletion paths so rate-limit signals propagate to the outer
handler that stops processing remaining channels for the run.

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 10, 2026

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #309

Comment thread NerdyPy/modules/moderation.py Fixed
Comment thread NerdyPy/modules/moderation.py Fixed
coderabbitai Bot added a commit that referenced this pull request Mar 10, 2026
Docstrings generation was requested by @karaktaka.

* #308 (comment)

The following files were modified:

* `NerdyPy/bot.py`
* `NerdyPy/modules/moderation.py`
Remove unnecessary pass statements from both thread-delete except blocks;
the if ex.status == 429: raise is already a valid statement body so pass
was redundant. Replace with an inline comment to preserve intent.

Update test_autodelete_deletes_old_messages to supply the mock history in
newest-first order ([newer_msg, old_msg]) to match the oldest_first=False
fetch; candidates[1:] then correctly targets old_msg for deletion.

Co-Authored-By: Claude <noreply@anthropic.com>
@karaktaka karaktaka merged commit 39954b3 into main Mar 10, 2026
9 checks passed
@karaktaka karaktaka deleted the fix/autodeleter-rate-limits branch March 10, 2026 09:36
karaktaka pushed a commit that referenced this pull request Mar 10, 2026
Docstrings generation was requested by @karaktaka.

* #308 (comment)

The following files were modified:

* `NerdyPy/bot.py`
* `NerdyPy/modules/moderation.py`
karaktaka added a commit that referenced this pull request Mar 10, 2026
…bility (#308)

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant