"returned a result with an error set" instead of Exception for functions that return a value type

[justinfx]

trafficstars

I've encountered a situation where a certain wrapped function fails in the Go runtime, but the exception is swallowed and instead a generic Python exception is raised:

SystemError: <built-in function mymodule.MyFunction> returned a result with an error set

We can see an exiting example in the gopy/_examples/pyerrors test:

func Div(i, j int) (int, error) {
	if j == 0 {
		return 0, errors.New("Divide by zero.")
	}
	return i / j, nil
}

def div(a, b):
    try:
        r = pyerrors.Div(a, b)
        print("pyerrors.Div(%d, %d) = %d"% (a, b, r))
    except Exception as e:
        print(e)

div(5,0)
# <built-in function pyerrors_Div> returned a result with an error set

This makes it pretty difficult to debug since one would have to track down the wrapped code and figure out which return statement is actually being hit, then add some logging to see the real error.

I've been digging through pygo and PyBindGen for a few hours to better understand this problem. It seems like @rcoreilly was already aware of this problem when the test was written, and coded around it to have this output actually be a passing test, with a TODO to fix it later? But it seems like there is a bit of a disconnect between the location where gopy generates the C code with python C.PyErr_SetString error handling (bind/gen_func.go) vs where PyBindGen accepts C functions being added. I don't understand where there is any existing PyBindGen API to indicate what return value should be considered an error and when to check for one.

Problem: If a Python error has been set, then a PyObject* should not be returned. gopy sets the error, but PyBindGen has no current logic in the generated C code that checks PyErr_Occurred() other than some special int pointer case.

I have no experience with PyBindGen, but a decent amount with Cython. In cython there are a few different ways where you can indicate that a wrapped function can raise an exception. You can tell it an exact value that indicate an exception (ie -1 always being an exception) or you can tell it to always check PyErr_Occurred at the extra expense of that check for each call. I don't know how to do that for PyBindGen but ultimately I think we need to figure out how to set that.

Here is an example of what PyBindGen generates:

# build.py
mod.add_function('mypackage_DoThing', retval('int64_t'), [param('int64_t', '_handle')])

PyObject *
_wrap__mypackage_DoThing(PyObject * PYBINDGEN_UNUSED(dummy), PyObject *args, PyObject *kwargs)
{
    PyObject *py_retval;
    int64_t retval;
    int64_t _handle;
    const char *keywords[] = {"_handle", NULL};

    if (!PyArg_ParseTupleAndKeywords(args, kwargs, (char *) "L", (char **) keywords, &_handle)) {
        return NULL;
    }
    retval = mypackage_DoThing(_handle);
    py_retval = Py_BuildValue((char *) "L", retval);
    return py_retval;
}
PyObject * _wrap__mypackage_DoThing(PyObject * PYBINDGEN_UNUSED(dummy), PyObject *args, PyObject *kwargs);

The key part is this:

    retval = mypackage_DoThing(_handle);
    py_retval = Py_BuildValue((char *) "L", retval);
    return py_retval;

If mypackage_DoThing() has an error, it sets PyErr_SetString, and in which case it should return NULL. Is there a way to tell PyBindGen how a wrapped C function indicates an error? Ultimately we want something like this:

    retval = mypackage_DoThing(_handle);
    if (PyErr_Occurred()) {
        return NULL;
    }
    py_retval = Py_BuildValue((char *) "L", retval);
    return py_retval;

[justinfx]

I have a WIP fix available for discussion via #255