safer GetAttr(name, default)

lostmsu · lostmsu · commit 5d0d01abe659 · 2021-10-01T13:07:32.000-07:00
fixes #1036
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -71,6 +71,7 @@ See [Mixins/collections.py](src/runtime/Mixins/collections.py).
 -   BREAKING: When trying to convert Python `int` to `System.Object`, result will
 be of type `PyInt` instead of `System.Int32` due to possible loss of information.
 Python `float` will continue to be converted to `System.Double`.
+-   BREAKING: `PyObject.GetAttr(name, default)` now only ignores `AttributeError` (previously ignored all exceptions).
 -   BREAKING: `PyObject` no longer implements `IEnumerable<PyObject>`.
 Instead, `PyIterable` does that.
 
diff --git a/src/embed_tests/TestPyObject.cs b/src/embed_tests/TestPyObject.cs
@@ -79,5 +79,26 @@ public void UnaryMinus_ThrowsOnBadType()
             var error = Assert.Throws<PythonException>(() => list = -list);
             Assert.AreEqual("TypeError", error.Type.Name);
         }
+
+        [Test]
+        [Obsolete]
+        public void GetAttrDefault_IgnoresAttributeErrorOnly()
+        {
+            var ob = new PyObjectTestMethods().ToPython();
+            using var fallback = new PyList();
+            var attrErrResult = ob.GetAttr(nameof(PyObjectTestMethods.RaisesAttributeError), fallback);
+            Assert.IsTrue(PythonReferenceComparer.Instance.Equals(fallback, attrErrResult));
+
+            var typeErrResult = Assert.Throws<PythonException>(
+                () => ob.GetAttr(nameof(PyObjectTestMethods.RaisesTypeError), fallback)
+            );
+            Assert.AreEqual(Exceptions.TypeError, typeErrResult.Type.Handle);
+        }
+    }
+
+    public class PyObjectTestMethods
+    {
+        public string RaisesAttributeError => throw new PythonException(new PyType(new BorrowedReference(Exceptions.AttributeError)), value: null, traceback: null);
+        public string RaisesTypeError => throw new PythonException(new PyType(new BorrowedReference(Exceptions.TypeError)), value: null, traceback: null);
     }
 }
diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs
@@ -309,21 +309,36 @@ public PyObject GetAttr(string name)
 
 
         /// <summary>
-        /// GetAttr Method. Returns fallback value if getting attribute fails for any reason.
+        /// Returns the named attribute of the Python object, or the given
+        /// default object if the attribute access throws AttributeError.
         /// </summary>
         /// <remarks>
-        /// Returns the named attribute of the Python object, or the given
-        /// default object if the attribute access fails.
+        /// This method ignores any AttrubiteError(s), even ones
+        /// not raised due to missing requested attribute.
+        ///
+        /// For example, if attribute getter calls other Python code, and
+        /// that code happens to cause AttributeError elsewhere, it will be ignored
+        /// and <paramref name="_default"/> value will be returned instead.
         /// </remarks>
+        /// <param name="name">Name of the attribute.</param>
+        /// <param name="_default">The object to return on AttributeError.</param>
+        [Obsolete("See remarks")]
         public PyObject GetAttr(string name, PyObject _default)
         {
             if (name == null) throw new ArgumentNullException(nameof(name));
 
             IntPtr op = Runtime.PyObject_GetAttrString(obj, name);
             if (op == IntPtr.Zero)
             {
-                Runtime.PyErr_Clear();
-                return _default;
+                if (Exceptions.ExceptionMatches(Exceptions.AttributeError))
+                {
+                    Runtime.PyErr_Clear();
+                    return _default;
+                }
+                else
+                {
+                    throw PythonException.ThrowLastAsClrException();
+                }
             }
             return new PyObject(op);
         }
@@ -351,22 +366,36 @@ public PyObject GetAttr(PyObject name)
 
 
         /// <summary>
-        /// GetAttr Method
+        /// Returns the named attribute of the Python object, or the given
+        /// default object if the attribute access throws AttributeError.
         /// </summary>
         /// <remarks>
-        /// Returns the named attribute of the Python object, or the given
-        /// default object if the attribute access fails. The name argument
-        /// is a PyObject wrapping a Python string or unicode object.
+        /// This method ignores any AttrubiteError(s), even ones
+        /// not raised due to missing requested attribute.
+        ///
+        /// For example, if attribute getter calls other Python code, and
+        /// that code happens to cause AttributeError elsewhere, it will be ignored
+        /// and <paramref name="_default"/> value will be returned instead.
         /// </remarks>
+        /// <param name="name">Name of the attribute. Must be of Python type 'str'.</param>
+        /// <param name="_default">The object to return on AttributeError.</param>
+        [Obsolete("See remarks")]
         public PyObject GetAttr(PyObject name, PyObject _default)
         {
             if (name == null) throw new ArgumentNullException(nameof(name));
 
             IntPtr op = Runtime.PyObject_GetAttr(obj, name.obj);
             if (op == IntPtr.Zero)
             {
-                Runtime.PyErr_Clear();
-                return _default;
+                if (Exceptions.ExceptionMatches(Exceptions.AttributeError))
+                {
+                    Runtime.PyErr_Clear();
+                    return _default;
+                }
+                else
+                {
+                    throw PythonException.ThrowLastAsClrException();
+                }
             }
             return new PyObject(op);
         }
