Skip to content

Fix Error Handler#109

Open
PenguinBoi12 wants to merge 4 commits into
mainfrom
fix/error-handler
Open

Fix Error Handler#109
PenguinBoi12 wants to merge 4 commits into
mainfrom
fix/error-handler

Conversation

@PenguinBoi12

@PenguinBoi12 PenguinBoi12 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Description

Error handlers (@bot.error, @ext.error, @cmd.error) only matched the exact exception type, so handlers registered for a base class (or the default @bot.error() fallback) silently never fired for subclasses. Extension/bot-level command error
handlers also never ran for exceptions raised inside a command body.

Additionally, once an error was handled, the command would still send a generic "usage: ..." message and log it as unhandled.

This PR:

  • Matches errors by walking the exception's MRO instead of exact-type lookup
  • Stops double-handling

Type of Change

  • Feature
  • Refactor
  • Bug fix
  • Documentation
  • Other: ___

Pre-merge Checklist

  • Run tests: pytest
  • Run type check: mypy
  • Run formatting: `black .

@PenguinBoi12 PenguinBoi12 self-assigned this Jul 5, 2026
@PenguinBoi12 PenguinBoi12 changed the title Fix/error handler Fix Error Handler Jul 6, 2026
@PenguinBoi12 PenguinBoi12 marked this pull request as ready for review July 7, 2026 06:15
@PenguinBoi12 PenguinBoi12 requested a review from chrisdedman July 7, 2026 06:15

@chrisdedman chrisdedman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. I left on question about the unit test, otherwise good for merging.

Comment on lines +60 to +64
class BaseCustomError(Exception):
pass

class SubCustomError(BaseCustomError):
pass

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this is testing classes error handler, but I wonder if this could be the top header file definition as it is also used in the first unit test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants