Skip to content

feat(errors): add DBusFastError common base class#634

Merged
bdraco merged 2 commits into
mainfrom
koan/common-exception-base
May 15, 2026
Merged

feat(errors): add DBusFastError common base class#634
bdraco merged 2 commits into
mainfrom
koan/common-exception-base

Conversation

@bluetoothbot
Copy link
Copy Markdown
Contributor

@bluetoothbot bluetoothbot commented May 14, 2026

What

Introduce DBusFastError as a common base class for every exception raised by dbus-fast.

Why

Closes #507. Today, dbus-fast exceptions derive from a mix of ValueError, TypeError, and Exception, with no shared ancestor. Applications that want to distinguish dbus-fast errors from other errors must either wrap dbus-fast calls in dedicated try/except blocks or list every error class by name — both clutter caller code.

How

  • New DBusFastError(Exception) in dbus_fast/errors.py.
  • Every existing error class now inherits from DBusFastError.
  • Classes that previously derived from ValueError (SignatureBodyMismatchError, InvalidSignatureError, InvalidAddressError, InvalidMessageError, InvalidIntrospectionError) and from TypeError (InvalidBusNameError, InvalidObjectPathError, InvalidInterfaceNameError, InvalidMemberNameError) use multiple inheritance to preserve the built-in base — existing except ValueError / except TypeError handlers keep working.
  • DBusFastError is exported from the package root.

Testing

  • New tests/test_errors.py covers MRO, isinstance checks for the new base, preservation of ValueError / TypeError semantics, and unchanged error message formatting.
  • Full non-integration suite passes (pytest -q ... → 106 passed, 5 skipped).

🤖 Generated with Claude Code


Quality Report

Changes: 3 files changed, 154 insertions(+), 13 deletions(-)

Code scan: clean

Tests: failed ([Errno 13] Permission denied: 'pytest')

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

Introduce DBusFastError so callers can catch every dbus-fast exception
with one except clause instead of listing each error class individually
or wrapping calls in separate try/except blocks.

All existing error classes now inherit from DBusFastError. Classes that
previously derived from ValueError or TypeError still do — they now use
multiple inheritance so existing handlers keep working unchanged.

