fix(utils): surface original errors from faulty instrument properties#8039
fix(utils): surface original errors from faulty instrument properties#8039officialasishkumar wants to merge 1 commit intomicrosoft:mainfrom
Conversation
``DelegateAttributes.__getattr__`` is only reached when normal attribute lookup exhausted. For a ``@property`` that raises ``AttributeError`` from its getter, Python treats the getter failure as a missing attribute and falls through to ``__getattr__``, where the generic "object and its delegates have no attribute X" error masked the real cause and traceback. Before calling the delegate fallback, check the class for a data descriptor (``__get__`` plus ``__set__``/``__delete__``, i.e. properties and other data descriptors) registered under the requested key. If one exists, re-invoke it directly so the descriptor's original error propagates with its traceback intact. Normal attribute lookups are unaffected because they resolve before ``__getattr__`` is ever called. Adds regression tests for three scenarios: - faulty property whose getter raises because of a missing attribute - faulty property whose getter raises ``AttributeError`` with a custom message - working property still returns its value and is invoked exactly once Closes microsoft#5518 Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a Python attribute-resolution edge case in DelegateAttributes (used by Instrument and others) where AttributeError raised inside a @property getter gets masked by the delegation fallback, making debugging difficult.
Changes:
- Update
DelegateAttributes.__getattr__to detect class-level data descriptors and re-invoke them so the underlyingAttributeErroris surfaced. - Add regression tests for faulty properties and a “working property” sanity check.
- Add a Towncrier newsfragment documenting the behavior change.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/qcodes/utils/attribute_helpers.py |
Adds descriptor detection in __getattr__ to avoid masking AttributeError from property getters. |
tests/helpers/test_delegate_attribues.py |
Adds tests covering faulty-property behavior and a working-property regression check. |
docs/changes/newsfragments/5518.improved |
Documents improved error traceability for failing properties on DelegateAttributes subclasses. |
| with pytest.raises(AttributeError, match="specific underlying failure"): | ||
| obj.prop | ||
|
|
||
|
|
There was a problem hiding this comment.
test_faulty_property_preserves_inner_traceback claims to validate that the traceback includes the line inside the property getter, but the test currently only matches the exception message. Please assert on excinfo.traceback (e.g., that a frame/function corresponding to the property getter is present, or that the failing line is included) so this test actually guards the intended behavior.
| with pytest.raises(AttributeError, match="specific underlying failure"): | |
| obj.prop | |
| with pytest.raises( | |
| AttributeError, match="specific underlying failure" | |
| ) as excinfo: | |
| obj.prop | |
| assert any(entry.name == "prop" for entry in excinfo.traceback) |
| def __getattr__(self, key: str) -> Any: | ||
| # If ``key`` names a data descriptor (e.g. ``@property``) on the class, | ||
| # the only way Python reaches ``__getattr__`` is because the descriptor's | ||
| # own ``__get__`` raised ``AttributeError``. Re-invoke the descriptor so | ||
| # the underlying error surfaces with its original traceback, instead of | ||
| # being masked by the generic "has no attribute" error below. | ||
| descriptor = inspect.getattr_static(type(self), key, None) | ||
| if descriptor is not None and hasattr(descriptor, "__get__") and ( | ||
| hasattr(descriptor, "__set__") or hasattr(descriptor, "__delete__") | ||
| ): | ||
| return descriptor.__get__(self, type(self)) |
There was a problem hiding this comment.
DelegateAttributes.__getattr__ is on a hot path for instruments (e.g., resolving parameters/functions/submodules). Adding inspect.getattr_static on every delegated attribute lookup may introduce measurable overhead. Consider a cheaper descriptor lookup (e.g., walking type(self).__mro__ and checking __dict__ directly) and/or caching per-class descriptor results to avoid repeated introspection.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8039 +/- ##
=======================================
Coverage 70.63% 70.64%
=======================================
Files 333 333
Lines 32330 32334 +4
=======================================
+ Hits 22837 22841 +4
Misses 9493 9493 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Closes #5518.
When a
@propertydefined on a subclass ofDelegateAttributes— such as anyqcodes.Instrument— raisesAttributeErrorfrom its getter, Python treats the getter failure as a missing attribute and falls through toDelegateAttributes.__getattr__. The generic "object and its delegates have no attribute X" error then completely masks the real cause and its traceback, which is extremely cumbersome to debug.Reproducer (before this PR)
After this PR
The traceback walks through the property getter and points at
self.bar, with the underlying "no attribute 'bar'" error preserved.Approach
__getattr__is only called when normal attribute lookup has been exhausted, which for a data descriptor (@property) can only mean the descriptor's own__get__raisedAttributeError. Before running the delegate fallback, the method now checks the class for a data descriptor under the requested key (__get__plus__set__or__delete__) and, if found, re-invokes it directly so the descriptor's original error propagates with its traceback intact.Normal lookups of working properties are untouched because Python resolves them before
__getattr__runs — the testtest_working_property_still_returns_valuepins that the descriptor is invoked exactly once.Changes
src/qcodes/utils/attribute_helpers.py: detect data descriptors and re-invoke them to surface the original error.tests/helpers/test_delegate_attribues.py: three regression tests covering the faulty-property, custom-error, and happy-path cases.Test plan
pytest tests/helpers/test_delegate_attribues.py— 8 passed locally (5 existing + 3 new).pytest tests/test_instrument.py tests/helpers/ tests/test_channels.py— 118 passed.pytest tests/parameter/— 872 passed.self.bar.