diff --git a/docs/changes/newsfragments/5518.improved b/docs/changes/newsfragments/5518.improved new file mode 100644 index 000000000000..5b523ae6f002 --- /dev/null +++ b/docs/changes/newsfragments/5518.improved @@ -0,0 +1,6 @@ +Errors raised inside a ``@property`` getter on a subclass of +:class:`~qcodes.utils.DelegateAttributes` (such as :class:`~qcodes.instrument.Instrument`) +are now surfaced with their original traceback, instead of being masked by a +generic ``AttributeError: ... object and its delegates have no attribute ...`` +message. The underlying cause of the failure is now visible in the traceback, +making misbehaving properties much easier to debug. diff --git a/src/qcodes/utils/attribute_helpers.py b/src/qcodes/utils/attribute_helpers.py index b2b32a27fc3a..a75829d7ffc0 100644 --- a/src/qcodes/utils/attribute_helpers.py +++ b/src/qcodes/utils/attribute_helpers.py @@ -1,3 +1,4 @@ +import inspect from contextlib import contextmanager from typing import TYPE_CHECKING, Any, ClassVar @@ -38,6 +39,17 @@ class DelegateAttributes: """ 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)) + if key in self.omit_delegate_attrs: raise AttributeError( f"'{self.__class__.__name__}' does not delegate attribute {key}" diff --git a/tests/helpers/test_delegate_attribues.py b/tests/helpers/test_delegate_attribues.py index 084edd32e1af..e77864348000 100644 --- a/tests/helpers/test_delegate_attribues.py +++ b/tests/helpers/test_delegate_attribues.py @@ -180,3 +180,58 @@ class ToBoth(DelegateAttributes): # all appropriate items are in dir() exactly once for attr in ["rock", "paper", "scissors", "year", "water"]: assert dir(tb).count(attr) == 1 + + +def test_faulty_property_surfaces_original_attribute_error() -> None: + """A ``@property`` whose getter raises ``AttributeError`` should surface + the *inner* error instead of being masked by the generic + ``DelegateAttributes.__getattr__`` fallback. + """ + + class WithFaultyProperty(DelegateAttributes): + @property + def prop(self) -> int: + # ``missing`` does not exist; accessing it raises AttributeError. + # Before this fix, Python fell through to ``__getattr__`` which + # reported "no attribute 'prop'" and hid the real cause. + return self.missing # type: ignore[attr-defined] + + obj = WithFaultyProperty() + with pytest.raises(AttributeError, match="missing"): + obj.prop + + +def test_faulty_property_preserves_inner_traceback() -> None: + """The traceback of the surfaced error must include the line inside the + property's getter that actually raised, so downstream debugging is + possible. + """ + + class WithFaultyProperty(DelegateAttributes): + @property + def prop(self) -> int: + raise AttributeError("specific underlying failure") + + obj = WithFaultyProperty() + with pytest.raises(AttributeError, match="specific underlying failure"): + obj.prop + + +def test_working_property_still_returns_value() -> None: + """Re-invocation of a descriptor from ``__getattr__`` must never happen for + a well-behaved ``@property`` — normal attribute lookup should resolve the + value directly without touching ``__getattr__`` at all. + """ + + call_count = 0 + + class WithWorkingProperty(DelegateAttributes): + @property + def prop(self) -> int: + nonlocal call_count + call_count += 1 + return 42 + + obj = WithWorkingProperty() + assert obj.prop == 42 + assert call_count == 1