pythonnet · lostmsu · Jan 27, 2021 · Jan 22, 2021 · Jan 26, 2021 · Jan 26, 2021
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,6 +31,7 @@ details about the cause of the failure
 -   `PyObject` now implements `IEnumerable<PyObject>` in addition to `IEnumerable`
 -   floating point values passed from Python are no longer silently truncated
 when .NET expects an integer [#1342][i1342]
+-   More specific error messages for method argument mismatch
 
 ### Fixed
 
@@ -49,6 +50,7 @@ when .NET expects an integer [#1342][i1342]
 -    Fixed objects returned by enumerating `PyObject` being disposed too soon
 -    Incorrectly using a non-generic type with type parameters now produces a helpful Python error instead of throwing NullReferenceException
 -   `import` may now raise errors with more detail than "No module named X"
+-    Providing an invalid type parameter to a generic type or method produces a helpful Python error
 
 ### Removed
 

diff --git a/src/clrmodule/ClrModule.cs b/src/clrmodule/ClrModule.cs
@@ -62,9 +62,9 @@ public static IntPtr PyInit_clr()
             pythonRuntime = Assembly.Load(pythonRuntimeName);
             DebugPrint("Success loading 'Python.Runtime' using standard binding rules.");
         }
-        catch (IOException)
+        catch (IOException ex)
         {
-            DebugPrint("'Python.Runtime' not found using standard binding rules.");
+            DebugPrint($"'Python.Runtime' not found using standard binding rules: {ex}");
             try
             {
                 // If the above fails for any reason, we fallback to attempting to load "Python.Runtime.dll"

diff --git a/src/domain_tests/test_domain_reload.py b/src/domain_tests/test_domain_reload.py
@@ -57,6 +57,7 @@ def test_property_visibility_change():
 def test_class_visibility_change():
     _run_test("class_visibility_change")
 
+@pytest.mark.skip(reason='FIXME: Domain reload fails when Python points to a .NET object which points back to Python objects')
 @pytest.mark.skipif(platform.system() == 'Darwin', reason='FIXME: macos can\'t find the python library')
 def test_method_parameters_change():
     _run_test("method_parameters_change")

diff --git a/src/embed_tests/TestPythonException.cs b/src/embed_tests/TestPythonException.cs
@@ -97,6 +97,7 @@ public void TestPythonExceptionFormatNormalized()
             try
             {
                 PythonEngine.Exec("a=b\n");
+                Assert.Fail("Exception should have been raised");
             }
             catch (PythonException ex)
             {
@@ -135,5 +136,14 @@ def __init__(self, val):
                 }
             }
         }
+
+        [Test]
+        public void TestPythonException_Normalize_ThrowsWhenErrorSet()
+        {
+            Exceptions.SetError(Exceptions.TypeError, "Error!");
+            var pythonException = new PythonException();
+            Exceptions.SetError(Exceptions.TypeError, "Another error");
+            Assert.Throws<InvalidOperationException>(() => pythonException.Normalize());
+        }
     }
 }
diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs
@@ -66,7 +66,16 @@ public virtual IntPtr type_subscript(IntPtr idx)
 
             if (target != null)
             {
-                Type t = target.MakeGenericType(types);
+                Type t;
+                try
+                {
+                    // MakeGenericType can throw ArgumentException
+                    t = target.MakeGenericType(types);
+                }
+                catch (ArgumentException e)
+                {
+                    return Exceptions.RaiseTypeError(e.Message);
+                }
                 ManagedType c = ClassManager.GetClass(t);
                 Runtime.XIncref(c.pyHandle);
                 return c.pyHandle;
@@ -263,14 +272,14 @@ public static IntPtr tp_iter(IntPtr ob)
         /// <summary>
         /// Standard __hash__ implementation for instances of reflected types.
         /// </summary>
-        public static IntPtr tp_hash(IntPtr ob)
+        public static nint tp_hash(IntPtr ob)
         {
             var co = GetManagedObject(ob) as CLRObject;
             if (co == null)
             {
                 return Exceptions.RaiseTypeError("unhashable type");
             }
-            return new IntPtr(co.inst.GetHashCode());
+            return co.inst.GetHashCode();
         }
 
 

diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs
@@ -303,6 +303,11 @@ internal static IntPtr ToPythonImplicit(object value)
         /// Return a managed object for the given Python object, taking funny
         /// byref types into account.
         /// </summary>
+        /// <param name="value">A Python object</param>
+        /// <param name="type">The desired managed type</param>
+        /// <param name="result">Receives the managed object</param>
+        /// <param name="setError">If true, call <c>Exceptions.SetError</c> with the reason for failure.</param>
+        /// <returns>True on success</returns>
         internal static bool ToManaged(IntPtr value, Type type,
             out object result, bool setError)
         {
@@ -341,7 +346,10 @@ internal static bool ToManagedValue(IntPtr value, Type obType,
                         result = tmp;
                         return true;
                     }
-                    Exceptions.SetError(Exceptions.TypeError, $"value cannot be converted to {obType}");
+                    if (setError)
+                    {
+                        Exceptions.SetError(Exceptions.TypeError, $"value cannot be converted to {obType}");
+                    }
                     return false;
                 }
                 if (mt is ClassBase)
@@ -376,6 +384,15 @@ internal static bool ToManagedValue(IntPtr value, Type obType,
                 obType = obType.GetGenericArguments()[0];
             }
 
+            if (obType.ContainsGenericParameters)
+            {
+                if (setError)
+                {
+                    Exceptions.SetError(Exceptions.TypeError, $"Cannot create an instance of the open generic type {obType}");
+                }
+                return false;
+            }
+
             if (obType.IsArray)
             {
                 return ToArray(value, obType, out result, setError);
@@ -777,7 +794,7 @@ private static void SetConversionError(IntPtr value, Type target)
             IntPtr ob = Runtime.PyObject_Repr(value);
             string src = Runtime.GetManagedString(ob);
             Runtime.XDecref(ob);
-            Exceptions.SetError(Exceptions.TypeError, $"Cannot convert {src} to {target}");
+            Exceptions.RaiseTypeError($"Cannot convert {src} to {target}");
         }
 
 
@@ -791,32 +808,58 @@ private static bool ToArray(IntPtr value, Type obType, out object result, bool s
             Type elementType = obType.GetElementType();
             result = null;
 
-            bool IsSeqObj = Runtime.PySequence_Check(value);
-            var len = IsSeqObj ? Runtime.PySequence_Size(value) : -1;
-
             IntPtr IterObject = Runtime.PyObject_GetIter(value);
-
-            if(IterObject==IntPtr.Zero) {
+            if (IterObject == IntPtr.Zero)
+            {
                 if (setError)
                 {
                     SetConversionError(value, obType);
                 }
+                else
+                {
+                    // PyObject_GetIter will have set an error
+                    Exceptions.Clear();
+                }
                 return false;
             }
 
-            Array items;
+            IList list;
+            try
+            {
+                // MakeGenericType can throw because elementType may not be a valid generic argument even though elementType[] is a valid array type.
+                // For example, if elementType is a pointer type.
+                // See https://docs.microsoft.com/en-us/dotnet/api/system.type.makegenerictype#System_Type_MakeGenericType_System_Type
+                var constructedListType = typeof(List<>).MakeGenericType(elementType);
+                bool IsSeqObj = Runtime.PySequence_Check(value);
+                if (IsSeqObj)
+                {
+                    var len = Runtime.PySequence_Size(value);
+                    list = (IList)Activator.CreateInstance(constructedListType, new Object[] { (int)len });
+                }
+                else
+                {
+                    // CreateInstance can throw even if MakeGenericType succeeded.
+                    // See https://docs.microsoft.com/en-us/dotnet/api/system.activator.createinstance#System_Activator_CreateInstance_System_Type_
+                    list = (IList)Activator.CreateInstance(constructedListType);
+                }
+            }
+            catch (Exception e)
+            {
+                if (setError)
+                {
+                    Exceptions.SetError(e);
+                    SetConversionError(value, obType);
+                }
+                return false;
+            }
 
-            var listType = typeof(List<>);
-            var constructedListType = listType.MakeGenericType(elementType);
-            IList list = IsSeqObj ? (IList) Activator.CreateInstance(constructedListType, new Object[] {(int) len}) :
-                                        (IList) Activator.CreateInstance(constructedListType);
             IntPtr item;
 
             while ((item = Runtime.PyIter_Next(IterObject)) != IntPtr.Zero)
             {
-                object obj = null;
+                object obj;
 
-                if (!Converter.ToManaged(item, elementType, out obj, true))
+                if (!Converter.ToManaged(item, elementType, out obj, setError))
                 {
                     Runtime.XDecref(item);
                     return false;
@@ -827,7 +870,7 @@ private static bool ToArray(IntPtr value, Type obType, out object result, bool s
             }
             Runtime.XDecref(IterObject);
 
-            items = Array.CreateInstance(elementType, list.Count);
+            Array items = Array.CreateInstance(elementType, list.Count);
             list.CopyTo(items, 0);
 
             result = items;

diff --git a/src/runtime/eventbinding.cs b/src/runtime/eventbinding.cs
@@ -68,35 +68,27 @@ public static IntPtr nb_inplace_subtract(IntPtr ob, IntPtr arg)
         /// <summary>
         /// EventBinding  __hash__ implementation.
         /// </summary>
-        public static IntPtr tp_hash(IntPtr ob)
+        public static nint tp_hash(IntPtr ob)
         {
             var self = (EventBinding)GetManagedObject(ob);
-            long x = 0;
-            long y = 0;
+            nint x = 0;
 
             if (self.target != IntPtr.Zero)
             {
-                x = Runtime.PyObject_Hash(self.target).ToInt64();
+                x = Runtime.PyObject_Hash(self.target);
                 if (x == -1)
                 {
-                    return new IntPtr(-1);
+                    return x;
                 }
             }
 
-            y = Runtime.PyObject_Hash(self.e.pyHandle).ToInt64();
+            nint y = Runtime.PyObject_Hash(self.e.pyHandle);
             if (y == -1)
             {
-                return new IntPtr(-1);
+                return y;
             }
 
-            x ^= y;
-
-            if (x == -1)
-            {
-                x = -1;
-            }
-
-            return new IntPtr(x);
+            return x ^ y;
         }
 
 

diff --git a/src/runtime/eventobject.cs b/src/runtime/eventobject.cs
@@ -72,17 +72,22 @@ internal bool AddEventHandler(IntPtr target, IntPtr handler)
         /// </summary>
         internal bool RemoveEventHandler(IntPtr target, IntPtr handler)
         {
+            if (reg == null)
+            {
+                Exceptions.SetError(Exceptions.ValueError, "unknown event handler");
+                return false;
+            }
+
             object obj = null;
             if (target != IntPtr.Zero)
             {
                 var co = (CLRObject)GetManagedObject(target);
                 obj = co.inst;
             }
 
-            IntPtr hash = Runtime.PyObject_Hash(handler);
-            if (Exceptions.ErrorOccurred() || reg == null)
+            nint hash = Runtime.PyObject_Hash(handler);
+            if (hash == -1 && Exceptions.ErrorOccurred())
             {
-                Exceptions.SetError(Exceptions.ValueError, "unknown event handler");
                 return false;
             }
 

diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Reflection;
 using System.Runtime.InteropServices;
+using System.Text;
 
 namespace Python.Runtime
 {
@@ -71,14 +72,16 @@ internal static Exception ToException(IntPtr ob)
                 return Exceptions.RaiseTypeError("invalid object");
             }
 
-            string message = string.Empty;
-            if (e.Message != string.Empty)
+            string message = e.ToString();
+            string fullTypeName = e.GetType().FullName;
+            string prefix = fullTypeName + ": ";
+            if (message.StartsWith(prefix))
             {
-                message = e.Message;
+                message = message.Substring(prefix.Length);
             }
-            if (!string.IsNullOrEmpty(e.StackTrace))
+            else if (message.StartsWith(fullTypeName))
             {
-                message = message + "\n" + e.StackTrace;
+                message = message.Substring(fullTypeName.Length);
             }
             return Runtime.PyUnicode_FromString(message);
         }
@@ -181,6 +184,7 @@ internal static void SetArgsAndCause(IntPtr ob)
 
             if (e.InnerException != null)
             {
+                // Note: For an AggregateException, InnerException is only the first of the InnerExceptions.
                 IntPtr cause = CLRObject.GetInstHandle(e.InnerException);
                 Marshal.WriteIntPtr(ob, ExceptionOffset.cause, cause);
             }
@@ -290,6 +294,20 @@ public static void SetError(Exception e)
             Runtime.XDecref(op);
         }
 
+        /// <summary>
+        /// When called after SetError, sets the cause of the error.
+        /// </summary>
+        /// <param name="cause">The cause of the current error</param>
+        public static void SetCause(PythonException cause)
+        {
+            var currentException = new PythonException();
+            currentException.Normalize();
+            cause.Normalize();
+            Runtime.XIncref(cause.PyValue);
+            Runtime.PyException_SetCause(currentException.PyValue, cause.PyValue);
+            currentException.Restore();
+        }
+
         /// <summary>
         /// ErrorOccurred Method
         /// </summary>
@@ -368,17 +386,31 @@ public static void deprecation(string message)
         // Internal helper methods for common error handling scenarios.
         //====================================================================
 
+        /// <summary>
+        /// Raises a TypeError exception and attaches any existing exception as its cause.
+        /// </summary>
+        /// <param name="message">The exception message</param>
+        /// <returns><c>IntPtr.Zero</c></returns>
         internal static IntPtr RaiseTypeError(string message)
         {
+            PythonException previousException = null;
+            if (ErrorOccurred())
+            {
+                previousException = new PythonException();
+            }
             Exceptions.SetError(Exceptions.TypeError, message);
+            if (previousException != null)
+            {
+                SetCause(previousException);
+            }
             return IntPtr.Zero;
         }
 
         // 2010-11-16: Arranged in python (2.6 & 2.7) source header file order
         /* Predefined exceptions are
-           puplic static variables on the Exceptions class filled in from
+           public static variables on the Exceptions class filled in from
            the python class using reflection in Initialize() looked up by
-		   name, not posistion. */
+		   name, not position. */
         public static IntPtr BaseException;
         public static IntPtr Exception;
         public static IntPtr StopIteration;