Launchpad has imported 13 comments from the remote bug at https://bugs.freedesktop.org/show_bug.cgi?id=55899.
If you reply to an imported comment from within Launchpad, your comment will be sent to the remote bug automatically. Read more about Launchpad's inter-bugtracker facilities at https://help.launchpad.net/InterBugTracking. ------------------------------------------------------------------------ On 2012-10-12T07:19:56+00:00 Michael Vogt wrote: Created attachment 68472 Patch that allows exceptions with unicode error messages Hi, I got a crash report releated to unicode for "aptdaemon" and was able to trigger it with the following code fragment. """" #!/usr/bin/python # -*- coding: utf-8 -*- import dbus e = dbus.exceptions.DBusException(u"bäää") msg= e.get_dbus_message() """ it would be nice if dbus-python would support that, especially since get_dbus_message() returns a unicode object in py2. Attached is a patch with testcases that works for me and got a brief review from Barry Warsaw as well. I'm happy to adjust the patch as needed, just let me know your thoughts. Cheers, Michael Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/25 ------------------------------------------------------------------------ On 2012-10-12T10:43:29+00:00 Simon McVittie wrote: Comment on attachment 68472 Patch that allows exceptions with unicode error messages Review of attachment 68472: ----------------------------------------------------------------- "git format-patch"-formatted patches would be appreciated in future; dbus-python, like all other dbus subprojects, is maintained in git. Please actually run the tests during "make check". I realise dbus- python's test framework is pretty convoluted: the right place to plug in these tests is test/run-test.sh, near test-standalone.py. If the tests will compile, but fail, under their non-target version of Python, you could do something like: import dbus._compat if not dbus._compat.is_py3: print "SKIP: not the appropriate Python version" raise SystemExit(0) If they won't even compile under their non-target version of Python, you may have to do something like this: case `$PYTHON -c "print(__import__('sys').version)"` in (2*) echo "running test-exception-py2.py" $PYTHON "$DBUS_TOP_SRCDIR"/test/test-exception-py2.py || die "... failed" ;; (3*) echo "running test-exception-py3.py" $PYTHON "$DBUS_TOP_SRCDIR"/test/test-exception-py3.py || die "... failed" ;; esac ::: test/test-exception-py2.py @@ +4,5 @@ > +import unittest > + > +import dbus > + > +class DbusExceptionTestCase(unittest.TestCase): Pet annoyance: it's spelled "D-Bus", or DBus when you're restricted to C-style identifiers. (Not Dbus or D-BUS.) @@ +5,5 @@ > + > +import dbus > + > +class DbusExceptionTestCase(unittest.TestCase): > + """Test the DBusException str/unicode behavior with py2""" Please also test each case with a subclass of DBusException that sets the _dbus_error_name class member, like ServerError in test-service.py's RaiseDBusException: class ServerError(dbus.DBusException): """Exception representing a normal "environmental" error""" include_traceback = False _dbus_error_name = 'com.example.Networking.ServerError' (Every DBusException raised across D-Bus by an interoperable service should set its _dbus_error_name.) The most "realistic" version is where isinstance(_dbus_error_name, str), whatever that means in this Python version. I don't think anyone deliberately sets it to a unicode in Python 2 or a bytestring in Python 3. The expected result is that get_dbus_message() will still return "baaa", but str() and unicode() will return something more like "com.example.Networking.ServerError: baaa". ::: test/test-exception-py3.py @@ +8,5 @@ > +class DbusExceptionTestCase(unittest.TestCase): > + > + def test_dbus_exception(self): > + e = dbus.exceptions.DBusException("bäää") > + msg= e.get_dbus_message() Coding style: msg = e... As in the Python 2 case, please test that unicode(e) also works and gives the desired result. As in the Python 2 case, please also test with a subclass that sets _dbus_error_name. Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/26 ------------------------------------------------------------------------ On 2012-10-12T12:08:24+00:00 Michael Vogt wrote: Created attachment 68486 Updated Thanks for your patch review. The attached version should address the points you raised: - it uses git format-patch - integrates into make check - fixes the DBus spelling - add ServerError(dbus.DBusException) based test - fix coding style - add missing cases for the py3 test Please let me know if I missed anything. Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/27 ------------------------------------------------------------------------ On 2012-10-16T14:48:20+00:00 Barry Warsaw wrote: One quick question about this test: + def test_dbus_exception_convert_str_fail(self): + """Test that a non-ascii Exception fails to convert to str""" + with self.assertRaises(ValueError): + e = dbus.exceptions.DBusException(u"bä") + print str(e) Are you sure you get a ValueError here and not a UnicodeEncodeError? Python 2.7.3 (default, Sep 26 2012, 21:51:14) [GCC 4.7.2] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> a = u'b\xe4\xe4\xe4' >>> print a bäää >>> import dbus.exceptions >>> x = dbus.exceptions.DBusException(a) >>> str(x) Traceback (most recent call last): File "<stdin>", line 1, in <module> UnicodeEncodeError: 'ascii' codec can't encode characters in position 1-3: ordinal not in range(128) Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/30 ------------------------------------------------------------------------ On 2012-10-16T15:39:18+00:00 Michael Vogt wrote: Thanks Barry - you are right, its a UnicodeEncodeError, its a subclass of a ValueError this is why the test works. I can update the test (I guess that is actually the right thing to do). How is the patch looking otherwise? Sorry for rushing a bit but this is a relatively common error for my application so it would be great to have a fix :) Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/32 ------------------------------------------------------------------------ On 2012-10-16T17:39:48+00:00 Simon McVittie wrote: This appears to be LP#846044. It would be nice if you'd mentioned that. Unfortunately, this patch causes some regressions. I can tell you didn't run 'make check', because when I did, it caught them... Without the patch: >>> import dbus >>> e = dbus.DBusException('bees') >>> e._dbus_error_name = 'com.example.Badness' >>> str(e) 'com.example.Badness: bees' With it (same preparation steps): >>> str(e) 'bees' Fixed in the fdo55899 branch in: git://people.freedesktop.org/~smcv/dbus-python Please review, re-test. cgit at <http://cgit.freedesktop.org/~smcv/dbus- python/log/?h=fdo55899> when it gets round to refreshing. Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/34 ------------------------------------------------------------------------ On 2012-10-16T17:41:09+00:00 Simon McVittie wrote: Created attachment 68627 Support unicode messages for DBusException in Python 2 [commit message amended -smcv] Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/35 ------------------------------------------------------------------------ On 2012-10-16T17:41:27+00:00 Simon McVittie wrote: Created attachment 68628 Skip test_dbus_exception_convert_str_fail under unusual default encodings This would actually work fine if the default encoding was UTF-8 or Latin-1 or something. Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/36 ------------------------------------------------------------------------ On 2012-10-16T17:41:39+00:00 Simon McVittie wrote: Created attachment 68629 Use a form of assertRaises() that works in Python 2.6 Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/37 ------------------------------------------------------------------------ On 2012-10-16T17:41:52+00:00 Simon McVittie wrote: Created attachment 68630 Slightly better test coverage Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/38 ------------------------------------------------------------------------ On 2012-10-16T17:42:09+00:00 Simon McVittie wrote: Created attachment 68631 DBusException: override both __str__ and __unicode__ Avoid chaining up to the superclass, because that behaves particularly oddly. This fixes regression test failures: str(some_dbus_exception) was no longer prefixed with the D-Bus error name under Python 2. Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/39 ------------------------------------------------------------------------ On 2012-10-16T19:38:59+00:00 Barry Warsaw wrote: After applying all 5 patches, the test suite passes for me in both Python 2.7 and 3.2 on Ubuntu 12.10. Visual review of the patches: * I'm not sure that you need the lambda in test_dbus_exception_convert_str_fail(). It looks weird but I haven't tried it without the lambda. * Can you provide some more detail about the weird up-chaining you're seeing with Exception.__unicode__? * I'd probably swap this code around (I always dislike testing a negative when testing a positive will do just as well): + if self._dbus_error_name is not None: + return '%s: %s' % (self._dbus_error_name, s) + else: + return s i.e. if self._dbus_error_name is None: return s else: return '%s: %s' % (self._dbus_error_name, s) Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/41 ------------------------------------------------------------------------ On 2012-10-17T07:35:52+00:00 Michael Vogt wrote: Hello Simon, sorry about the regression my patch caused (and the lack of referencing LP: #846044 directly). I did run make check to ensure that my new part of the testsuite is executed but failed to see the regression. My mistake. I used ~smcv/dbus-python and commit d262628e89115bbcc33c439c77d33733c4a23491 to build a dbus-python 1.1.1.1-0ubuntu1 test deb and ran the test I outlined in https://bugs.launchpad.net/ubuntu/quantal/+source/dbus-python/+bug/846044/comments/24. This works fine when using your branch and without it fails with UnicodeEncodeError. The testsuite is also fine for me on both 2.7 and 3.2 now. So from my POV this is fine. Thanks a lot! Cheers, Michael Reply at: https://bugs.launchpad.net/ubuntu/+source/aptdaemon/+bug/846044/comments/42 ** Changed in: python-dbus Status: Unknown => Confirmed ** Changed in: python-dbus Importance: Unknown => Medium -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/846044 Title: software-center crashed with UnicodeEncodeError in get_dbus_message(): 'ascii' codec can't encode character u'\xfc' in position 65: ordinal not in range(128) To manage notifications about this bug go to: https://bugs.launchpad.net/aptdaemon/+bug/846044/+subscriptions -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
