Propagate exceptions from TryGetMember in tp_getattro_dlr_proxy (#2718)

greateggsgreg · filmor · commit 8b12b56d13e3 · 2026-05-15T21:57:36.000+02:00
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.
diff --git a/src/runtime/TypeManager.cs b/src/runtime/TypeManager.cs
@@ -123,8 +123,13 @@ public static NewReference tp_getattro_dlr_proxy(BorrowedReference ob, BorrowedR
             {
                 resolved = dynamicMemberAccessor.TryGetMember(dynamicObject, memberName, out value);
             }
-            catch
+            catch (Exception e)
             {
+                // Avoid wrapping the CLR exception via Converter.ToPython here: that would trigger
+                // CLR type initialisation which can re-enter this slot on the same live object,
+                // causing infinite recursion. A plain RuntimeError with the message is safe.
+                Runtime.PyErr_Clear();
+                Exceptions.SetError(Exceptions.RuntimeError, e.Message);
                 return default;
             }
 
diff --git a/src/testing/dlrtest.cs b/src/testing/dlrtest.cs
@@ -62,6 +62,12 @@ public override bool TrySetMember(SetMemberBinder binder, object value)
     }
 }
 
+public class ThrowingGetDynamicObject : DynamicStorageObject
+{
+    public override bool TryGetMember(GetMemberBinder binder, out object result)
+        => throw new InvalidOperationException($"TryGetMember failed for '{binder.Name}'");
+}
+
 public class ThrowingSetDynamicObject : DynamicStorageObject
 {
     public override bool TrySetMember(SetMemberBinder binder, object value)
diff --git a/tests/test_dynamic.py b/tests/test_dynamic.py
@@ -8,6 +8,7 @@
 from Python.Test import RejectingDeleteDynamicObject
 from Python.Test import RejectingSetDynamicObject
 from Python.Test import ThrowingDeleteDynamicObject
+from Python.Test import ThrowingGetDynamicObject
 from Python.Test import ThrowingSetDynamicObject
 
 
@@ -186,6 +187,14 @@ def test_trysetmember_false_raises_attributeerror_instead_of_silent_python_setat
     assert not hasattr(obj, "typoed_name")
 
 
+def test_trygetmember_exception_is_raised_in_python():
+    obj = ThrowingGetDynamicObject()
+    obj.AddDynamicMember("any_key", 1)
+
+    with pytest.raises(Exception, match="TryGetMember failed for 'any_key'"):
+        _ = obj.any_key
+
+
 def test_trysetmember_exception_is_raised_in_python():
     obj = ThrowingSetDynamicObject()
 

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)`