diff --git a/src/embed_tests/TestPropertyAccess.cs b/src/embed_tests/TestPropertyAccess.cs index 54acc08f0..8dba383d6 100644 --- a/src/embed_tests/TestPropertyAccess.cs +++ b/src/embed_tests/TestPropertyAccess.cs @@ -1420,6 +1420,16 @@ public override bool TryGetMember(GetMemberBinder binder, out object result) } return true; } + + public override bool TrySetMember(SetMemberBinder binder, object value) + { + if (value is PyObject pyValue && PyString.IsStringType(pyValue)) + { + throw new InvalidOperationException("Cannot set string value"); + } + + return base.TrySetMember(binder, value); + } } [Test] @@ -1430,7 +1440,6 @@ public void TestHasAttrShouldNotThrowIfAttributeIsNotPresentForDynamicClassObjec dynamic module = PyModule.FromString("TestHasAttrShouldNotThrowIfAttributeIsNotPresentForDynamicClassObjects", @" from clr import AddReference AddReference(""Python.EmbeddingTest"") -AddReference(""System"") from Python.EmbeddingTest import TestPropertyAccess @@ -1466,6 +1475,40 @@ def has_attribute(obj, attribute): Assert.IsFalse(hasAttributeResult); } + [Test] + public void TestSetAttrShouldThrowPythonExceptionOnFailure() + { + using var _ = Py.GIL(); + + dynamic module = PyModule.FromString("TestHasAttrShouldNotThrowIfAttributeIsNotPresentForDynamicClassObjects", @" +from clr import AddReference +AddReference(""Python.EmbeddingTest"") + +from Python.EmbeddingTest import TestPropertyAccess + +class TestDynamicClass(TestPropertyAccess.ThrowingDynamicFixture): + pass + +def set_attribute(obj): + obj.int_attribute = 11 + +def set_string_attribute(obj): + obj.string_attribute = 'string' +"); + + dynamic fixture = module.GetAttr("TestDynamicClass")(); + + dynamic setAttribute = module.GetAttr("set_attribute"); + Assert.DoesNotThrow(() => setAttribute(fixture)); + + dynamic setStringAttribute = module.GetAttr("set_string_attribute"); + var exception = Assert.Throws(() => setStringAttribute(fixture)); + Assert.AreEqual("Cannot set string value", exception.Message); + + using var expectedExceptionType = new PyType(Exceptions.AttributeError); + Assert.AreEqual(expectedExceptionType, exception.Type); + } + public interface IModel { void InvokeModel(); diff --git a/src/runtime/Types/DynamicClassObject.cs b/src/runtime/Types/DynamicClassObject.cs index 94e94b568..cb6fd5650 100644 --- a/src/runtime/Types/DynamicClassObject.cs +++ b/src/runtime/Types/DynamicClassObject.cs @@ -1,7 +1,5 @@ using System; using System.Collections.Generic; -using System.Dynamic; -using System.Reflection; using System.Runtime.CompilerServices; using RuntimeBinder = Microsoft.CSharp.RuntimeBinder; @@ -94,6 +92,7 @@ public static NewReference tp_getattro(BorrowedReference ob, BorrowedReference k // e.g hasattr uses this method to check if the attribute exists. If we throw anything other than AttributeError, // hasattr will throw instead of catching and returning False. Exceptions.SetError(Exceptions.AttributeError, exception.Message); + return default; } } @@ -120,7 +119,10 @@ public static int tp_setattro(BorrowedReference ob, BorrowedReference key, Borro // Catch C# exceptions and raise them as Python exceptions. catch (Exception exception) { - Exceptions.SetError(exception); + // tp_setattro should call PyObject_GenericSetAttr (which we already did) + // which must throw AttributeError on failure and return -1 (see https://docs.python.org/3/c-api/object.html#c.PyObject_GenericSetAttr) + Exceptions.SetError(Exceptions.AttributeError, exception.Message); + return -1; } return 0;