This doesn't look right to me. I think what you're trying to do is take an Exception object, find the simple repr of that object, and then use it as either part of a log message.
I'll write some comments on the approach you're taking and an alternative
inline with the patch.
On Fri, Dec 10, 2010 at 01:53:11PM -0500, James Antill wrote:
> ---
> yum/Errors.py | 2 +-
> yummain.py | 34 +++++++++++++++++++++++++++-------
> 2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/yum/Errors.py b/yum/Errors.py
> index 143c9a4..c1af4ad 100644
> --- a/yum/Errors.py
> +++ b/yum/Errors.py
> @@ -65,7 +65,7 @@ class YumRPMTransError(YumBaseError):
>
> class LockError(YumBaseError):
> def __init__(self, errno, msg, pid=0):
> - YumBaseError.__init__(self)
> + YumBaseError.__init__(self, msg)
> self.errno = errno
> self.msg = msg
> self.pid = pid
> diff --git a/yummain.py b/yummain.py
> index 0241684..39df675 100755
> --- a/yummain.py
> +++ b/yummain.py
> @@ -38,6 +38,26 @@ def main(args):
>
> yum.misc.setup_locale(override_time=True)
>
> + def exception2msg(e):
> + """ DIE python DIE! Which one works:
> + to_unicode(e.value); unicode(e); str(e);
> + Call this so you don't have to care. """
> + try:
> + return to_unicode(e.value)
> + except:
> + pass
> +
> + try:
> + return unicode(e)
> + except:
> + pass
> +
> + try:
> + return str(e)
> + except:
> + pass
> + return "<exception failed to convert to text>"
> +
AFAICT, this function is taking an Exception object and attempting to return
a string representation of it. The problem it seems to be addressing is
that Exception objects don't have a defined interface. So while some
Exceptions might have a value arg, others might have msg, others message,
others args[0], etc. This function is trying to get something reasonable no
matter what form the Exception object takes.
This function, though, has several problems:
* It can return either unicode or byte str. That means that when you got to
use the return value from this you have the potential to raise
a UnicodeError.
* It checks a very small subset of the common ways to express what the
Exception is as a string
I'd probably code a function like this instead:
def exception2unicode(e):
# add a function for each of the methods you want to try for converting
# from an exception object to a unicode string. They will be tried in
# order
conversion_funcs = (str, unicode, lambda e: str(e.args[0]), lambda e:
str(e.value))
msg = None
for func in conversion_funcs:
try:
msg = func(e)
except:
pass
else:
break
if msg:
return to_unicode(msg)
return u'<exception failed to convert to text>'
If I was using kitchen's to_unicode, I could shorten the list of functions
and skip the final to_unicode call::
from kitchen.text.converters import to_unicode
conversion_funcs = (to_unicode, lambda e: to_unicode(e.args[0]), lambda e:
to_unicode(e.value))
for func in conversion_funcs:
try:
return func(e)
except:
pass
return u'<exception failed to convert to text>'
The to_unicode() function in kitchen tries to take the simplerepr of an
object already so it's not necessary to try attaining it explicitly via the
str() function.
Note that the order of conversion_funcs is important. I see that you try
e.value before trying str(e). So you might want to try lambda e:
str(e.value) and lambda e: str(args[0]) before str... It all depends on what
message you're trying to get out of the function.
> def exUserCancel():
> logger.critical(_('\n\nExiting on user cancel'))
> if unlock(): return 200
> @@ -47,7 +67,7 @@ def main(args):
> if e.errno == 32:
> logger.critical(_('\n\nExiting on Broken Pipe'))
> else:
> - logger.critical(_('\n\n%s') % str(e))
> + logger.critical(_('\n\n%s') % exception2msg(e))
> if unlock(): return 200
> return 1
>
> @@ -56,14 +76,14 @@ def main(args):
>
> Log the plugin's exit message if one was supplied.
> ''' # ' xemacs hack
> - exitmsg = str(e)
> + exitmsg = exception2msg(e)
> if exitmsg:
> logger.warn('\n\n%s', exitmsg)
> if unlock(): return 200
> return 1
>
> def exFatal(e):
> - logger.critical('\n\n%s', to_unicode(e.value))
> + logger.critical('\n\n%s', exception2msg(e.value))
> if unlock(): return 200
> return 1
>
> @@ -96,8 +116,8 @@ def main(args):
> try:
> base.doLock()
> except Errors.LockError, e:
> - if "%s" %(e.msg,) != lockerr:
> - lockerr = "%s" %(e.msg,)
> + if exception2msg(e) != lockerr:
> + lockerr = exception2msg(e)
> logger.critical(lockerr)
> if not base.conf.exit_on_lock:
> logger.critical(_("Another app is currently holding the yum
> lock; waiting for it to exit..."))
All of these logger calls can potentially fail with a UnicodeError. The
reason is that the original exception2msg() function and my new
exception2unicode() function can return unicode strings in some circumstances
but the string format you're using will attempt to convert those to byte
strings.
With exception2msg(), you'd need to write these lines like this:
logger.critical(_(u'\n\n%s') % to_unicode(exception2msg(e)))
With exception2unicode() you can write it like this:
logger.critical(_(u'\n\n%s') % exception2unicode(e))
> @@ -117,7 +137,7 @@ def main(args):
> return exPluginExit(e)
> except Errors.YumBaseError, e:
> result = 1
> - resultmsgs = [unicode(e)]
> + resultmsgs = [exception2msg(e)]
> except KeyboardInterrupt:
> return exUserCancel()
> except IOError, e:
> @@ -158,7 +178,7 @@ def main(args):
> return exPluginExit(e)
> except Errors.YumBaseError, e:
> result = 1
> - resultmsgs = [unicode(e)]
> + resultmsgs = [exception2msg(e)]
> except KeyboardInterrupt:
> return exUserCancel()
> except IOError, e:
>
Here I'll just note that exception2msg() doesn't perform the same function
as unicode(). unicode() will normalize all the strings as unicode type (or
throw an exception). exception2msg() will chnage the exception object into
a str or a unicode object without throwing an exception ever at this point
in the code. But when uses the message in resultmsgs, it could throw an
exception if it's expecting unicode but exception2msg() returned a byte
str. exception2unicode() won't suffer from that problem.
-Toshio
pgpeKRIeWhBSm.pgp
Description: PGP signature
_______________________________________________ Yum-devel mailing list Yum-devel@lists.baseurl.org http://lists.baseurl.org/mailman/listinfo/yum-devel
