Commit 789726c
Propagate exceptions from TryGetMember in tp_getattro_dlr_proxy (#2718)
The dynamic getter swallowed any exception from TryGetMember and
returned default to Python with the prior AttributeError still set,
so user code observed a misleading AttributeError instead of the real
failure.
Set a Python exception in the catch arm. We use RuntimeError with the
message string rather than Converter.ToPython(e) because wrapping the
CLR exception object can trigger type initialisation that re-enters
this same slot on the live dynamic object, producing infinite
recursion.
Mirrors the symmetry already present in the setter (#2706 review,
@lostmsu) and adds a regression test alongside the existing
ThrowingSetDynamicObject coverage.1 parent 85878d8 commit 789726c
3 files changed
Lines changed: 21 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -123,8 +123,13 @@ public static NewReference tp_getattro_dlr_proxy(BorrowedReference ob, BorrowedR | |||
| 123 | 123 | { | |
| 124 | 124 | resolved = dynamicMemberAccessor.TryGetMember(dynamicObject, memberName, out value); | |
| 125 | 125 | } | |
| 126 | - catch | ||
| 126 | + catch (Exception e) | ||
| 127 | 127 | { | |
| 128 | + // Avoid wrapping the CLR exception via Converter.ToPython here: that would trigger | ||
| 129 | + // CLR type initialisation which can re-enter this slot on the same live object, | ||
| 130 | + // causing infinite recursion. A plain RuntimeError with the message is safe. | ||
| 131 | + Runtime.PyErr_Clear(); | ||
| 132 | + Exceptions.SetError(Exceptions.RuntimeError, e.Message); | ||
| 128 | 133 | return default; | |
| 129 | 134 | } | |
| 130 | 135 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -62,6 +62,12 @@ public override bool TrySetMember(SetMemberBinder binder, object value) | |||
| 62 | 62 | } | |
| 63 | 63 | } | |
| 64 | 64 | ||
| 65 | + public class ThrowingGetDynamicObject : DynamicStorageObject | ||
| 66 | + { | ||
| 67 | + public override bool TryGetMember(GetMemberBinder binder, out object result) | ||
| 68 | + => throw new InvalidOperationException($"TryGetMember failed for '{binder.Name}'"); | ||
| 69 | + } | ||
| 70 | + | ||
| 65 | 71 | public class ThrowingSetDynamicObject : DynamicStorageObject | |
| 66 | 72 | { | |
| 67 | 73 | public override bool TrySetMember(SetMemberBinder binder, object value) | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -8,6 +8,7 @@ | |||
| 8 | 8 | from Python.Test import RejectingDeleteDynamicObject | |
| 9 | 9 | from Python.Test import RejectingSetDynamicObject | |
| 10 | 10 | from Python.Test import ThrowingDeleteDynamicObject | |
| 11 | + from Python.Test import ThrowingGetDynamicObject | ||
| 11 | 12 | from Python.Test import ThrowingSetDynamicObject | |
| 12 | 13 | ||
| 13 | 14 | ||
@@ -186,6 +187,14 @@ def test_trysetmember_false_raises_attributeerror_instead_of_silent_python_setat | |||
| 186 | 187 | assert not hasattr(obj, "typoed_name") | |
| 187 | 188 | ||
| 188 | 189 | ||
| 190 | + def test_trygetmember_exception_is_raised_in_python(): | ||
| 191 | + obj = ThrowingGetDynamicObject() | ||
| 192 | + obj.AddDynamicMember("any_key", 1) | ||
| 193 | + | ||
| 194 | + with pytest.raises(Exception, match="TryGetMember failed for 'any_key'"): | ||
| 195 | + _ = obj.any_key | ||
| 196 | + | ||
| 197 | + | ||
| 189 | 198 | def test_trysetmember_exception_is_raised_in_python(): | |
| 190 | 199 | obj = ThrowingSetDynamicObject() | |
| 191 | 200 | ||
0 commit comments