[Libguestfs] [PATCH libnbd v2] python: Raise a custom exception containing error string and errno.

[Eric Blake]

On 6/28/19 7:31 AM, Richard W.M. Jones wrote:
> Previously errors caused a RuntimeException to be raised.  This commit
> defines a custom exception (nbd.Error) which has two parameters, the
> required error string, and the optional errno (which may be 0 if
> unavailable).
> 
> For example:
> 
> $ ./run nbdsh -c 'h.pread(0, 0)'
> Traceback (most recent call last):
>   File "/usr/lib64/python3.7/runpy.py", line 193, in _run_module_as_main
>     "__main__", mod_spec)
>   File "/usr/lib64/python3.7/runpy.py", line 85, in _run_code
>     exec(code, run_globals)
>   File "/home/rjones/d/libnbd/python/nbd.py", line 1136, in <module>
>     nbdsh.shell()
>   File "/home/rjones/d/libnbd/python/nbdsh.py", line 62, in shell
>     exec (c)
>   File "<string>", line 1, in <module>
>   File "/home/rjones/d/libnbd/python/nbd.py", line 456, in pread
>     return libnbdmod.pread (self._o, count, offset, flags)
> nbd.Error: ('nbd_pread: invalid state: START: the handle must be connected and finished handshaking with the server: Transport endpoint is not connected', 107)
> ---
>  generator/generator       | 34 +++++++++++++++++++++++++++++++++-
>  python/t/610-exception.py | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+), 1 deletion(-)

> +++ b/generator/generator
> @@ -3337,6 +3337,19 @@ get_handle (PyObject *obj)
>    return (struct nbd_handle *) PyCapsule_GetPointer(obj, \"nbd_handle\");
>  }
>  
> +/* nbd.Error exception. */
> +extern PyObject *nbd_internal_py_Error;
> +
> +static inline void
> +raise_exception ()
> +{
> +  PyObject *args = PyTuple_New (2);
> +
> +  PyTuple_SetItem (args, 0, PyUnicode_FromString (nbd_get_error ()));
> +  PyTuple_SetItem (args, 1, PyLong_FromLong (nbd_get_errno ()));
> +  PyErr_SetObject (nbd_internal_py_Error, args);

Misses error checking that PyTyple_New/PyTuple_SetItem didn't fail.

I think we want to ensure that our exception class has an __init__
method that can parse the tuple that we pass here and turn it into two
sane attributes (of course, we still want to pass that tuple to the
parent class, as all Python exceptions expect to access ex.args).
Adding helper methods to our exception class would happen...

> +}
> +
>  ";
>  
>    List.iter (
> @@ -3390,6 +3403,9 @@ static struct PyModuleDef moduledef = {
>    NULL,                  /* m_free */
>  };
>  
> +/* nbd.Error exception. */
> +PyObject *nbd_internal_py_Error;
> +
>  extern PyMODINIT_FUNC PyInit_libnbdmod (void);
>  
>  PyMODINIT_FUNC
> @@ -3401,6 +3417,19 @@ PyInit_libnbdmod (void)
>    if (mod == NULL)
>      return NULL;
>  
> +  nbd_internal_py_Error = PyErr_NewExceptionWithDoc (
> +    \"nbd.Error\",
> +    \"Exception thrown when the underlying libnbd call fails.  This\\n\"
> +    \"exception carries a two element tuple.  The first element is a\\n\"
> +    \"printable string containing the error message.  The second\\n\"
> +    \"element is the optional errno (which may be 0 in some cases\\n\"
> +    \"if the error does not correspond to a system call failure).\\n\",
> +    NULL, NULL
> +  );

...here. I don't know if it is as simple as passing a non-NULL dict
argument that includes an entry for the appropriate '__init__' method
(for parsing our incoming tuple) and an appropriate '__str__' for sanely
printing the exception, or if it is more complicated by actually
constructing our type in C
(https://docs.python.org/3/extending/newtypes_tutorial.html#subclassing-other-types).
 And if we figure out that, we should update the docs to refer to direct
attribute names (we can bikeshed on whether to name it e.message or
e.error as the message, and e.errno as an integer that maps back to the
standard 'errno' module).

In fact, is there a way to just write the subclass in pure python, then
import that in C code, rather than trying to generate it directly in C
code? As long as we have a way to suck in the correct PyObject (well,
its subclass PyTypeObject) representing the exception subclass, that's
all the more we need for populating nbd_internal_py_Error for later
handing off to PyErr_SetObject.


> +  if (nbd_internal_py_Error == NULL)
> +    return NULL;
> +  PyModule_AddObject (mod, \"Error\", nbd_internal_py_Error);
> +
>    return mod;
>  }
>  "
> @@ -3796,7 +3825,7 @@ let print_python_binding name { args; ret } =
>     | RBool | RErr | RFd | RInt | RInt64 -> pr "  if (ret == -1) {\n";
>     | RConstString | RString -> pr "  if (ret == NULL) {\n";
>    );
> -  pr "    PyErr_SetString (PyExc_RuntimeError, nbd_get_error ());\n";
> +  pr "    raise_exception ();\n";
>    pr "    py_ret = NULL;\n";
>    pr "    goto out;\n";
>    pr "  }\n";
> @@ -3917,6 +3946,9 @@ Read the libnbd(3) man page to find out how to use the API.
>  
>  import libnbdmod
>  
> +# Reexport Error exception as nbd.Error
> +from libnbdmod import Error
> +

This part looks good.

>  ";
>  
>    List.iter (fun (n, i) -> pr "%-30s = %d\n" n i) constants;
> diff --git a/python/t/610-exception.py b/python/t/610-exception.py
> new file mode 100644
> index 0000000..c7edf37
> --- /dev/null
> +++ b/python/t/610-exception.py

> +try:
> +    # This will always throw an exception because the handle is not
> +    # connected.
> +    h.pread (0, 0)
> +except nbd.Error as ex:
> +    str = ex.args[0]
> +    errno = ex.args[1]

So, if we figure out how to tie in the right __init__ method, this would
be spelled:

  str = ex.message
  err = ex.errno
  err_name = errno.errorcode[err]

> +    print ("str = %s\nerr = %d\n" % (str, errno))

and with a decent-enough __str__, we could even just:

print (ex)

where that uses errno.errorcode to print a name rather than a number (or
omit things if the number is 0).

> +    exit (0)
> +
> +# If we reach here then we didn't catch the exception above.
> +exit (1)
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190628/6023b9e9/attachment.sig>