feat: discord_errors() middleware + store.mutate migration (server_stats) - #130
Merged
Conversation
…lies A central, type-aware complement to catch_errors(): catches discord.Forbidden, NotFound, and HTTPException and sends localized ephemeral messages, so plugins need not repeat the same try/except per command. Register after catch_errors() so it handles Discord errors first and catch_errors() stays the outer fallback; non-Discord errors propagate untouched. Includes 6 tests.
Replace the manual GuildLockManager + load/save ceremony with store.mutate, consolidating on the single canonical lock domain the store already provides (behavior-preserving). Removes the now-unused _locks and updates the plugin's tests to seed config the same way. Reference migration for the other plugins that still use the manual lock+load+save pattern.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideAdds a new discord_errors() middleware for centralized, type-aware handling of Discord API exceptions, and migrates the server_stats plugin (and its tests) from manual GuildLockManager + load/save patterns to ServerConfigStore.mutate-based config updates, removing the custom lock manager. Sequence diagram for discord_errors() middleware handling Discord API exceptionssequenceDiagram
participant Bot
participant discord_errors_middleware as discord_errors
participant CommandHandler as proceed
participant Context as ctx
Bot->>discord_errors_middleware: handler(ctx, proceed)
discord_errors_middleware->>CommandHandler: await proceed()
alt discord.Forbidden raised
CommandHandler-->>discord_errors_middleware: discord.Forbidden
discord_errors_middleware->>Context: respond(forbidden or ctx.t("errors.forbidden"), ephemeral=True)
else discord.NotFound raised
CommandHandler-->>discord_errors_middleware: discord.NotFound
discord_errors_middleware->>Context: respond(not_found or ctx.t("errors.not_found"), ephemeral=True)
else discord.HTTPException raised
CommandHandler-->>discord_errors_middleware: discord.HTTPException
discord_errors_middleware->>Context: respond(http_error or ctx.t("errors.http"), ephemeral=True)
else no Discord error
CommandHandler-->>discord_errors_middleware: success
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
PR Summary by QodoAdd discord_errors middleware; migrate server_stats to store.mutate
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
discord_errors, consider whetherlogger.warningforForbidden/NotFoundandlogger.errorfor allHTTPExceptions is the right granularity for your logs, as these may be routine occurrences and could create noisy logs over time. - The three exception branches in
discord_errorsrepeat the samectx.respondpattern with only message text and log level differing; factoring this into a small helper (e.g., taking the message, log method, and error label) would reduce duplication and make future changes less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `discord_errors`, consider whether `logger.warning` for `Forbidden`/`NotFound` and `logger.error` for all `HTTPException`s is the right granularity for your logs, as these may be routine occurrences and could create noisy logs over time.
- The three exception branches in `discord_errors` repeat the same `ctx.respond` pattern with only message text and log level differing; factoring this into a small helper (e.g., taking the message, log method, and error label) would reduce duplication and make future changes less error-prone.
## Individual Comments
### Comment 1
<location path="easycord/middleware.py" line_range="387-390" />
<code_context>
+ ),
+ ephemeral=True,
+ )
+ except discord.HTTPException as exc:
+ logger.error("HTTPException in %s: %s", ctx.command_name, exc)
+ with contextlib.suppress(Exception):
+ await ctx.respond(
</code_context>
<issue_to_address>
**suggestion:** Use `logger.exception` to retain the traceback for HTTP errors.
This branch only logs the exception message, so the traceback is lost and debugging HTTP issues becomes harder. Using `logger.exception("HTTPException in %s", ctx.command_name)` will capture the full stack trace while keeping the log concise and independent of `exc.__str__`.
```suggestion
except discord.HTTPException as exc:
logger.exception("HTTPException in %s", ctx.command_name)
with contextlib.suppress(Exception):
await ctx.respond(
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code Review by Qodo
1. Lost Discord stack traces
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| import discord | ||
| import pytest | ||
|
|
||
| from easycord.middleware import discord_errors |
There was a problem hiding this comment.
2. discord_errors not in public api 📘 Rule violation ⌂ Architecture
The new test imports discord_errors from easycord.middleware, but the public API contract requires consumers outside easycord/ to import only from the top-level easycord package. This makes the test (and any external consumers copying it) rely on a non-public module path.
Agent Prompt
## Issue description
Code outside the `easycord/` package should not import from `easycord.*` submodules unless the symbol is part of the public, top-level API. The new test imports `discord_errors` from `easycord.middleware`, but `discord_errors` is not re-exported from `easycord/__init__.py`.
## Issue Context
This PR adds a new public-facing middleware. Tests serve as examples for users, so they should use only the supported public import surface.
## Fix Focus Areas
- tests/test_discord_errors_middleware.py[9-9]
- easycord/__init__.py[52-54]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+365
to
+368
| except discord.Forbidden: | ||
| logger.warning("Forbidden in %s", ctx.command_name) | ||
| with contextlib.suppress(Exception): | ||
| await ctx.respond( |
There was a problem hiding this comment.
3. Lost discord stack traces 🐞 Bug ◔ Observability
discord_errors() consumes Discord exceptions so catch_errors() never logs the traceback, but the new log lines don’t include exc_info, losing the call site and stack trace for production debugging. This is especially impactful for discord.Forbidden/NotFound where only a generic warning is emitted.
Agent Prompt
## Issue description
`discord_errors()` catches `discord.Forbidden`, `discord.NotFound`, and `discord.HTTPException` and does not re-raise them, which is intended, but it logs without `exc_info`. Since these exceptions no longer reach `catch_errors()` (which uses `logger.exception`), the traceback is lost.
## Issue Context
- `catch_errors()` logs full tracebacks with `logger.exception`, but only for exceptions that bubble out.
- `discord_errors()` intercepts Discord exceptions earlier in the chain, so it becomes the primary logging point for these failures.
## Fix Focus Areas
- easycord/middleware.py[311-399]
## Suggested change
- Capture the exception (`except discord.Forbidden as exc:` / `except discord.NotFound as exc:` / `except discord.HTTPException as exc:`) and log with traceback, e.g.:
- `logger.warning("Forbidden in %s", ctx.command_name, exc_info=exc)`
- `logger.warning("NotFound in %s", ctx.command_name, exc_info=exc)`
- `logger.error("HTTPException in %s", ctx.command_name, exc_info=exc)`
- or use `logger.exception(...)` where appropriate if you want stack traces consistently.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+347
to
+350
| Register it *after* :func:`catch_errors` so it runs closer to the command | ||
| and handles Discord errors first, leaving ``catch_errors`` as the outer | ||
| fallback for everything else:: | ||
|
|
There was a problem hiding this comment.
4. Ordering guidance too narrow 🐞 Bug ⚙ Maintainability
The discord_errors() docstring says to register it after catch_errors() so it runs closer to the command, but middleware execution depends on append order and the stack can include other built-ins (e.g., rate_limit). The guidance can lead to placing discord_errors() outside later middleware unintentionally, expanding its scope beyond “while running a command.”
Agent Prompt
## Issue description
The docstring guidance for registering `discord_errors()` only references `catch_errors()`, but in practice middleware order is determined by append order + `build_chain(reversed(...))`. Users may have other middleware (e.g., `rate_limit`) and could place `discord_errors()` in a position that changes what it wraps/catches.
## Issue Context
- Middlewares are appended via `Bot.use()`.
- The first middleware in the list runs first (outermost), because the chain is built by iterating `reversed(middleware)`.
## Fix Focus Areas
- easycord/middleware.py[31-40]
- easycord/middleware.py[338-353]
## Suggested change
Update the `discord_errors()` docstring to explicitly describe ordering in terms of “outermost vs innermost”, e.g.:
- Recommend registering `catch_errors()` early (outermost) and registering `discord_errors()` late (innermost/closest to the command), ideally after other middleware that should remain outside it (like rate limiting / auth gates).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
rolling-codes
added a commit
that referenced
this pull request
Aug 11, 2026
Release **v5.61.0**, bundling everything merged since v5.60.0. ## Included - **#128** — offline `easycord try` CLI (run a slash command without Discord) - **#129** — plugin ergonomics helpers: `PluginConfigHelper` (atomic config via `store.mutate`), decorator stacks (`slash_admin_command` etc.), `TaskManager`/`TimerManager` - **#130** — `discord_errors()` middleware + `server_stats` `store.mutate` migration - **#131** — `slash_mod_command` now uses `moderate_members` (real perm) instead of the nonexistent `timeout_members` ## Version bump `5.60.0 → 5.61.0` (minor: additive features + fixes, no breaking changes) across `pyproject.toml`, `easycord/__init__.py`, `README.md`, `docs/getting-started.md`; CHANGELOG entry added. ## Gates (per RELEASE.md) - [x] `check_release_metadata.py` — no drift - [x] `ruff check ... --select E9,F63,F7,F82` — clean - [x] `verify_plugin_tests.py` — thresholds met - [x] `pytest` — 1865 passed - [ ] `pyright easycord tests` — 12 errors / 135 warnings, all **pre-existing on main** and unrelated to this release (CI does not gate on pyright); noted, not introduced here. After merge: tag `v5.61.0` and publish a GitHub Release with the built wheel + sdist. PyPI `twine upload` left as a manual step (requires credentials). ## Summary by Sourcery Release EasyCord v5.61.0 with new CLI tooling, plugin ergonomics helpers, Discord error handling middleware, and minor fixes, plus corresponding version and metadata updates. New Features: - Add offline `easycord try` CLI to run registered slash commands without Discord connectivity. - Introduce plugin ergonomics helpers including config mixin, decorator stacks, and task/timer lifecycle managers. - Add `discord_errors()` middleware to handle common Discord API errors with user-facing messages. Bug Fixes: - Correct `slash_mod_command` to require the real `moderate_members` permission instead of nonexistent `timeout_members`. - Fix `TaskManager.track` race that could leave replacement tasks untracked and uncancellable. - Remove unused imports in decorator stacks flagged by static analysis. Enhancements: - Route `server_stats` configuration writes through `ServerConfigStore.mutate` for consolidated locking and consistency. Build: - Bump project version to 5.61.0 in pyproject, package metadata, and module `__version__`, and add corresponding changelog entry. - Update release metadata entries for wheel and source distribution artifacts. Documentation: - Update README and getting-started docs to reference the v5.61.0 release, download URLs, and badges. Co-authored-by: tee <Thomas_BIRRELL@proton.me>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related plugin-ergonomics improvements, grounded in what the framework already provides:
discord_errors()middleware (easycord/middleware.py) — catchesdiscord.Forbidden/NotFound/HTTPExceptionraised while running a command and replies with a localized, type-specific ephemeral message. It's the central, type-aware complement to the existingcatch_errors()(a generic catch-all) and the existing@command_error(per-command). Register it aftercatch_errors()so it handles Discord errors first andcatch_errors()remains the outer fallback; non-Discord errors propagate untouched.server_statsconfig writes →store.mutate— replaces the manualGuildLockManager+load/saveceremony withServerConfigStore.mutate, consolidating on the single lock domain the store already owns. Behavior-preserving; removes the now-unused_locks.Why this shape (not a decorator / not a bulk refactor)
@safe_discordwas considered and rejected: EasyCord already has@command_error("cmd")for per-command type-specific handling andcatch_errors()for the safety net. A new execution-wrapping decorator would duplicate@command_errorand risk breaking slash-parameter introspection. A middleware fits the framework's model, needs one registration, and carries no introspection risk.load/savewrap synchronous I/O and never suspend between them).store.mutateis a simplification/consistency win.Scope note (server_stats only)
server_statsis included as a verified reference migration. The remaining plugins that still use the manual pattern (auto_role, birthday, giveaway, polls, reminder, reputation, scheduled_announcements, tickets, verification, word_filter) are not in this PR: the migration is not mechanical — several interleave earlyreturns and post-lock Discord I/O with their mutation (which can't move intomutate's synchronous callback verbatim), and each plugin's tests couple to_locksand need updating too. Recommend doing them incrementally, one verified plugin per change, rather than a risky bulk pass.Test plan
discord_errors(): 6 new tests (tests/test_discord_errors_middleware.py) — per-type messages, custom overrides, non-Discord errors propagate, success path silent.server_stats: existing suite updated and green (19 tests).ruff check ... --select E9,F63,F7,F82clean; pyright clean on changed files.Summary by Sourcery
Introduce middleware to translate Discord API errors into user-friendly command responses and simplify server_stats configuration updates by using the shared config store mutation API.
New Features:
Enhancements:
Tests: