Libguestfs - Jun 2019 - Re: [PATCH libnbd v3] python: Raise a custom exception containing error string and errno.

On 6/28/19 1:27 PM, 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 1163, 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 483, 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
(ENOTCONN)
Cool - in the time I spent writing my reply to v2 1/1, your reaction to
my reply on 0/1 figured out the way to get what we want:

> @@ -3917,6 +3938,36 @@ Read the libnbd(3) man page to find out how to use
the API.
>  
>  import libnbdmod
>  
> +# Re-export Error exception as nbd.Error, adding some methods.
> +from libnbdmod import Error
Implement all the cool stuff in pure Python on top of the bare-bones
minimum :)  Lots less hassle than writing it in C code.  I like it!
> +
> +Error.__doc__ = '''
> +Exception thrown when the underlying libnbd call fails.
> +
> +This exception has two properties to query the error.  Use
> +the .string property to return a printable string containing
> +the error message.  Use the .errno property to return a
> +Python errno (which may be None in some cases if the error
> +did not correspond to a system call failure).
> +'''
> +
> +Error.string = property (lambda self: self.args[0])
> +
> +def _errno (self):
> +    import errno
> +    try:
> +        return errno.errorcode[self.args[1]]
> +    except KeyError:
> +        return None
> +Error.errno = property (_errno)
> +
> +def _str (self):
> +    if self.errno:
> +        return (\"%%s (%%s)\" %% (self.string, self.errno))
> +    else:
> +        return (\"%%s\" %% self.string)
> +Error.__str__ = _str
Looks good to me now!  Thanks for figuring this out while I was
struggling with reading lots of documentation on C bindings.

ACK

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

On Fri, Jun 28, 2019 at 01:53:11PM -0500, Eric Blake
wrote:
> On 6/28/19 1:27 PM, 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 1163, 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 483, 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 (ENOTCONN)
> 
> Cool - in the time I spent writing my reply to v2 1/1, your reaction to
> my reply on 0/1 figured out the way to get what we want:
> 
> 
> > @@ -3917,6 +3938,36 @@ Read the libnbd(3) man page to find out how to
use the API.
> >  
> >  import libnbdmod
> >  
> > +# Re-export Error exception as nbd.Error, adding some methods.
> > +from libnbdmod import Error
> 
> Implement all the cool stuff in pure Python on top of the bare-bones
> minimum :)  Lots less hassle than writing it in C code.  I like it!
> 
> > +
> > +Error.__doc__ = '''
> > +Exception thrown when the underlying libnbd call fails.
> > +
> > +This exception has two properties to query the error.  Use
> > +the .string property to return a printable string containing
> > +the error message.  Use the .errno property to return a
> > +Python errno (which may be None in some cases if the error
> > +did not correspond to a system call failure).
> > +'''
> > +
> > +Error.string = property (lambda self: self.args[0])
> > +
> > +def _errno (self):
> > +    import errno
> > +    try:
> > +        return errno.errorcode[self.args[1]]
> > +    except KeyError:
> > +        return None
> > +Error.errno = property (_errno)
> > +
> > +def _str (self):
> > +    if self.errno:
> > +        return (\"%%s (%%s)\" %% (self.string, self.errno))
> > +    else:
> > +        return (\"%%s\" %% self.string)
> > +Error.__str__ = _str
> 
> Looks good to me now!  Thanks for figuring this out while I was
> struggling with reading lots of documentation on C bindings.
> 
> ACK
I pushed it, but there may be a few issues still:

- Still no error checking in raise_exception().  We're on an error
  path already here so it's hard to do anything useful, although
  perhaps we should not segfault.

- The .errno attribute returns a (Python module) errno value, not a
  number, so the number is effectively lost, should that really be an
  issue.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On 6/28/19 2:17 PM, Richard W.M. Jones wrote:
>>
>> Looks good to me now!  Thanks for figuring this out while I was
>> struggling with reading lots of documentation on C bindings.
>>
>> ACK
> 
> I pushed it, but there may be a few issues still:
> 
> - Still no error checking in raise_exception().  We're on an error
>   path already here so it's hard to do anything useful, although
>   perhaps we should not segfault.
Yeah, avoiding the segfault is still worthwhile - but our lack of
PyFOO_BAR() error checking is more pervasive than just in
raise_exception(), so a patch to audit all of our generated code will
pick that up along with the rest.
> 
> - The .errno attribute returns a (Python module) errno value, not a
>   number, so the number is effectively lost, should that really be an
>   issue.
Maybe we want two fields, both .errno (string name, or None if Python
errno.errorcode couldn't map it to a name), and .errnum (raw numeric
value, accessible no matter what).

Maybe as simple as this (or with one further tweak to __str__ to at
least output .errnum when .errno is None):

diff --git i/generator/generator w/generator/generator
index 7c2fb59..9192988 100755
--- i/generator/generator
+++ w/generator/generator
@@ -3944,11 +3944,14 @@ from libnbdmod import Error
 Error.__doc__ = '''
 Exception thrown when the underlying libnbd call fails.

-This exception has two properties to query the error.  Use
+This exception has three properties to query the error.  Use
 the .string property to return a printable string containing
-the error message.  Use the .errno property to return a
-Python errno (which may be None in some cases if the error
-did not correspond to a system call failure).
+the error message.  Use the .errnum property for the associated
+numeric error value (which may be 0 if the error did not
+correspond to a system call failure), or the .errno property to
+return a string containing the Python errno name if one is known
+(which may be None if the numeric value does not correspond to
+a known errno name).
 '''

 Error.string = property (lambda self: self.args[0])
@@ -3961,6 +3964,8 @@ def _errno (self):
         return None
 Error.errno = property (_errno)

+Error.errnum = property (lambda self: self.args[1])
+
 def _str (self):
     if self.errno:
         return (\"%%s (%%s)\" %% (self.string, self.errno))




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