Skip to content

Commit 3632c1a

Browse files
jordemortjordemort
committed
Make ConversionErrors more useful
- Subclass into ConversionToPythonFailed and ConversionToStarlarkFailed - Make messages much more informative and indicate which key or index failed - If conversion fails because of a Python exception, add that exception as the source of ours Closes #143
1 parent 8e3969d commit 3632c1a

9 files changed

+286
-85
lines changed

‎python_exceptions.go

+33
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,39 @@ import (
2020
"go.starlark.net/syntax"
2121
)
2222

23+
func getCurrentPythonException() (*C.PyObject, *C.PyObject, *C.PyObject) {
24+
var ptype *C.PyObject = nil
25+
var pvalue *C.PyObject = nil
26+
var ptraceback *C.PyObject = nil
27+
28+
C.PyErr_Fetch(&ptype, &pvalue, &ptraceback)
29+
C.PyErr_NormalizeException(&ptype, &pvalue, &ptraceback)
30+
if ptraceback != nil {
31+
C.PyException_SetTraceback(pvalue, ptraceback)
32+
C.Py_DecRef(ptraceback)
33+
}
34+
35+
return ptype, pvalue, ptraceback
36+
}
37+
38+
func setPythonExceptionCause(cause *C.PyObject) {
39+
ptype, pvalue, ptraceback := getCurrentPythonException()
40+
C.PyException_SetCause(pvalue, cause)
41+
C.PyErr_Restore(ptype, pvalue, ptraceback)
42+
}
43+
44+
func handleConversionError(err error, pytype *C.PyObject) {
45+
_, pvalue, _ := getCurrentPythonException()
46+
47+
if pvalue != nil {
48+
pvalue = C.cgoPy_NewRef(pvalue)
49+
defer setPythonExceptionCause(pvalue)
50+
}
51+
errmsg := C.CString(err.Error())
52+
defer C.free(unsafe.Pointer(errmsg))
53+
C.PyErr_SetString(pytype, errmsg)
54+
}
55+
2356
func raisePythonException(err error) {
2457
var (
2558
exc_args *C.PyObject

‎python_globals.go

-2
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package main
22

33
/*
44
#include "starlark.h"
5-
6-
extern PyObject *ConversionError;
75
*/
86
import "C"
97

‎python_to_starlark.go

+71-49
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,13 @@ package main
33
/*
44
#include "starlark.h"
55
6-
extern PyObject *ConversionError;
6+
extern PyObject *ConversionToStarlarkFailed;
77
*/
88
import "C"
99

1010
import (
1111
"fmt"
1212
"math/big"
13-
"unsafe"
1413

1514
"go.starlark.net/starlark"
1615
)
@@ -19,19 +18,25 @@ func pythonToStarlarkTuple(obj *C.PyObject) (starlark.Tuple, error) {
1918
var elems []starlark.Value
2019
pyiter := C.PyObject_GetIter(obj)
2120
if pyiter == nil {
22-
return starlark.Tuple{}, fmt.Errorf("List: couldn't get iterator")
21+
return starlark.Tuple{}, fmt.Errorf("Couldn't get iterator for Python tuple")
2322
}
2423
defer C.Py_DecRef(pyiter)
2524

25+
index := 0
2626
for pyvalue := C.PyIter_Next(pyiter); pyvalue != nil; pyvalue = C.PyIter_Next(pyiter) {
2727
defer C.Py_DecRef(pyvalue)
2828

29-
value, err := pythonToStarlarkValue(pyvalue)
29+
value, err := innerPythonToStarlarkValue(pyvalue)
3030
if err != nil {
31-
return starlark.Tuple{}, err
31+
return starlark.Tuple{}, fmt.Errorf("While converting value at index %v in Python tuple: %v", index, err)
3232
}
3333

3434
elems = append(elems, value)
35+
index += 1
36+
}
37+
38+
if C.PyErr_Occurred() != nil {
39+
return starlark.Tuple{}, fmt.Errorf("Python exception while converting value at index %v in Python tuple", index)
3540
}
3641

3742
return starlark.Tuple(elems), nil
@@ -40,7 +45,7 @@ func pythonToStarlarkTuple(obj *C.PyObject) (starlark.Tuple, error) {
4045
func pythonToStarlarkBytes(obj *C.PyObject) (starlark.Bytes, error) {
4146
cbytes := C.PyBytes_AsString(obj)
4247
if cbytes == nil {
43-
return starlark.Bytes(""), fmt.Errorf("Bytes: couldn't get pointer")
48+
return starlark.Bytes(""), fmt.Errorf("Couldn't get pointer to Python bytes")
4449
}
4550

4651
return starlark.Bytes(C.GoString(cbytes)), nil
@@ -49,107 +54,120 @@ func pythonToStarlarkBytes(obj *C.PyObject) (starlark.Bytes, error) {
4954
func pythonToStarlarkList(obj *C.PyObject) (*starlark.List, error) {
5055
len := C.PyObject_Length(obj)
5156
if len < 0 {
52-
return &starlark.List{}, fmt.Errorf("List: couldn't get length of object")
57+
return &starlark.List{}, fmt.Errorf("Couldn't get size of Python list")
5358
}
5459

5560
var elems []starlark.Value
5661
pyiter := C.PyObject_GetIter(obj)
5762
if pyiter == nil {
58-
return &starlark.List{}, fmt.Errorf("List: couldn't get iterator")
63+
return &starlark.List{}, fmt.Errorf("Couldn't get iterator for Python list")
5964
}
6065
defer C.Py_DecRef(pyiter)
6166

67+
index := 0
6268
for pyvalue := C.PyIter_Next(pyiter); pyvalue != nil; pyvalue = C.PyIter_Next(pyiter) {
6369
defer C.Py_DecRef(pyvalue)
64-
65-
value, err := pythonToStarlarkValue(pyvalue)
70+
value, err := innerPythonToStarlarkValue(pyvalue)
6671
if err != nil {
67-
return &starlark.List{}, err
72+
return &starlark.List{}, fmt.Errorf("While converting value at index %v in Python list: %v", index, err)
6873
}
6974

7075
elems = append(elems, value)
76+
index += 1
77+
}
78+
79+
if C.PyErr_Occurred() != nil {
80+
return &starlark.List{}, fmt.Errorf("Python exception while converting value at index %v in Python list", index)
7181
}
7282

7383
return starlark.NewList(elems), nil
7484
}
7585

7686
func pythonToStarlarkDict(obj *C.PyObject) (*starlark.Dict, error) {
77-
len := C.PyObject_Length(obj)
78-
if len < 0 {
79-
return &starlark.Dict{}, fmt.Errorf("Dict: couldn't get length of object")
87+
size := C.PyObject_Length(obj)
88+
if size < 0 {
89+
return &starlark.Dict{}, fmt.Errorf("Couldn't get size of Python dict")
8090
}
8191

82-
dict := starlark.NewDict(int(len))
92+
dict := starlark.NewDict(int(size))
8393
pyiter := C.PyObject_GetIter(obj)
8494
if pyiter == nil {
85-
return &starlark.Dict{}, fmt.Errorf("Dict: couldn't get iterator")
95+
return &starlark.Dict{}, fmt.Errorf("Couldn't get iterator for Python dict")
8696
}
8797
defer C.Py_DecRef(pyiter)
8898

8999
for pykey := C.PyIter_Next(pyiter); pykey != nil; pykey = C.PyIter_Next(pyiter) {
90100
defer C.Py_DecRef(pykey)
91101

102+
key, err := innerPythonToStarlarkValue(pykey)
103+
if err != nil {
104+
return &starlark.Dict{}, fmt.Errorf("While converting key in Python dict: %v", err)
105+
}
106+
92107
pyvalue := C.PyObject_GetItem(obj, pykey)
93108
if pyvalue == nil {
94-
return &starlark.Dict{}, fmt.Errorf("Dict: couldn't get value")
109+
return &starlark.Dict{}, fmt.Errorf("Couldn't get value of key %v in Python dict", key)
95110
}
96111
defer C.Py_DecRef(pyvalue)
97112

98-
key, err := pythonToStarlarkValue(pykey)
99-
if err != nil {
100-
return &starlark.Dict{}, err
101-
}
102-
103-
value, err := pythonToStarlarkValue(pyvalue)
113+
value, err := innerPythonToStarlarkValue(pyvalue)
104114
if err != nil {
105-
return &starlark.Dict{}, err
115+
return &starlark.Dict{}, fmt.Errorf("While converting value of key %v in Python dict: %v", key, err)
106116
}
107117

108118
err = dict.SetKey(key, value)
109119
if err != nil {
110-
return &starlark.Dict{}, err
120+
return &starlark.Dict{}, fmt.Errorf("While setting %v to %v in Starlark dict: %v", key, value, err)
111121
}
112122
}
113123

124+
if C.PyErr_Occurred() != nil {
125+
return &starlark.Dict{}, fmt.Errorf("Python exception while iterating through Python dict")
126+
}
127+
114128
return dict, nil
115129
}
116130

117131
func pythonToStarlarkSet(obj *C.PyObject) (*starlark.Set, error) {
118-
len := C.PyObject_Length(obj)
119-
if len < 0 {
120-
return &starlark.Set{}, fmt.Errorf("Set: couldn't get length of object")
132+
size := C.PyObject_Length(obj)
133+
if size < 0 {
134+
return &starlark.Set{}, fmt.Errorf("Couldn't get size of Python set")
121135
}
122136

123-
set := starlark.NewSet(int(len))
137+
set := starlark.NewSet(int(size))
124138
pyiter := C.PyObject_GetIter(obj)
125139
if pyiter == nil {
126-
return &starlark.Set{}, fmt.Errorf("Set: couldn't get iterator")
140+
return &starlark.Set{}, fmt.Errorf("Couldn't get iterator for Python set")
127141
}
128142
defer C.Py_DecRef(pyiter)
129143

130144
for pyvalue := C.PyIter_Next(pyiter); pyvalue != nil; pyvalue = C.PyIter_Next(pyiter) {
131145
defer C.Py_DecRef(pyvalue)
132146

133-
value, err := pythonToStarlarkValue(pyvalue)
147+
value, err := innerPythonToStarlarkValue(pyvalue)
134148
if err != nil {
135-
return &starlark.Set{}, err
149+
return &starlark.Set{}, fmt.Errorf("While converting value in Python set: %v", err)
136150
}
137151

138152
err = set.Insert(value)
139153
if err != nil {
140154
raisePythonException(err)
141-
return &starlark.Set{}, err
155+
return &starlark.Set{}, fmt.Errorf("While inserting %v into Starlark set: %v", value, err)
142156
}
143157
}
144158

159+
if C.PyErr_Occurred() != nil {
160+
return &starlark.Set{}, fmt.Errorf("Python exception while converting value in Python set to Starlark")
161+
}
162+
145163
return set, nil
146164
}
147165

148166
func pythonToStarlarkString(obj *C.PyObject) (starlark.String, error) {
149167
var size C.Py_ssize_t
150168
cstr := C.PyUnicode_AsUTF8AndSize(obj, &size)
151169
if cstr == nil {
152-
return starlark.String(""), fmt.Errorf("Int: couldn't convert to C string")
170+
return starlark.String(""), fmt.Errorf("Couldn't convert Python string to C string")
153171
}
154172

155173
return starlark.String(C.GoString(cstr)), nil
@@ -159,7 +177,7 @@ func pythonToStarlarkInt(obj *C.PyObject) (starlark.Int, error) {
159177
overflow := C.int(0)
160178
longlong := int64(C.PyLong_AsLongLongAndOverflow(obj, &overflow)) // https://youtu.be/6-1Ue0FFrHY
161179
if C.PyErr_Occurred() != nil {
162-
return starlark.Int{}, fmt.Errorf("Int: couldn't convert to long")
180+
return starlark.Int{}, fmt.Errorf("Couldn't convert Python int to long")
163181
}
164182

165183
if overflow == 0 {
@@ -168,14 +186,14 @@ func pythonToStarlarkInt(obj *C.PyObject) (starlark.Int, error) {
168186

169187
pystr := C.PyObject_Str(obj)
170188
if pystr == nil {
171-
return starlark.Int{}, fmt.Errorf("Int: couldn't convert to Python string")
189+
return starlark.Int{}, fmt.Errorf("Couldn't convert Python int to string")
172190
}
173191
defer C.Py_DecRef(pystr)
174192

175193
var size C.Py_ssize_t
176194
cstr := C.PyUnicode_AsUTF8AndSize(pystr, &size)
177195
if cstr == nil {
178-
return starlark.Int{}, fmt.Errorf("Int: couldn't convert to C string")
196+
return starlark.Int{}, fmt.Errorf("Couldn't convert Python int to C string")
179197
}
180198

181199
i := new(big.Int)
@@ -186,13 +204,13 @@ func pythonToStarlarkInt(obj *C.PyObject) (starlark.Int, error) {
186204
func pythonToStarlarkFloat(obj *C.PyObject) (starlark.Float, error) {
187205
cvalue := C.PyFloat_AsDouble(obj)
188206
if C.PyErr_Occurred() != nil {
189-
return starlark.Float(0), fmt.Errorf("Float: couldn't conver to double")
207+
return starlark.Float(0), fmt.Errorf("Couldn't convert Python float to double")
190208
}
191209

192210
return starlark.Float(cvalue), nil
193211
}
194212

195-
func pythonToStarlarkValue(obj *C.PyObject) (starlark.Value, error) {
213+
func innerPythonToStarlarkValue(obj *C.PyObject) (starlark.Value, error) {
196214
var value starlark.Value = nil
197215
var err error = nil
198216

@@ -219,24 +237,28 @@ func pythonToStarlarkValue(obj *C.PyObject) (starlark.Value, error) {
219237
value, err = pythonToStarlarkList(obj)
220238
case C.cgoPyTuple_Check(obj) == 1:
221239
value, err = pythonToStarlarkTuple(obj)
222-
case C.PyMapping_Check(obj) == 1:
223-
value, err = pythonToStarlarkDict(obj)
224240
case C.PySequence_Check(obj) == 1:
225241
value, err = pythonToStarlarkList(obj)
242+
case C.PyMapping_Check(obj) == 1:
243+
value, err = pythonToStarlarkDict(obj)
244+
default:
245+
err = fmt.Errorf("Don't know how to convert Python %s to Starlark", C.GoString(obj.ob_type.tp_name))
226246
}
227247

228-
if value == nil {
229-
if err == nil {
230-
err = fmt.Errorf("Can't convert Python %s to Starlark", C.GoString(obj.ob_type.tp_name))
248+
if err == nil {
249+
if C.PyErr_Occurred() != nil {
250+
err = fmt.Errorf("Python exception while converting to Starlark")
231251
}
232252
}
233253

254+
return value, err
255+
}
256+
257+
func pythonToStarlarkValue(obj *C.PyObject) (starlark.Value, error) {
258+
value, err := innerPythonToStarlarkValue(obj)
234259
if err != nil {
235-
if C.PyErr_Occurred() == nil {
236-
errmsg := C.CString(err.Error())
237-
defer C.free(unsafe.Pointer(errmsg))
238-
C.PyErr_SetString(C.ConversionError, errmsg)
239-
}
260+
handleConversionError(err, C.ConversionToStarlarkFailed)
261+
return nil, err
240262
}
241263

242264
return value, err

‎src/starlark_go/__init__.py

+4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
from starlark_go.errors import (
22
ConversionError,
3+
ConversionToPythonFailed,
4+
ConversionToStarlarkFailed,
35
EvalError,
46
ResolveError,
57
ResolveErrorItem,
@@ -16,6 +18,8 @@
1618
"Starlark",
1719
"StarlarkError",
1820
"ConversionError",
21+
"ConversionToPythonFailed",
22+
"ConversionToStarlarkFailed",
1923
"EvalError",
2024
"ResolveError",
2125
"ResolveErrorItem",

‎src/starlark_go/errors.py

+19-5
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,24 @@ def __init__(
185185

186186
class ConversionError(StarlarkError):
187187
"""
188-
A Starlark conversion error.
188+
A base class for conversion errors.
189+
"""
190+
191+
192+
class ConversionToPythonFailed(ConversionError):
193+
"""
194+
An error when converting a Starlark value to a Python value.
195+
196+
This exception is raied by :py:meth:`starlark_go.Starlark.eval`
197+
and :py:meth:`starlark_go.Starlark.get when a Starlark value can
198+
not be converted to a Python value.
199+
"""
200+
201+
202+
class ConversionToStarlarkFailed(ConversionError):
203+
"""
204+
An error when converting a Python value to a Starlark value.
189205
190-
This exception is raied by :py:meth:`starlark_go.Starlark.eval`,
191-
:py:meth:`starlark_go.Starlark.get`, and :py:meth:`starlark_go.Starlark.set`
192-
when a Starlark value can not be converted to a Python value, or when a
193-
Python value can not be converted to a Starlark value.
206+
This exception is raied by :py:meth:`starlark_go.Starlark.set`
207+
when a Python value can not be converted to a Starlark value.
194208
"""

0 commit comments

Comments
 (0)