Skip to content

Commit

Permalink
Throw AttributeError in tp_getattro for dynamic classes
Browse files Browse the repository at this point in the history
Python api documentation indicates it should throw AttributeError on failure
  • Loading branch information
jhonabreul committed Dec 2, 2024
1 parent 360948f commit ae118ea
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 2 deletions.
45 changes: 44 additions & 1 deletion src/embed_tests/TestPropertyAccess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down Expand Up @@ -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<PythonException>(() => 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();
Expand Down
5 changes: 4 additions & 1 deletion src/runtime/Types/DynamicClassObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,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;
Expand Down

0 comments on commit ae118ea

Please sign in to comment.