Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Centralize error handling #205

Merged
merged 6 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
Changelog
=========

- :feature:`205` Add :obj:`pydis_core.utils.error_handling.commands.abc.AbstractCommandErrorHandler` and :obj:`pydis_core.utils.error_handling.commands.manager.CommandErrorManager` to implement and register command error handlers independantly.
- :breaking:`208` Split ``fakeredis`` optional dependency from the ``async-rediscache`` extra. You can now install with ``[fakeredis]`` to just install fakeredis (with lua support), ``[async-rediscache]`` to install just ``async-rediscache``, or use either ``[all]`` or ``[async-rediscache,fakeredis]`` to install both. This allows users who do no rely on fakeredis to install in 3.12 environments.
- :support:`208` Add support for Python 3.12. Be aware, at time of writing, our usage of fakeredis does not currently support 3.12. This is due to :literal-url:`this lupa issue<https://github.com/scoder/lupa/issues/245>`. Lupa is required by async-rediscache for lua script support within fakeredis. As such, fakeredis can not be installed in a Python 3.12 environment.
- :breaking:`208` Drop support for Python 3.10
Expand Down
31 changes: 31 additions & 0 deletions pydis_core/_bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@

import aiohttp
import discord
from discord import app_commands
from discord.ext import commands

from pydis_core.async_stats import AsyncStatsClient
from pydis_core.site_api import APIClient
from pydis_core.utils import scheduling
from pydis_core.utils._extensions import walk_extensions
from pydis_core.utils.error_handling.commands import CommandErrorManager
from pydis_core.utils.logging import get_logger

try:
Expand All @@ -32,6 +34,23 @@ def __init__(self, base: Exception):
self.exception = base


class CommandTreeBase(app_commands.CommandTree):
"""A sub-class of the Command tree that implements common features that Python Discord bots use."""

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# Instance is None since discordpy only passes an instance of the client to the command tree in its constructor.
self.command_error_manager: CommandErrorManager | None = None

async def on_error(self, interaction: discord.Interaction, error: app_commands.AppCommandError) -> None:
"""A callback that is called when any command raises an :exc:`AppCommandError`."""
if not self.command_error_manager:
log.warning("Command error manager hasn't been loaded in the command tree.")
await super().on_error(interaction, error)
return
await self.command_error_manager.handle_error(error, interaction)


class BotBase(commands.Bot):
"""
A sub-class that implements many common features that Python Discord bots use.
Expand Down Expand Up @@ -76,9 +95,11 @@ def __init__(
super().__init__(
*args,
allowed_roles=allowed_roles,
tree_cls=CommandTreeBase,
**kwargs,
)

self.command_error_manager: CommandErrorManager | None = None
self.guild_id = guild_id
self.http_session = http_session
self.api_client = api_client
Expand All @@ -100,6 +121,16 @@ def __init__(

self.all_extensions: frozenset[str] | None = None

def register_command_error_manager(self, manager: CommandErrorManager) -> None:
"""
Bind an instance of the command error manager to both the bot and the command tree.

The reason this doesn't happen in the constructor is because error handlers might need an instance of the bot.
So registration needs to happen once the bot instance has been created.
"""
self.command_error_manager = manager
self.tree.command_error_manager = manager

def _connect_statsd(
self,
statsd_url: str,
Expand Down
6 changes: 6 additions & 0 deletions pydis_core/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
"""Useful utilities and tools for Discord bot development."""

import pydis_core.utils.error_handling.commands as command_error_handling_module
import pydis_core.utils.error_handling.commands.abc as error_handling_abstractions
import pydis_core.utils.error_handling.commands.manager as command_error_manager
from pydis_core.utils import (
_monkey_patches,
caching,
Expand Down Expand Up @@ -44,6 +47,9 @@ def apply_monkey_patches() -> None:
channel,
checks,
commands,
command_error_handling_module,
error_handling_abstractions,
command_error_manager,
cooldown,
error_handling,
function,
Expand Down
4 changes: 4 additions & 0 deletions pydis_core/utils/error_handling/commands/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from .abc import AbstractCommandErrorHandler
from .manager import CommandErrorManager

__all__ = ["AbstractCommandErrorHandler", "CommandErrorManager"]
24 changes: 24 additions & 0 deletions pydis_core/utils/error_handling/commands/abc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from abc import ABC, abstractmethod
from typing import NoReturn

from discord import Interaction
from discord.ext.commands import Context


class AbstractCommandErrorHandler(ABC):
"""An abstract command error handler."""

@abstractmethod
async def should_handle_error(self, error: Exception) -> bool:
"""A predicate that determines whether the error should be handled."""
raise NotImplementedError

@abstractmethod
async def handle_app_command_error(self, interaction: Interaction, error: Exception) -> NoReturn:
"""Handle error raised in the context of app commands."""
raise NotImplementedError

@abstractmethod
async def handle_text_command_error(self, context: Context, error: Exception) -> NoReturn:
"""Handle error raised in the context of text commands."""
raise NotImplementedError
58 changes: 58 additions & 0 deletions pydis_core/utils/error_handling/commands/manager.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
from collections.abc import Callable
from typing import NoReturn

from discord import Interaction
from discord.ext.commands import Context

from pydis_core.utils.error_handling.commands import AbstractCommandErrorHandler
from pydis_core.utils.logging import get_logger

log = get_logger(__name__)


class CommandErrorManager:
"""A class that registers error handlers and handles all command related errors."""

def __init__(self, default: AbstractCommandErrorHandler):
self._handlers = []
self._registered_handlers = set()
self._default = default

async def handle_error(
self,
error: Exception,
context_or_interaction: Context | Interaction
) -> None:
"""
Handle a Discord exception.

Iterate through available handlers by registration order, and choose the first one capable of handling
the error as determined by `should_handle_error`; there is no priority system.
"""
for handler in self._handlers + [self._default]:
if await handler.should_handle_error(error):
callback = self._get_callback(handler, context_or_interaction)
await callback(context_or_interaction, error)
break

def register_handler(self, handler: AbstractCommandErrorHandler) -> None:
"""Register a command error handler."""
handler_name = type(handler).__name__
if handler_name in self._registered_handlers:
log.info(f"Skipping registration of command error handler {handler_name!r} as it's already registered.")
return

self._handlers.append(handler)
self._registered_handlers.add(handler_name)

@staticmethod
def _get_callback(
handler: AbstractCommandErrorHandler,
context_or_interaction: Context | Interaction
) -> Callable[[Context, Exception], NoReturn] | Callable[[Interaction, Exception], NoReturn] | None:
"""Get the handling callback that will be used."""
if isinstance(context_or_interaction, Context):
return handler.handle_text_command_error
if isinstance(context_or_interaction, Interaction):
return handler.handle_app_command_error
raise ValueError(f"Expected Context or Interaction, got {type(context_or_interaction).__name__}")
Loading