Skip to content

Commit 716c98d

Browse files
tminkatminka
committed
Better error messages for method argument mismatch, Python to managed value conversion, and other problems
Converter.ToManaged obeys setError consistently PyObject_Hash and tp_hash return nint MakeGenericType and MakeGenericMethod have try/catch RaiseTypeError sets __cause__ instead of changing the message string
1 parent 32fdc9c commit 716c98d

20 files changed

+299
-83
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ details about the cause of the failure
3131
- `PyObject` now implements `IEnumerable<PyObject>` in addition to `IEnumerable`
3232
- floating point values passed from Python are no longer silently truncated
3333
when .NET expects an integer [#1342][i1342]
34+
- More specific error messages for method argument mismatch
3435

3536
### Fixed
3637

@@ -49,6 +50,7 @@ when .NET expects an integer [#1342][i1342]
4950
- Fixed objects returned by enumerating `PyObject` being disposed too soon
5051
- Incorrectly using a non-generic type with type parameters now produces a helpful Python error instead of throwing NullReferenceException
5152
- `import` may now raise errors with more detail than "No module named X"
53+
- Providing an invalid type parameter to a generic type or method produces a helpful Python error
5254

5355
### Removed
5456

‎src/clrmodule/ClrModule.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ public static IntPtr PyInit_clr()
6262
pythonRuntime = Assembly.Load(pythonRuntimeName);
6363
DebugPrint("Success loading 'Python.Runtime' using standard binding rules.");
6464
}
65-
catch (IOException)
65+
catch (IOException ex)
6666
{
67-
DebugPrint("'Python.Runtime' not found using standard binding rules.");
67+
DebugPrint($"'Python.Runtime' not found using standard binding rules: {ex}");
6868
try
6969
{
7070
// If the above fails for any reason, we fallback to attempting to load "Python.Runtime.dll"

‎src/runtime/classbase.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,16 @@ public virtual IntPtr type_subscript(IntPtr idx)
6666

6767
if (target != null)
6868
{
69-
Type t = target.MakeGenericType(types);
69+
Type t;
70+
try
71+
{
72+
// MakeGenericType can throw ArgumentException
73+
t = target.MakeGenericType(types);
74+
}
75+
catch (ArgumentException e)
76+
{
77+
return Exceptions.RaiseTypeError(e.Message);
78+
}
7079
ManagedType c = ClassManager.GetClass(t);
7180
Runtime.XIncref(c.pyHandle);
7281
return c.pyHandle;
@@ -263,14 +272,14 @@ public static IntPtr tp_iter(IntPtr ob)
263272
/// <summary>
264273
/// Standard __hash__ implementation for instances of reflected types.
265274
/// </summary>
266-
public static IntPtr tp_hash(IntPtr ob)
275+
public static nint tp_hash(IntPtr ob)
267276
{
268277
var co = GetManagedObject(ob) as CLRObject;
269278
if (co == null)
270279
{
271280
return Exceptions.RaiseTypeError("unhashable type");
272281
}
273-
return new IntPtr(co.inst.GetHashCode());
282+
return co.inst.GetHashCode();
274283
}
275284

276285

‎src/runtime/converter.cs

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,11 @@ internal static IntPtr ToPythonImplicit(object value)
303303
/// Return a managed object for the given Python object, taking funny
304304
/// byref types into account.
305305
/// </summary>
306+
/// <param name="value">A Python object</param>
307+
/// <param name="type">The desired managed type</param>
308+
/// <param name="result">Receives the managed object</param>
309+
/// <param name="setError">If true, call <c>Exceptions.SetError</c> with the reason for failure.</param>
310+
/// <returns>True on success</returns>
306311
internal static bool ToManaged(IntPtr value, Type type,
307312
out object result, bool setError)
308313
{
@@ -341,7 +346,10 @@ internal static bool ToManagedValue(IntPtr value, Type obType,
341346
result = tmp;
342347
return true;
343348
}
344-
Exceptions.SetError(Exceptions.TypeError, $"value cannot be converted to {obType}");
349+
if (setError)
350+
{
351+
Exceptions.SetError(Exceptions.TypeError, $"value cannot be converted to {obType}");
352+
}
345353
return false;
346354
}
347355
if (mt is ClassBase)
@@ -376,6 +384,15 @@ internal static bool ToManagedValue(IntPtr value, Type obType,
376384
obType = obType.GetGenericArguments()[0];
377385
}
378386

387+
if (obType.ContainsGenericParameters)
388+
{
389+
if (setError)
390+
{
391+
Exceptions.SetError(Exceptions.TypeError, $"Cannot create an instance of the open generic type {obType}");
392+
}
393+
return false;
394+
}
395+
379396
if (obType.IsArray)
380397
{
381398
return ToArray(value, obType, out result, setError);
@@ -777,7 +794,7 @@ private static void SetConversionError(IntPtr value, Type target)
777794
IntPtr ob = Runtime.PyObject_Repr(value);
778795
string src = Runtime.GetManagedString(ob);
779796
Runtime.XDecref(ob);
780-
Exceptions.SetError(Exceptions.TypeError, $"Cannot convert {src} to {target}");
797+
Exceptions.RaiseTypeError($"Cannot convert {src} to {target}");
781798
}
782799

783800

@@ -791,32 +808,58 @@ private static bool ToArray(IntPtr value, Type obType, out object result, bool s
791808
Type elementType = obType.GetElementType();
792809
result = null;
793810

794-
bool IsSeqObj = Runtime.PySequence_Check(value);
795-
var len = IsSeqObj ? Runtime.PySequence_Size(value) : -1;
796-
797811
IntPtr IterObject = Runtime.PyObject_GetIter(value);
798-
799-
if(IterObject==IntPtr.Zero) {
812+
if (IterObject == IntPtr.Zero)
813+
{
800814
if (setError)
801815
{
802816
SetConversionError(value, obType);
803817
}
818+
else
819+
{
820+
// PyObject_GetIter will have set an error
821+
Exceptions.Clear();
822+
}
804823
return false;
805824
}
806825

807-
Array items;
826+
IList list;
827+
try
828+
{
829+
// MakeGenericType can throw because elementType may not be a valid generic argument even though elementType[] is a valid array type.
830+
// For example, if elementType is a pointer type.
831+
// See https://docs.microsoft.com/en-us/dotnet/api/system.type.makegenerictype#System_Type_MakeGenericType_System_Type
832+
var constructedListType = typeof(List<>).MakeGenericType(elementType);
833+
bool IsSeqObj = Runtime.PySequence_Check(value);
834+
if (IsSeqObj)
835+
{
836+
var len = Runtime.PySequence_Size(value);
837+
list = (IList)Activator.CreateInstance(constructedListType, new Object[] { (int)len });
838+
}
839+
else
840+
{
841+
// CreateInstance can throw even if MakeGenericType succeeded.
842+
// See https://docs.microsoft.com/en-us/dotnet/api/system.activator.createinstance#System_Activator_CreateInstance_System_Type_
843+
list = (IList)Activator.CreateInstance(constructedListType);
844+
}
845+
}
846+
catch (Exception e)
847+
{
848+
if (setError)
849+
{
850+
Exceptions.SetError(e);
851+
SetConversionError(value, obType);
852+
}
853+
return false;
854+
}
808855

809-
var listType = typeof(List<>);
810-
var constructedListType = listType.MakeGenericType(elementType);
811-
IList list = IsSeqObj ? (IList) Activator.CreateInstance(constructedListType, new Object[] {(int) len}) :
812-
(IList) Activator.CreateInstance(constructedListType);
813856
IntPtr item;
814857

815858
while ((item = Runtime.PyIter_Next(IterObject)) != IntPtr.Zero)
816859
{
817-
object obj = null;
860+
object obj;
818861

819-
if (!Converter.ToManaged(item, elementType, out obj, true))
862+
if (!Converter.ToManaged(item, elementType, out obj, setError))
820863
{
821864
Runtime.XDecref(item);
822865
return false;
@@ -827,7 +870,7 @@ private static bool ToArray(IntPtr value, Type obType, out object result, bool s
827870
}
828871
Runtime.XDecref(IterObject);
829872

830-
items = Array.CreateInstance(elementType, list.Count);
873+
Array items = Array.CreateInstance(elementType, list.Count);
831874
list.CopyTo(items, 0);
832875

833876
result = items;

‎src/runtime/eventbinding.cs

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -68,35 +68,27 @@ public static IntPtr nb_inplace_subtract(IntPtr ob, IntPtr arg)
6868
/// <summary>
6969
/// EventBinding __hash__ implementation.
7070
/// </summary>
71-
public static IntPtr tp_hash(IntPtr ob)
71+
public static nint tp_hash(IntPtr ob)
7272
{
7373
var self = (EventBinding)GetManagedObject(ob);
74-
long x = 0;
75-
long y = 0;
74+
nint x = 0;
7675

7776
if (self.target != IntPtr.Zero)
7877
{
79-
x = Runtime.PyObject_Hash(self.target).ToInt64();
78+
x = Runtime.PyObject_Hash(self.target);
8079
if (x == -1)
8180
{
82-
return new IntPtr(-1);
81+
return x;
8382
}
8483
}
8584

86-
y = Runtime.PyObject_Hash(self.e.pyHandle).ToInt64();
85+
nint y = Runtime.PyObject_Hash(self.e.pyHandle);
8786
if (y == -1)
8887
{
89-
return new IntPtr(-1);
88+
return y;
9089
}
9190

92-
x ^= y;
93-
94-
if (x == -1)
95-
{
96-
x = -1;
97-
}
98-
99-
return new IntPtr(x);
91+
return x ^ y;
10092
}
10193

10294

‎src/runtime/eventobject.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,17 +72,22 @@ internal bool AddEventHandler(IntPtr target, IntPtr handler)
7272
/// </summary>
7373
internal bool RemoveEventHandler(IntPtr target, IntPtr handler)
7474
{
75+
if (reg == null)
76+
{
77+
Exceptions.SetError(Exceptions.ValueError, "unknown event handler");
78+
return false;
79+
}
80+
7581
object obj = null;
7682
if (target != IntPtr.Zero)
7783
{
7884
var co = (CLRObject)GetManagedObject(target);
7985
obj = co.inst;
8086
}
8187

82-
IntPtr hash = Runtime.PyObject_Hash(handler);
83-
if (Exceptions.ErrorOccurred() || reg == null)
88+
nint hash = Runtime.PyObject_Hash(handler);
89+
if (hash == -1 || Exceptions.ErrorOccurred())
8490
{
85-
Exceptions.SetError(Exceptions.ValueError, "unknown event handler");
8691
return false;
8792
}
8893

‎src/runtime/exceptions.cs

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,20 @@ public static void SetError(Exception e)
290290
Runtime.XDecref(op);
291291
}
292292

293+
/// <summary>
294+
/// When called after SetError, sets the cause of the error.
295+
/// </summary>
296+
/// <param name="cause"></param>
297+
public static void SetCause(PythonException cause)
298+
{
299+
var currentException = new PythonException();
300+
currentException.Normalize();
301+
cause.Normalize();
302+
Runtime.XIncref(cause.PyValue);
303+
Runtime.PyException_SetCause(currentException.PyValue, cause.PyValue);
304+
currentException.Restore();
305+
}
306+
293307
/// <summary>
294308
/// ErrorOccurred Method
295309
/// </summary>
@@ -368,17 +382,31 @@ public static void deprecation(string message)
368382
// Internal helper methods for common error handling scenarios.
369383
//====================================================================
370384

385+
/// <summary>
386+
/// Raises a TypeError exception and attaches any existing exception as its cause.
387+
/// </summary>
388+
/// <param name="message">The exception message</param>
389+
/// <returns><c>IntPtr.Zero</c></returns>
371390
internal static IntPtr RaiseTypeError(string message)
372391
{
392+
PythonException previousException = null;
393+
if (ErrorOccurred())
394+
{
395+
previousException = new PythonException();
396+
}
373397
Exceptions.SetError(Exceptions.TypeError, message);
398+
if (previousException != null)
399+
{
400+
SetCause(previousException);
401+
}
374402
return IntPtr.Zero;
375403
}
376404

377405
// 2010-11-16: Arranged in python (2.6 & 2.7) source header file order
378406
/* Predefined exceptions are
379-
puplic static variables on the Exceptions class filled in from
407+
public static variables on the Exceptions class filled in from
380408
the python class using reflection in Initialize() looked up by
381-
name, not posistion. */
409+
name, not position. */
382410
public static IntPtr BaseException;
383411
public static IntPtr Exception;
384412
public static IntPtr StopIteration;

0 commit comments

Comments
 (0)