Fixes #507

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.06%. Comparing base (e959ea0) to head (658f6d8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #634      +/-   ##
==========================================
- Coverage   86.11%   86.06%   -0.06%     
==========================================
  Files          29       29              
  Lines        3487     3488       +1     
  Branches      602      602              
==========================================
- Hits         3003     3002       -1     
- Misses        299      300       +1     
- Partials      185      186       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 14, 2026

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing koan/common-exception-base (658f6d8) with main (e959ea0)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (54b6825) during the generation of this report, so e959ea0 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new DBusFastError base exception to unify dbus-fast’s public exception hierarchy, allowing callers to catch library-specific errors with a single except DBusFastError while keeping compatibility with existing except ValueError / except TypeError handlers.

Changes:

  • Added DBusFastError(Exception) and updated existing dbus-fast exception classes to inherit from it (with multiple inheritance for the historical ValueError / TypeError subclasses).
  • Exported DBusFastError from the package root (dbus_fast.__init__).
  • Added a new test module validating subclass relationships and message formatting for key exceptions.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/dbus_fast/errors.py Introduces DBusFastError and updates existing exception classes to inherit from it.
src/dbus_fast/__init__.py Re-exports DBusFastError from the package root and adds it to __all__.
tests/test_errors.py Adds tests for the new exception hierarchy and ensures key behaviors remain unchanged.
Comments suppressed due to low confidence (1)

src/dbus_fast/errors.py:46

  • In these TypeError-derived exceptions, super().init(...) currently resolves to Exception.init (because DBusFastError precedes TypeError in the MRO), so TypeError.init is never invoked. If the goal is to preserve TypeError semantics as closely as possible, prefer ordering the bases as (TypeError, DBusFastError) (and likewise for the ValueError subclasses).
class InvalidBusNameError(DBusFastError, TypeError):
    def __init__(self, name: str) -> None:
        super().__init__(f"invalid bus name: {name}")


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/dbus_fast/errors.py Outdated
Comment thread tests/test_errors.py
@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 14, 2026

@bluetoothbot reviewrebase

Address the copilot comments

@bluetoothbot
Copy link
Copy Markdown
Contributor Author

bluetoothbot commented May 14, 2026

PR Review — feat(errors): add DBusFastError common base class

Clean, well-scoped change. Introduces DBusFastError as a common base, preserves ValueError/TypeError semantics via multiple inheritance, exports the new class, and adds focused tests. No correctness, security, or performance concerns. The two suggestions (flip base order to put the built-in first; add explicit MRO assertions) are non-blocking polish items that align with the Copilot feedback the author was asked to address. Merge-ready as-is; addressing the suggestions would just make the review history complete.


🟢 Suggestions

1. MRO order in multiple inheritance — consider built-in first (`src/dbus_fast/errors.py`, L14-57)

Copilot flagged this, but the technical claim is partially incorrect: ValueError and TypeError do not override Exception.__init__, so super().__init__(msg) ends up at the same Exception.__init__ regardless of base order. Functionally these classes behave identically whether you write (DBusFastError, ValueError) or (ValueError, DBusFastError).

That said, there's still a defensible reason to swap the order to (ValueError, DBusFastError) / (TypeError, DBusFastError):

  1. It preserves the historical ordering more faithfully (these were originally class Foo(ValueError): — the built-in was the primary identity).
  2. It matches the convention seen in stdlib mixins (more-specific built-in type first, mixin/marker second).
  3. __mro__, repr(cls.__bases__), and Sphinx-generated docs all reflect the base order, so this is visible to users and doc tools even if runtime behavior is identical.

Not a blocker — current code is correct — but if you're touching the file to address review, flipping the order is a low-cost improvement that also moots Copilot's concern.

class InvalidBusNameError(DBusFastError, TypeError):
    def __init__(self, name: str) -> None:
        super().__init__(f"invalid bus name: {name}")
2. Add explicit MRO assertion to lock in base order (`tests/test_errors.py`, L60-87)

The existing issubclass(...) checks confirm the inheritance relationships exist, but they don't pin the order of the bases. If someone later refactors and accidentally swaps (DBusFastError, ValueError)(ValueError, DBusFastError) (or vice versa), every current test still passes, even though __mro__ and visible base order would change.

A single assertion per family would lock this in:

def test_value_error_subclass_mro_order() -> None:
    mro = SignatureBodyMismatchError.__mro__
    # Assert the chosen order — whichever you decide is canonical.
    assert mro.index(DBusFastError) < mro.index(ValueError)

def test_type_error_subclass_mro_order() -> None:
    mro = InvalidBusNameError.__mro__
    assert mro.index(DBusFastError) < mro.index(TypeError)

This is what Copilot's second comment was getting at and it's a cheap regression guard.


Checklist

  • No hardcoded secrets
  • Input validation at boundaries
  • No bare except / swallowed errors
  • Backwards-compatible (ValueError/TypeError still catchable)
  • MRO order is deliberate and tested — suggestion #1, suggestion #2
  • Test coverage for new behavior
  • Public API exported correctly (init.py + all)
  • Python 3.11+ compatible syntax

Summary

Clean, well-scoped change. Introduces DBusFastError as a common base, preserves ValueError/TypeError semantics via multiple inheritance, exports the new class, and adds focused tests. No correctness, security, or performance concerns. The two suggestions (flip base order to put the built-in first; add explicit MRO assertions) are non-blocking polish items that align with the Copilot feedback the author was asked to address. Merge-ready as-is; addressing the suggestions would just make the review history complete.


Automated review by Kōancdc4432

@bluetoothbot
Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/common-exception-base was rebased onto main and review feedback was applied.

Changes applied

  • Changes applied.
  • Summary

  • src/dbus_fast/errors.py: Swapped base-class order for all multiple-inheritance error classes per Copilot review. ValueError/TypeError now precede DBusFastError, so super().__init__ routes through the built-in exception type's __init__ instead of skipping to Exception.__init__. Affects SignatureBodyMismatchError, InvalidSignatureError, InvalidAddressError, InvalidMessageError, InvalidIntrospectionError, InvalidBusNameError, InvalidObjectPathError, InvalidInterfaceNameError, InvalidMemberNameError.
  • tests/test_errors.py: Added test_value_error_subclass_mro_order and test_type_error_subclass_mro_order per Copilot review, asserting ValueError/TypeError come before DBusFastError in the MRO to lock in the intended base ordering against future regressions.

Stats

3 files changed, 177 insertions(+), 13 deletions(-)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=main's src/dbus_fast/errors.py contains no DBusFastError base class; only PR branch adds it.)
  • Rebased koan/common-exception-base onto origin/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan/common-exception-base to origin
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@bdraco bdraco marked this pull request as ready for review May 15, 2026 00:46
@bdraco bdraco merged commit 7edf99f into main May 15, 2026
23 of 24 checks passed
@bdraco bdraco deleted the koan/common-exception-base branch May 15, 2026 00:46
bluetoothbot added a commit that referenced this pull request May 15, 2026
Five internal sanity-check raises used bare `Exception` / `RuntimeError`,
so they escaped `except DBusFastError` introduced in #634. Replace them
with a new `InternalError(RuntimeError, DBusFastError)` that keeps
backward compatibility (still a `RuntimeError`) while wiring into the
common base.

Sites: _private/address.py (unknown bus type), message_bus.py (missing
proxy object class, missing interface path), signature.py (missing
validator for token), _private/marshaller.py ("should not reach here").

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

exception types could use a common base class

3 participants