From 62e4bf908ced8ae36f3253a74a0190a0302b9769 Mon Sep 17 00:00:00 2001 From: penguinboi Date: Sat, 4 Jul 2026 18:12:40 -0400 Subject: [PATCH 1/4] Fix error handlers not matching exception subclasses or firing for extensions --- matrix/bot.py | 4 +-- matrix/command.py | 10 +++++-- matrix/registry.py | 10 +++++++ tests/test_bot.py | 69 +++++++++++++++++++++++++++++++++++++++++++ tests/test_command.py | 27 +++++++++++++++++ 5 files changed, 116 insertions(+), 4 deletions(-) diff --git a/matrix/bot.py b/matrix/bot.py index 4164f79..2dccc2d 100644 --- a/matrix/bot.py +++ b/matrix/bot.py @@ -242,7 +242,7 @@ async def on_error(self, error: Exception) -> None: self.log.exception("Unhandled error: '%s'", error) async def _on_error(self, error: Exception) -> None: - if handler := self._error_handlers.get(type(error)): + if handler := self.resolve_handler(self._error_handlers, error): await handler(error) return @@ -267,7 +267,7 @@ async def _on_command_error(self, ctx: Context, error: Exception) -> None: If a specific error handler is registered for the type of the exception, it will be invoked with the current context and error. """ - if handler := self._command_error_handlers.get(type(error)): + if handler := self.resolve_handler(self._command_error_handlers, error): await handler(ctx, error) return diff --git a/matrix/command.py b/matrix/command.py index 4d2808f..cd1a290 100644 --- a/matrix/command.py +++ b/matrix/command.py @@ -285,12 +285,18 @@ async def on_error(self, ctx: "Context", error: Exception) -> None: """ Executes the registered error handler if present. """ + handler = None - if handler := self._error_handlers.get(type(error)): + for cls in inspect.getmro(type(error)): + if cls in self._error_handlers: + handler = self._error_handlers[cls] + break + + if handler: await handler(ctx, error) return - await ctx.bot.on_command_error(ctx, error) + await ctx.bot._on_command_error(ctx, error) if self._on_error: await self._on_error(ctx, error) diff --git a/matrix/registry.py b/matrix/registry.py index 3da0d57..a7510d9 100644 --- a/matrix/registry.py +++ b/matrix/registry.py @@ -458,3 +458,13 @@ def _register_command_error( if not exception: exception = Exception self._command_error_handlers[exception] = func + + @staticmethod + def resolve_handler( + handlers: Dict[type[Exception], F], error: Exception + ) -> Optional[F]: + """Look up the handler registered for the error's type or nearest base class.""" + for cls in inspect.getmro(type(error)): + if cls in handlers: + return handlers[cls] + return None diff --git a/tests/test_bot.py b/tests/test_bot.py index 18b7f16..b863ae6 100644 --- a/tests/test_bot.py +++ b/tests/test_bot.py @@ -290,6 +290,44 @@ async def custom_error_handler(e): assert called, "Fallback error handler was not called" +@pytest.mark.asyncio +async def test_on_error__with_subclass_error__expect_fallback_handler_called(bot): + called_with = None + + @bot.error() + async def custom_error_handler(e): + nonlocal called_with + called_with = e + + error = ValueError("subclass of Exception") + await bot._on_error(error) + + assert called_with is error, "Fallback handler should catch Exception subclasses" + + +@pytest.mark.asyncio +async def test_on_error__with_specific_and_fallback_handlers__expect_specific_handler_called( + bot, +): + fallback_called = False + specific_called = False + + @bot.error() + async def fallback_handler(e): + nonlocal fallback_called + fallback_called = True + + @bot.error(ValueError) + async def specific_handler(e): + nonlocal specific_called + specific_called = True + + await bot._on_error(ValueError("test error")) + + assert specific_called, "Specific handler should be used when available" + assert not fallback_called, "Fallback handler should not run when a specific one matches" + + @pytest.mark.asyncio async def test_on_error_logs_when_no_handler(bot): error = Exception("test") @@ -379,6 +417,37 @@ async def global_check(ctx): assert isinstance(bot._on_command_error.call_args[0][1], CheckError) +@pytest.mark.asyncio +async def test_command_error_handler__with_error_raised_in_command_body__expect_handler_called( + bot, +): + handled = None + + @bot.error(ValueError, context=True) + async def on_value_error(ctx, error): + nonlocal handled + handled = error + + @bot.command() + async def boom(ctx): + raise ValueError("kaboom") + + event = RoomMessageText.from_dict( + { + "content": {"body": "!boom", "msgtype": "m.text"}, + "event_id": "$ev3", + "origin_server_ts": 1234567890, + "sender": "@user:matrix.org", + "type": "m.room.message", + } + ) + + room = MatrixRoom("!roomid", "alias") + await bot._process_commands(room, event) + + assert isinstance(handled, ValueError) + + @pytest.mark.asyncio async def test_command_executes(bot): called = False diff --git a/tests/test_command.py b/tests/test_command.py index b836d5b..d71c29b 100644 --- a/tests/test_command.py +++ b/tests/test_command.py @@ -10,6 +10,9 @@ class DummyBot: async def on_command_error(self, _ctx, _error): return None + async def _on_command_error(self, ctx, error): + return await self.on_command_error(ctx, error) + class DummyContext: def __init__(self, args=None): @@ -145,6 +148,30 @@ async def handler(_ctx, _error): assert called +@pytest.mark.asyncio +async def test_error_handler__with_exception_subclass__expect_handler_called(): + class BaseCustomError(Exception): + pass + + class SubCustomError(BaseCustomError): + pass + + async def failing_command(ctx): + raise SubCustomError("boom") + + cmd = Command(failing_command) + ctx = DummyContext(args=[]) + handled = None + + @cmd.error(BaseCustomError) + async def handler(_ctx, error): + nonlocal handled + handled = error + + await cmd(ctx) + assert isinstance(handled, SubCustomError) + + @pytest.mark.asyncio async def test_before_and_after_invoke(): call_order = [] From a2f6dd5f15f8c7727dbe5dc8cc5cd85c17d6f817 Mon Sep 17 00:00:00 2001 From: penguinboi Date: Sun, 5 Jul 2026 02:00:03 -0400 Subject: [PATCH 2/4] Stop error handler from double-handling already-caught command errors --- matrix/bot.py | 8 ++++++-- matrix/command.py | 3 ++- tests/test_bot.py | 29 ++++++++++++++++++++++++++--- tests/test_command.py | 37 +++++++++++++++++++++++++++++++++++-- 4 files changed, 69 insertions(+), 8 deletions(-) diff --git a/matrix/bot.py b/matrix/bot.py index 2dccc2d..3f140bb 100644 --- a/matrix/bot.py +++ b/matrix/bot.py @@ -259,19 +259,23 @@ async def on_command_error(self, _ctx: Context, error: Exception) -> None: """Override this in a subclass.""" self.log.exception("Unhandled error: '%s'", error) - async def _on_command_error(self, ctx: Context, error: Exception) -> None: + async def _on_command_error(self, ctx: Context, error: Exception) -> bool: """ Handles errors raised during command invocation. This method is called automatically when a command error occurs. If a specific error handler is registered for the type of the exception, it will be invoked with the current context and error. + + Returns True if a specific handler was found and invoked, False if + it fell through to the default dispatch/log path. """ if handler := self.resolve_handler(self._command_error_handlers, error): await handler(ctx, error) - return + return True await self._dispatch("on_command_error", ctx, error) + return False # ENTRYPOINT diff --git a/matrix/command.py b/matrix/command.py index cd1a290..1b66271 100644 --- a/matrix/command.py +++ b/matrix/command.py @@ -296,7 +296,8 @@ async def on_error(self, ctx: "Context", error: Exception) -> None: await handler(ctx, error) return - await ctx.bot._on_command_error(ctx, error) + if await ctx.bot._on_command_error(ctx, error): + return if self._on_error: await self._on_error(ctx, error) diff --git a/tests/test_bot.py b/tests/test_bot.py index b863ae6..d9e4c9b 100644 --- a/tests/test_bot.py +++ b/tests/test_bot.py @@ -3,7 +3,7 @@ from unittest.mock import AsyncMock, MagicMock, patch from nio import MatrixRoom, RoomMessageText, LoginError -from matrix import Bot, Config, Extension, Room, Space +from matrix import Bot, Config, Context, Extension, Room, Space from matrix.errors import ( CheckError, CommandNotFoundError, @@ -325,7 +325,9 @@ async def specific_handler(e): await bot._on_error(ValueError("test error")) assert specific_called, "Specific handler should be used when available" - assert not fallback_called, "Fallback handler should not run when a specific one matches" + assert ( + not fallback_called + ), "Fallback handler should not run when a specific one matches" @pytest.mark.asyncio @@ -336,6 +338,24 @@ async def test_on_error_logs_when_no_handler(bot): bot.log.exception.assert_called_once_with("Unhandled error: '%s'", error) +@pytest.mark.asyncio +async def test_on_command_error__with_matching_handler__expect_returns_true(bot): + @bot.error(ValueError, context=True) + async def handler(ctx, error): + pass + + result = await bot._on_command_error(MagicMock(), ValueError("boom")) + + assert result is True + + +@pytest.mark.asyncio +async def test_on_command_error__with_no_matching_handler__expect_returns_false(bot): + result = await bot._on_command_error(MagicMock(), ValueError("boom")) + + assert result is False + + @pytest.mark.asyncio async def test_process_commands_executes_command(bot, event): called = False @@ -443,9 +463,12 @@ async def boom(ctx): ) room = MatrixRoom("!roomid", "alias") - await bot._process_commands(room, event) + + with patch.object(Context, "send_help", new_callable=AsyncMock) as mock_send_help: + await bot._process_commands(room, event) assert isinstance(handled, ValueError) + mock_send_help.assert_not_called() @pytest.mark.asyncio diff --git a/tests/test_command.py b/tests/test_command.py index d71c29b..a00dca0 100644 --- a/tests/test_command.py +++ b/tests/test_command.py @@ -1,7 +1,7 @@ import pytest import inspect -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock from matrix.errors import MissingArgumentError from matrix.command import Command @@ -11,7 +11,8 @@ async def on_command_error(self, _ctx, _error): return None async def _on_command_error(self, ctx, error): - return await self.on_command_error(ctx, error) + await self.on_command_error(ctx, error) + return False class DummyContext: @@ -172,6 +173,38 @@ async def handler(_ctx, error): assert isinstance(handled, SubCustomError) +@pytest.mark.asyncio +async def test_on_error__with_bot_handler_matched__expect_no_fallback_help(): + async def failing_command(ctx): + raise ValueError("boom") + + cmd = Command(failing_command) + ctx = DummyContext(args=[]) + ctx.bot._on_command_error = AsyncMock(return_value=True) + ctx.send_help = AsyncMock() + + await cmd(ctx) + + ctx.send_help.assert_not_called() + ctx.logger.exception.assert_not_called() + + +@pytest.mark.asyncio +async def test_on_error__with_no_bot_handler_matched__expect_fallback_help(): + async def failing_command(ctx): + raise ValueError("boom") + + cmd = Command(failing_command) + ctx = DummyContext(args=[]) + ctx.bot._on_command_error = AsyncMock(return_value=False) + ctx.send_help = AsyncMock() + + await cmd(ctx) + + ctx.send_help.assert_awaited_once() + ctx.logger.exception.assert_called_once() + + @pytest.mark.asyncio async def test_before_and_after_invoke(): call_order = [] From 7824c41c3e92494f0893c1d15f6f3b81a7e91853 Mon Sep 17 00:00:00 2001 From: penguinboi Date: Tue, 7 Jul 2026 00:34:34 -0400 Subject: [PATCH 3/4] Add more tests --- tests/test_bot.py | 38 ++++++++++++++++++++++++++++++++++++++ tests/test_command.py | 27 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/tests/test_bot.py b/tests/test_bot.py index d9e4c9b..99bb9cc 100644 --- a/tests/test_bot.py +++ b/tests/test_bot.py @@ -471,6 +471,44 @@ async def boom(ctx): mock_send_help.assert_not_called() +@pytest.mark.asyncio +async def test_command_error_handler__with_extension_handler__expect_handler_called_for_command_body_error( + bot, +): + handled = None + + ext = Extension(name="errors_ext", prefix="!") + + @ext.error(ValueError, context=True) + async def on_value_error(ctx, error): + nonlocal handled + handled = error + + @ext.command() + async def boom(ctx): + raise ValueError("kaboom") + + bot.load_extension(ext) + + event = RoomMessageText.from_dict( + { + "content": {"body": "!boom", "msgtype": "m.text"}, + "event_id": "$ev4", + "origin_server_ts": 1234567890, + "sender": "@user:matrix.org", + "type": "m.room.message", + } + ) + + room = MatrixRoom("!roomid", "alias") + + with patch.object(Context, "send_help", new_callable=AsyncMock) as mock_send_help: + await bot._process_commands(room, event) + + assert isinstance(handled, ValueError) + mock_send_help.assert_not_called() + + @pytest.mark.asyncio async def test_command_executes(bot): called = False diff --git a/tests/test_command.py b/tests/test_command.py index a00dca0..14ceb18 100644 --- a/tests/test_command.py +++ b/tests/test_command.py @@ -173,6 +173,33 @@ async def handler(_ctx, error): assert isinstance(handled, SubCustomError) +@pytest.mark.asyncio +async def test_error_handler__with_grandparent_exception_class__expect_handler_called(): + class GrandparentError(Exception): + pass + + class ParentError(GrandparentError): + pass + + class ChildError(ParentError): + pass + + async def failing_command(ctx): + raise ChildError("boom") + + cmd = Command(failing_command) + ctx = DummyContext(args=[]) + handled = None + + @cmd.error(GrandparentError) + async def handler(_ctx, error): + nonlocal handled + handled = error + + await cmd(ctx) + assert isinstance(handled, ChildError) + + @pytest.mark.asyncio async def test_on_error__with_bot_handler_matched__expect_no_fallback_help(): async def failing_command(ctx): From fe4a4c1c8d4b72365a7e49acca7c32bcaf0ea064 Mon Sep 17 00:00:00 2001 From: penguinboi Date: Tue, 7 Jul 2026 01:57:39 -0400 Subject: [PATCH 4/4] Deduplicate error handler resolver --- matrix/_error_handler.py | 14 +++++++ matrix/bot.py | 5 ++- matrix/command.py | 8 +--- matrix/registry.py | 10 ----- tests/test_error_handler.py | 74 +++++++++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 18 deletions(-) create mode 100644 matrix/_error_handler.py create mode 100644 tests/test_error_handler.py diff --git a/matrix/_error_handler.py b/matrix/_error_handler.py new file mode 100644 index 0000000..df87f9f --- /dev/null +++ b/matrix/_error_handler.py @@ -0,0 +1,14 @@ +import inspect +from typing import Callable, Dict, Optional, TypeVar + +F = TypeVar("F", bound=Callable[..., object]) + + +def resolve_error_handler( + handlers: Dict[type[Exception], F], error: Exception +) -> Optional[F]: + """Look up the handler registered for the error's type or nearest base class.""" + for cls in inspect.getmro(type(error)): + if cls in handlers: + return handlers[cls] + return None diff --git a/matrix/bot.py b/matrix/bot.py index 3f140bb..67c7e24 100644 --- a/matrix/bot.py +++ b/matrix/bot.py @@ -16,6 +16,7 @@ from .registry import Registry from .help import HelpCommand, DefaultHelpCommand from .scheduler import Scheduler +from ._error_handler import resolve_error_handler from .errors import ( AlreadyRegisteredError, CommandNotFoundError, @@ -242,7 +243,7 @@ async def on_error(self, error: Exception) -> None: self.log.exception("Unhandled error: '%s'", error) async def _on_error(self, error: Exception) -> None: - if handler := self.resolve_handler(self._error_handlers, error): + if handler := resolve_error_handler(self._error_handlers, error): await handler(error) return @@ -270,7 +271,7 @@ async def _on_command_error(self, ctx: Context, error: Exception) -> bool: Returns True if a specific handler was found and invoked, False if it fell through to the default dispatch/log path. """ - if handler := self.resolve_handler(self._command_error_handlers, error): + if handler := resolve_error_handler(self._command_error_handlers, error): await handler(ctx, error) return True diff --git a/matrix/command.py b/matrix/command.py index 1b66271..f123b1b 100644 --- a/matrix/command.py +++ b/matrix/command.py @@ -16,6 +16,7 @@ ) from .errors import MissingArgumentError, CheckError, CooldownError +from ._error_handler import resolve_error_handler from time import monotonic from collections import defaultdict, deque @@ -285,12 +286,7 @@ async def on_error(self, ctx: "Context", error: Exception) -> None: """ Executes the registered error handler if present. """ - handler = None - - for cls in inspect.getmro(type(error)): - if cls in self._error_handlers: - handler = self._error_handlers[cls] - break + handler = resolve_error_handler(self._error_handlers, error) if handler: await handler(ctx, error) diff --git a/matrix/registry.py b/matrix/registry.py index a7510d9..3da0d57 100644 --- a/matrix/registry.py +++ b/matrix/registry.py @@ -458,13 +458,3 @@ def _register_command_error( if not exception: exception = Exception self._command_error_handlers[exception] = func - - @staticmethod - def resolve_handler( - handlers: Dict[type[Exception], F], error: Exception - ) -> Optional[F]: - """Look up the handler registered for the error's type or nearest base class.""" - for cls in inspect.getmro(type(error)): - if cls in handlers: - return handlers[cls] - return None diff --git a/tests/test_error_handler.py b/tests/test_error_handler.py new file mode 100644 index 0000000..5d2c984 --- /dev/null +++ b/tests/test_error_handler.py @@ -0,0 +1,74 @@ +from matrix._error_handler import resolve_error_handler + + +def test_resolve_error_handler_with_exact_type_match__expect_handler_returned(): + def handler(error): + pass + + handlers = {ValueError: handler} + + assert resolve_error_handler(handlers, ValueError("boom")) is handler + + +def test_resolve_error_handler_with_exception_subclass__expect_base_handler_returned(): + class BaseCustomError(Exception): + pass + + class SubCustomError(BaseCustomError): + pass + + def handler(error): + pass + + handlers = {BaseCustomError: handler} + + assert resolve_error_handler(handlers, SubCustomError("boom")) is handler + + +def test_resolve_error_handler_with_grandparent_exception_class__expect_handler_returned(): + class GrandparentError(Exception): + pass + + class ParentError(GrandparentError): + pass + + class ChildError(ParentError): + pass + + def handler(error): + pass + + handlers = {GrandparentError: handler} + + assert resolve_error_handler(handlers, ChildError("boom")) is handler + + +def test_resolve_error_handler_with_no_matching_handler__expect_none_returned(): + def handler(error): + pass + + handlers = {ValueError: handler} + + assert resolve_error_handler(handlers, TypeError("boom")) is None + + +def test_resolve_error_handler_with_empty_handlers__expect_none_returned(): + assert resolve_error_handler({}, ValueError("boom")) is None + + +def test_resolve_error_handler_with_specific_and_base_registered__expect_specific_handler_returned(): + class BaseCustomError(Exception): + pass + + class SubCustomError(BaseCustomError): + pass + + def base_handler(error): + pass + + def specific_handler(error): + pass + + handlers = {BaseCustomError: base_handler, SubCustomError: specific_handler} + + assert resolve_error_handler(handlers, SubCustomError("boom")) is specific_handler