Make ConversionErrors more useful

python_exceptions.go

@@ -20,6 +20,39 @@ import (
 	"go.starlark.net/syntax"
 )
 
+func getCurrentPythonException() (*C.PyObject, *C.PyObject, *C.PyObject) {
+	var ptype *C.PyObject = nil
+	var pvalue *C.PyObject = nil
+	var ptraceback *C.PyObject = nil
+
+	C.PyErr_Fetch(&ptype, &pvalue, &ptraceback)
+	C.PyErr_NormalizeException(&ptype, &pvalue, &ptraceback)
+	if ptraceback != nil {
+		C.PyException_SetTraceback(pvalue, ptraceback)
+		C.Py_DecRef(ptraceback)
+	}
+
+	return ptype, pvalue, ptraceback
+}
+
+func setPythonExceptionCause(cause *C.PyObject) {
+	ptype, pvalue, ptraceback := getCurrentPythonException()
+	C.PyException_SetCause(pvalue, cause)
+	C.PyErr_Restore(ptype, pvalue, ptraceback)
+}
+
+func handleConversionError(err error, pytype *C.PyObject) {
+	_, pvalue, _ := getCurrentPythonException()
+
+	if pvalue != nil {
+		pvalue = C.cgoPy_NewRef(pvalue)
+		defer setPythonExceptionCause(pvalue)
+	}
+	errmsg := C.CString(err.Error())
+	defer C.free(unsafe.Pointer(errmsg))
+	C.PyErr_SetString(pytype, errmsg)
+}
+
 func raisePythonException(err error) {
 	var (
 		exc_args   *C.PyObject

python_globals.go

@@ -2,8 +2,6 @@ package main
 
 /*
 #include "starlark.h"
-
-extern PyObject *ConversionError;
 */
 import "C"
 

python_to_starlark.go

@@ -3,14 +3,13 @@ package main
 /*
 #include "starlark.h"
 
-extern PyObject *ConversionError;
+extern PyObject *ConversionToStarlarkFailed;
 */
 import "C"
 
 import (
 	"fmt"
 	"math/big"
-	"unsafe"
 
 	"go.starlark.net/starlark"
 )
@@ -19,19 +18,25 @@ func pythonToStarlarkTuple(obj *C.PyObject) (starlark.Tuple, error) {
 	var elems []starlark.Value
 	pyiter := C.PyObject_GetIter(obj)
 	if pyiter == nil {
-		return starlark.Tuple{}, fmt.Errorf("List: couldn't get iterator")
+		return starlark.Tuple{}, fmt.Errorf("Couldn't get iterator for Python tuple")
 	}
 	defer C.Py_DecRef(pyiter)
 
+	index := 0
 	for pyvalue := C.PyIter_Next(pyiter); pyvalue != nil; pyvalue = C.PyIter_Next(pyiter) {
 		defer C.Py_DecRef(pyvalue)
 
-		value, err := pythonToStarlarkValue(pyvalue)
+		value, err := innerPythonToStarlarkValue(pyvalue)
 		if err != nil {
-			return starlark.Tuple{}, err
+			return starlark.Tuple{}, fmt.Errorf("While converting value at index %v in Python tuple: %v", index, err)
 		}
 
 		elems = append(elems, value)
+		index += 1
+	}
+
+	if C.PyErr_Occurred() != nil {
+		return starlark.Tuple{}, fmt.Errorf("Python exception while converting value at index %v in Python tuple", index)
 	}
 
 	return starlark.Tuple(elems), nil
@@ -40,7 +45,7 @@ func pythonToStarlarkTuple(obj *C.PyObject) (starlark.Tuple, error) {
 func pythonToStarlarkBytes(obj *C.PyObject) (starlark.Bytes, error) {
 	cbytes := C.PyBytes_AsString(obj)
 	if cbytes == nil {
-		return starlark.Bytes(""), fmt.Errorf("Bytes: couldn't get pointer")
+		return starlark.Bytes(""), fmt.Errorf("Couldn't get pointer to Python bytes")
 	}
 
 	return starlark.Bytes(C.GoString(cbytes)), nil
@@ -49,107 +54,120 @@ func pythonToStarlarkBytes(obj *C.PyObject) (starlark.Bytes, error) {
 func pythonToStarlarkList(obj *C.PyObject) (*starlark.List, error) {
 	len := C.PyObject_Length(obj)
 	if len < 0 {
-		return &starlark.List{}, fmt.Errorf("List: couldn't get length of object")
+		return &starlark.List{}, fmt.Errorf("Couldn't get size of Python list")
 	}
 
 	var elems []starlark.Value
 	pyiter := C.PyObject_GetIter(obj)
 	if pyiter == nil {
-		return &starlark.List{}, fmt.Errorf("List: couldn't get iterator")
+		return &starlark.List{}, fmt.Errorf("Couldn't get iterator for Python list")
 	}
 	defer C.Py_DecRef(pyiter)
 
+	index := 0
 	for pyvalue := C.PyIter_Next(pyiter); pyvalue != nil; pyvalue = C.PyIter_Next(pyiter) {
 		defer C.Py_DecRef(pyvalue)
-
-		value, err := pythonToStarlarkValue(pyvalue)
+		value, err := innerPythonToStarlarkValue(pyvalue)
 		if err != nil {
-			return &starlark.List{}, err
+			return &starlark.List{}, fmt.Errorf("While converting value at index %v in Python list: %v", index, err)
 		}
 
 		elems = append(elems, value)
+		index += 1
+	}
+
+	if C.PyErr_Occurred() != nil {
+		return &starlark.List{}, fmt.Errorf("Python exception while converting value at index %v in Python list", index)
 	}
 
 	return starlark.NewList(elems), nil
 }
 
 func pythonToStarlarkDict(obj *C.PyObject) (*starlark.Dict, error) {
-	len := C.PyObject_Length(obj)
-	if len < 0 {
-		return &starlark.Dict{}, fmt.Errorf("Dict: couldn't get length of object")
+	size := C.PyObject_Length(obj)
+	if size < 0 {
+		return &starlark.Dict{}, fmt.Errorf("Couldn't get size of Python dict")
 	}
 
-	dict := starlark.NewDict(int(len))
+	dict := starlark.NewDict(int(size))
 	pyiter := C.PyObject_GetIter(obj)
 	if pyiter == nil {
-		return &starlark.Dict{}, fmt.Errorf("Dict: couldn't get iterator")
+		return &starlark.Dict{}, fmt.Errorf("Couldn't get iterator for Python dict")
 	}
 	defer C.Py_DecRef(pyiter)
 
 	for pykey := C.PyIter_Next(pyiter); pykey != nil; pykey = C.PyIter_Next(pyiter) {
 		defer C.Py_DecRef(pykey)
 
+		key, err := innerPythonToStarlarkValue(pykey)
+		if err != nil {
+			return &starlark.Dict{}, fmt.Errorf("While converting key in Python dict: %v", err)
+		}
+
 		pyvalue := C.PyObject_GetItem(obj, pykey)
 		if pyvalue == nil {
-			return &starlark.Dict{}, fmt.Errorf("Dict: couldn't get value")
+			return &starlark.Dict{}, fmt.Errorf("Couldn't get value of key %v in Python dict", key)
 		}
 		defer C.Py_DecRef(pyvalue)
 
-		key, err := pythonToStarlarkValue(pykey)
-		if err != nil {
-			return &starlark.Dict{}, err
-		}
-
-		value, err := pythonToStarlarkValue(pyvalue)
+		value, err := innerPythonToStarlarkValue(pyvalue)
 		if err != nil {
-			return &starlark.Dict{}, err
+			return &starlark.Dict{}, fmt.Errorf("While converting value of key %v in Python dict: %v", key, err)
 		}
 
 		err = dict.SetKey(key, value)
 		if err != nil {
-			return &starlark.Dict{}, err
+			return &starlark.Dict{}, fmt.Errorf("While setting %v to %v in Starlark dict: %v", key, value, err)
 		}
 	}
 
+	if C.PyErr_Occurred() != nil {
+		return &starlark.Dict{}, fmt.Errorf("Python exception while iterating through Python dict")
+	}
+
 	return dict, nil
 }
 
 func pythonToStarlarkSet(obj *C.PyObject) (*starlark.Set, error) {
-	len := C.PyObject_Length(obj)
-	if len < 0 {
-		return &starlark.Set{}, fmt.Errorf("Set: couldn't get length of object")
+	size := C.PyObject_Length(obj)
+	if size < 0 {
+		return &starlark.Set{}, fmt.Errorf("Couldn't get size of Python set")
 	}
 
-	set := starlark.NewSet(int(len))
+	set := starlark.NewSet(int(size))
 	pyiter := C.PyObject_GetIter(obj)
 	if pyiter == nil {
-		return &starlark.Set{}, fmt.Errorf("Set: couldn't get iterator")
+		return &starlark.Set{}, fmt.Errorf("Couldn't get iterator for Python set")
 	}
 	defer C.Py_DecRef(pyiter)
 
 	for pyvalue := C.PyIter_Next(pyiter); pyvalue != nil; pyvalue = C.PyIter_Next(pyiter) {
 		defer C.Py_DecRef(pyvalue)
 
-		value, err := pythonToStarlarkValue(pyvalue)
+		value, err := innerPythonToStarlarkValue(pyvalue)
 		if err != nil {
-			return &starlark.Set{}, err
+			return &starlark.Set{}, fmt.Errorf("While converting value in Python set: %v", err)
 		}
 
 		err = set.Insert(value)
 		if err != nil {
 			raisePythonException(err)
-			return &starlark.Set{}, err
+			return &starlark.Set{}, fmt.Errorf("While inserting %v into Starlark set: %v", value, err)
 		}
 	}
 
+	if C.PyErr_Occurred() != nil {
+		return &starlark.Set{}, fmt.Errorf("Python exception while converting value in Python set to Starlark")
+	}
+
 	return set, nil
 }
 
 func pythonToStarlarkString(obj *C.PyObject) (starlark.String, error) {
 	var size C.Py_ssize_t
 	cstr := C.PyUnicode_AsUTF8AndSize(obj, &size)
 	if cstr == nil {
-		return starlark.String(""), fmt.Errorf("Int: couldn't convert to C string")
+		return starlark.String(""), fmt.Errorf("Couldn't convert Python string to C string")
 	}
 
 	return starlark.String(C.GoString(cstr)), nil
@@ -159,7 +177,7 @@ func pythonToStarlarkInt(obj *C.PyObject) (starlark.Int, error) {
 	overflow := C.int(0)
 	longlong := int64(C.PyLong_AsLongLongAndOverflow(obj, &overflow)) // https://youtu.be/6-1Ue0FFrHY
 	if C.PyErr_Occurred() != nil {
-		return starlark.Int{}, fmt.Errorf("Int: couldn't convert to long")
+		return starlark.Int{}, fmt.Errorf("Couldn't convert Python int to long")
 	}
 
 	if overflow == 0 {
@@ -168,14 +186,14 @@ func pythonToStarlarkInt(obj *C.PyObject) (starlark.Int, error) {
 
 	pystr := C.PyObject_Str(obj)
 	if pystr == nil {
-		return starlark.Int{}, fmt.Errorf("Int: couldn't convert to Python string")
+		return starlark.Int{}, fmt.Errorf("Couldn't convert Python int to string")
 	}
 	defer C.Py_DecRef(pystr)
 
 	var size C.Py_ssize_t
 	cstr := C.PyUnicode_AsUTF8AndSize(pystr, &size)
 	if cstr == nil {
-		return starlark.Int{}, fmt.Errorf("Int: couldn't convert to C string")
+		return starlark.Int{}, fmt.Errorf("Couldn't convert Python int to C string")
 	}
 
 	i := new(big.Int)
@@ -186,13 +204,13 @@ func pythonToStarlarkInt(obj *C.PyObject) (starlark.Int, error) {
 func pythonToStarlarkFloat(obj *C.PyObject) (starlark.Float, error) {
 	cvalue := C.PyFloat_AsDouble(obj)
 	if C.PyErr_Occurred() != nil {
-		return starlark.Float(0), fmt.Errorf("Float: couldn't conver to double")
+		return starlark.Float(0), fmt.Errorf("Couldn't convert Python float to double")
 	}
 
 	return starlark.Float(cvalue), nil
 }
 
-func pythonToStarlarkValue(obj *C.PyObject) (starlark.Value, error) {
+func innerPythonToStarlarkValue(obj *C.PyObject) (starlark.Value, error) {
 	var value starlark.Value = nil
 	var err error = nil
 
@@ -219,25 +237,29 @@ func pythonToStarlarkValue(obj *C.PyObject) (starlark.Value, error) {
 		value, err = pythonToStarlarkList(obj)
 	case C.cgoPyTuple_Check(obj) == 1:
 		value, err = pythonToStarlarkTuple(obj)
-	case C.PyMapping_Check(obj) == 1:
-		value, err = pythonToStarlarkDict(obj)
 	case C.PySequence_Check(obj) == 1:
 		value, err = pythonToStarlarkList(obj)
+	case C.PyMapping_Check(obj) == 1:
+		value, err = pythonToStarlarkDict(obj)
+	default:
+		err = fmt.Errorf("Don't know how to convert Python %s to Starlark", C.GoString(obj.ob_type.tp_name))
 	}
 
-	if value == nil {
-		if err == nil {
-			err = fmt.Errorf("Can't convert Python %s to Starlark", C.GoString(obj.ob_type.tp_name))
+	if err == nil {
+		if C.PyErr_Occurred() != nil {
+			err = fmt.Errorf("Python exception while converting to Starlark")
 		}
 	}
 
+	return value, err
+}
+
+func pythonToStarlarkValue(obj *C.PyObject) (starlark.Value, error) {
+	value, err := innerPythonToStarlarkValue(obj)
 	if err != nil {
-		if C.PyErr_Occurred() == nil {
-			errmsg := C.CString(err.Error())
-			defer C.free(unsafe.Pointer(errmsg))
-			C.PyErr_SetString(C.ConversionError, errmsg)
-		}
+		handleConversionError(err, C.ConversionToStarlarkFailed)
+		return starlark.None, err
 	}
 
-	return value, err
+	return value, nil
 }

src/starlark_go/__init__.py

@@ -1,5 +1,7 @@
 from starlark_go.errors import (
     ConversionError,
+    ConversionToPythonFailed,
+    ConversionToStarlarkFailed,
     EvalError,
     ResolveError,
     ResolveErrorItem,
@@ -16,6 +18,8 @@
     "Starlark",
     "StarlarkError",
     "ConversionError",
+    "ConversionToPythonFailed",
+    "ConversionToStarlarkFailed",
     "EvalError",
     "ResolveError",
     "ResolveErrorItem",

src/starlark_go/errors.py

@@ -185,10 +185,24 @@ def __init__(
 
 class ConversionError(StarlarkError):
     """
-    A Starlark conversion error.
+    A base class for conversion errors.
+    """
+
+
+class ConversionToPythonFailed(ConversionError):
+    """
+    An error when converting a Starlark value to a Python value.
+
+    This exception is raied by :py:meth:`starlark_go.Starlark.eval`
+    and :py:meth:`starlark_go.Starlark.get` when a Starlark value can
+    not be converted to a Python value.
+    """
+
+
+class ConversionToStarlarkFailed(ConversionError):
+    """
+    An error when converting a Python value to a Starlark value.
 
-    This exception is raied by :py:meth:`starlark_go.Starlark.eval`,
-    :py:meth:`starlark_go.Starlark.get`, and :py:meth:`starlark_go.Starlark.set`
-    when a Starlark value can not be converted to a Python value, or when a
-    Python value can not be converted to a Starlark value.
+    This exception is raied by :py:meth:`starlark_go.Starlark.set`
+    when a Python value can not be converted to a Starlark value.
     """