Opened 17 years ago

Closed 16 years ago

Last modified 8 years ago

#6353 closed (fixed)

debug.technical_500_response fails on exceptions having localized text in unicode

Reported by: peter.melvyn@… Owned by: Malcolm Tredinnick
Component: Core (Other) Version: dev
Severity: Keywords:
Cc: alex@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If I raise an exception with localized text e.g.

raise XXXInternalException, ugettext('_XXX_ERROR_...') % (...)

and it is caught in base.get_response() function, technical_500_response() instead of reporting localized exception's message complains with error:

UnicodeEncodeError: 'ascii' codec can't encode character u'\u0161'
in position 2: ordinal not in range(128)

Attachments (2)

6353.diff (1.1 KB ) - added by Karen Tracey <kmtracey@…> 17 years ago.
6353-2.diff (1.3 KB ) - added by Thomas Güttler 17 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 by Thomas Güttler, 17 years ago

Resolution: worksforme
Status: newclosed

Please try to reproduce this with current SVN. Please provide a traceback if the problem still exists and reopen the ticket.

I tried to get this error. But the debug template did not fail. It showed the UnicodeError, but
the debug page itself was OK.

comment:2 by Peter <Melvyn>, 17 years ago

Resolution: worksforme
Status: closedreopened

I retest it with trunk rev. 7337 and it is still the same.
To reproduce it, it is just enough to raise Exception with Non-ASCII-7 text in any view function. Instead yellow debug page with raised exception's text:

  1. the plain traceback is shown
  2. the Django internal error message is reported instead of raised one.

Here is traceback:

Traceback (most recent call last):

  File "C:\Python24\Lib\site-packages\django\core\servers\basehttp.py", line 277, in run
    self.result = application(self.environ, self.start_response)

  File "C:\Python24\Lib\site-packages\django\core\servers\basehttp.py", line 631, in __call__
    return self.application(environ, start_response)

  File "C:\Python24\Lib\site-packages\django\core\handlers\wsgi.py", line 205, in __call__
    response = self.get_response(request)

  File "C:\Python24\Lib\site-packages\django\core\handlers\base.py", line 120, in get_response
    return debug.technical_500_response(request, *exc_info)

  File "C:\Python24\Lib\site-packages\django\views\debug.py", line 74, in technical_500_response
    html = get_traceback_html(request, exc_type, exc_value, tb)

  File "C:\Python24\Lib\site-packages\django\views\debug.py", line 151, in get_traceback_html
    c = Context({

  File "C:\Python24\Lib\site-packages\django\utils\encoding.py", line 37, in smart_unicode
    return force_unicode(s, encoding, strings_only, errors)

  File "C:\Python24\Lib\site-packages\django\utils\encoding.py", line 53, in force_unicode
    s = unicode(str(s), encoding, errors)

UnicodeEncodeError: 'ascii' codec can't encode character u'\u017e' in position 4: ordinal not in range(128)
Last edited 8 years ago by Tim Graham (previous) (diff)

in reply to:  2 comment:3 by Karen Tracey <kmtracey@…>, 17 years ago

Replying to Peter <Melvyn>:

I retest it with trunk rev. 7337 and it is still the same.
To reproduce it, it is just enough to raise Exception with Non-ASCII-7 text in any view function.

It's not actually, I tried inserting:

raise Exception(Authors.objects.get(pk=535))

(Where that Author is one in my DB that I happen to know has non-Ascii characters in its representation) into one of my views and I get a proper debug page that begins:

Exception at /combosearch/
Sue de Nîmes

In order to reproduce what you report I have to change my Author model to not have a __unicode__ method but instead only __str__, which makes it an error in my code. So the question is what is this XXXInternalException you are raising? I'm not familiar with it -- is it something provided by Django? I believe the error you are seeing is resulting from it not correctly implementing a __unicode__ function. If it's part of Django it needs to be fixed there, but if it's part of your code this is not a bug in Django.

(The bare traceback page instead of a proper debug page is, I think, covered by #6094.)

comment:4 by jd <jdemoor@…>, 17 years ago

Has patch: set
Needs tests: set

I had a similar problem with internationalization turned on.
A GET request on the post_comment view in django.contrib.comments caused a Http404 error to be raised. The translation text for the error description contains a non-ASCII character and the results were the same as reported above.

I also tried raising the built-in Exception with a unicode string containing non-ASCII characters and had the same problem.
The following patch solves the problem for me. It should work as long as the raised exception is passed only one argument, otherwise exception.message is the empty string.

Index: django/views/debug.py
===================================================================
--- django/views/debug.py       (revision 7338)
+++ django/views/debug.py       (working copy)
@@ -150,7 +150,7 @@
     t = Template(TECHNICAL_500_TEMPLATE, name='Technical 500 template')
     c = Context({
         'exception_type': exc_type.__name__,
-        'exception_value': smart_unicode(exc_value, errors='replace'),
+        'exception_value': smart_unicode(exc_value.message, errors='replace'),
         'unicode_hint': unicode_hint,
         'frames': frames,
         'lastframe': frames[-1],
@@ -184,7 +184,7 @@
         'root_urlconf': settings.ROOT_URLCONF,
         'request_path': request.path[1:], # Trim leading slash
         'urlpatterns': tried,
-        'reason': str(exception),
+        'reason': smart_unicode(exception.message, errors='replace'),
         'request': request,
         'request_protocol': request.is_secure() and "https" or "http",
         'settings': get_safe_settings(),

comment:5 by Peter <Melvyn>, 17 years ago

Pls, forget XXXInternalException. The problem occurs by raising Python's built-in Exception with localized non-ASCII-7 text as well, e.g.
raise Exception, ugettext('_ERROR_UNABLE_LOCATE_XXXX')

comment:6 by Karen Tracey <kmtracey@…>, 17 years ago

Oh, OK, I see -- just plain old Python Exceptions throw errors when you pass them plain unicode strings containing non-Ascii chars as arguments and then try to convert the Exception to unicode. I was able to recreate the posted traceback with a view that does this:

    from django.utils.translation import ugettext
    from django.utils import translation
    translation.activate('sv')
    raise Exception, ugettext("Only POSTs are allowed")

Note there's a slightly different traceback pointing to a different problem when you change Exception to Http404:

Traceback (most recent call last):

  File "/homedir/django/trunk/django/core/servers/basehttp.py", line 277, in run
    self.result = application(self.environ, self.start_response)

  File "/homedir/django/trunk/django/core/servers/basehttp.py", line 631, in __call__
    return self.application(environ, start_response)

  File "/homedir/django/trunk/django/core/handlers/wsgi.py", line 205, in __call__
    response = self.get_response(request)

  File "/homedir/django/trunk/django/core/handlers/base.py", line 105, in get_response
    return debug.technical_404_response(request, e)

  File "/homedir/django/trunk/django/views/debug.py", line 187, in technical_404_response
    'reason': str(exception),

UnicodeEncodeError: 'ascii' codec can't encode character u'\xe4' in position 12: ordinal not in range(128)

Here the error path isn't allowing for the fact that the Exception might contain a non-Ascii message. But when you fix that with a smart_unicode as is used in the technical_500_response, you hit the same error as you see there.

A possible fix is to enhance Django's force_unicode to handle Exceptions specially. I'll attach a patch I've tested and that works. I'm not really sure, though, if it's the right place to fix the problem since I'm not very familiar with this code.

by Karen Tracey <kmtracey@…>, 17 years ago

Attachment: 6353.diff added

comment:7 by Thomas Güttler, 17 years ago

Triage Stage: UnreviewedAccepted

I would modify the patch of Karen Tracey a bit. Since an exception class can have
a custom unicode method, this would should be tried first. If this fails, and
errors='replace', then use encode exc.args.

by Thomas Güttler, 17 years ago

Attachment: 6353-2.diff added

in reply to:  7 comment:8 by Karen Tracey <kmtracey@…>, 17 years ago

Replying to guettli:

I would modify the patch of Karen Tracey a bit. Since an exception class can have
a custom __unicode__ method, this would should be tried first. ....

The first patch does try a custom __unicode__ method first. The test for "isinstance(s, Exception)" only happens if "hasattr(s, '__unicode__')" is False.

comment:9 by Thomas Güttler, 17 years ago

Yes, but an exception can have a custom __str__ method without a __unicode__ method. Maybe it is better
to leave the method smart_unicode() unchanged, and use something like this in the debug view:

try:
    reason=smart_unicode(exception, errors='replace')
except UnicodeError:
    reason=smart_unicode(repr(exception), errors='replace')

comment:10 by anonymous, 16 years ago

Cc: alex@… added

comment:11 by mbarkhau, 16 years ago

Owner: changed from nobody to Malcolm Tredinnick
Status: reopenednew

I get the following trace:

http://dpaste.com/70921/

Bisect shows the error was introduced in r7927

comment:12 by Malcolm Tredinnick, 16 years ago

Component: UncategorizedCore framework
milestone: 1.0
Needs tests: unset

comment:13 by Jacob, 16 years ago

Resolution: fixed
Status: newclosed

(In [8560]) Fixed #6353: better handle unicode in exception reasons.

comment:14 by Karen Tracey <kmtracey@…>, 16 years ago

Resolution: fixed
Status: closedreopened

You're going to hate me, but there were two problems referenced in this ticket, and sadly the change committed fixes neither of them.

First the original reporter mentions raising Exceptions with non-ASCII data. I was able to recreate this with a view that does:

    from django.utils.translation import ugettext
    from django.utils import translation
    translation.activate('sv')
    raise Exception, ugettext("Only POSTs are allowed")

Using [8581] this view still generates a bare traceback page:

Traceback (most recent call last):

  File "/home/kmt/django/trunk/django/core/servers/basehttp.py", line 277, in run
    self.result = application(self.environ, self.start_response)

  File "/home/kmt/django/trunk/django/core/servers/basehttp.py", line 634, in __call__
    return self.application(environ, start_response)

  File "/home/kmt/django/trunk/django/core/handlers/wsgi.py", line 222, in __call__
    response = self.get_response(request)

  File "/home/kmt/django/trunk/django/core/handlers/base.py", line 128, in get_response
    return self.handle_uncaught_exception(request, resolver, exc_info)

  File "/home/kmt/django/trunk/django/core/handlers/base.py", line 148, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)

  File "/home/kmt/django/trunk/django/views/debug.py", line 39, in technical_500_response
    html = reporter.get_traceback_html()

  File "/home/kmt/django/trunk/django/views/debug.py", line 97, in get_traceback_html
    'exception_value': smart_unicode(self.exc_value, errors='replace'),

  File "/home/kmt/django/trunk/django/utils/encoding.py", line 35, in smart_unicode
    return force_unicode(s, encoding, strings_only, errors)

  File "/home/kmt/django/trunk/django/utils/encoding.py", line 51, in force_unicode
    s = unicode(str(s), encoding, errors)

UnicodeEncodeError: 'ascii' codec can't encode character u'\xe4' in position 12: ordinal not in range(128)

Second jdemoor commented on seeing a similar problem when raising an Http404. The traceback there was slightly different. Changing the test view to be:

    from django.utils.translation import ugettext
    from django.utils import translation
    translation.activate('sv')
    raise Http404, ugettext("Only POSTs are allowed")

and still using [8581], this also still generates a bare traceback:

Traceback (most recent call last):

  File "/home/kmt/django/trunk/django/core/servers/basehttp.py", line 277, in run
    self.result = application(self.environ, self.start_response)

  File "/home/kmt/django/trunk/django/core/servers/basehttp.py", line 634, in __call__
    return self.application(environ, start_response)

  File "/home/kmt/django/trunk/django/core/handlers/wsgi.py", line 222, in __call__
    response = self.get_response(request)

  File "/home/kmt/django/trunk/django/core/handlers/base.py", line 109, in get_response
    return debug.technical_404_response(request, e)

  File "/home/kmt/django/trunk/django/views/debug.py", line 259, in technical_404_response
    'reason': smart_str(exception, errors='replace'),

  File "/home/kmt/django/trunk/django/utils/encoding.py", line 79, in smart_str
    return unicode(s).encode(encoding, errors)

UnicodeEncodeError: 'ascii' codec can't encode character u'\xe4' in position 12: ordinal not in range(128)

The provided patch included two parts. First a change to django/utils/encoding.py in force_unicode to recognize when it was handed an Exception and force the Exception args individually to unicode. That part was not in the commited change at all. Adding it to [8581] fixes the error seen in the first view (the case where Exception was raised). Instead of the bare traceback I get a proper debug page:

Exception at /

Endast POST är tillåtet

Request Method: 	GET
Request URL: 	http://lbox:8000/
Exception Type: 	Exception
Exception Value: 	

Endast POST är tillåtet

But the 2nd case of raising Http404 still fails:

Traceback (most recent call last):

  File "/home/kmt/django/trunk/django/core/servers/basehttp.py", line 277, in run
    self.result = application(self.environ, self.start_response)

  File "/home/kmt/django/trunk/django/core/servers/basehttp.py", line 634, in __call__
    return self.application(environ, start_response)

  File "/home/kmt/django/trunk/django/core/handlers/wsgi.py", line 222, in __call__
    response = self.get_response(request)

  File "/home/kmt/django/trunk/django/core/handlers/base.py", line 109, in get_response
    return debug.technical_404_response(request, e)

  File "/home/kmt/django/trunk/django/views/debug.py", line 259, in technical_404_response
    'reason': smart_str(exception, errors='replace'),

  File "/home/kmt/django/trunk/django/utils/encoding.py", line 79, in smart_str
    return unicode(s).encode(encoding, errors)

UnicodeEncodeError: 'ascii' codec can't encode character u'\xe4' in position 12: ordinal not in range(128)

because I don't think it is smart_str that is needed there, but rather smart_unicode. The 2nd part of the provided patch had smart_unicode there, not smart_str. smart_str doesn't seem to do the job. Changing it to smart_unicode results in a proper debug page:

Page not found (404)
Request Method: 	GET
Request URL: 	http://lbox:8000/

Endast POST är tillåtet

You're seeing this error because you have DEBUG = True in your Django settings file. Change that to False, and Django will display a standard 404 page.

So to summarize, the Exception error is fixed with the change to encoding.py, but since that was not included in the commited change it was not fixed by the commit.
The Http404 error needs both the change to encoding.py and deubg.py, plus it appears to need the originally proposed fix (smart_unicode), not smart_str as committed.

comment:15 by Malcolm Tredinnick, 16 years ago

Karen, doing blanket coverage of exception classes in force_unicode() will disguise real bugs. The two problems you report are both cases of exceptions being poorly constructed -- in the first case your example simply doesn't work in Python (and hence in Django), in the second case, there's some extra work potentially needed in the Http404 exception class. An exception class must be able to be displayed with its __str__ method, because that's how Python does it. So we need to make sure that if we're passing non-ASCII data into particular exception classes, they have a __str__ method which can display the right data.

Django should remain faithful to Python's default behaviour there -- if you do something that isn't valid in Python, it shouldn't suddenly become valid just because Django was in the middle. Hiding the problem inside force_unicode() runs contrary to that. That's why we chose to fix this for the specific problem reported in the ticket (unfortunately, it turns out that other problems were reported in the comments and these were overlooked, so thanks for your diligence in preparing a summary of the issues).

I think the only thing remaining to do to close this off is fixing Http404 with a decent __str__ method and then doing a quick audit of our other exception classes. People can then open tickets for any specific cases we may miss.

comment:16 by Malcolm Tredinnick, 16 years ago

*sigh* 30 minutes looking around Google code shows that people really don't appreciate the need for exceptions to be able to display themselves as ASCII. So we're going to have to hide the problem here just to avoid infinite bug reports. Sub-optimal. :-(

comment:17 by Karen Tracey, 16 years ago

I knew y'all were gonna hate me. (BTW I'm not the original reporter, for either case. I just tried to puzzle out what the problem was.) I know bare Python is not very helpful when ones does stuff like this, I've encountered (usually when playing around with tests that have unicode strings) a few "Unprintable exception object"s and that's usually a good reflection of what I feel like yelling at the screen when I see one. I guess I don't see the harm in turning them into something that can be properly displayed?

comment:18 by Malcolm Tredinnick, 16 years ago

Resolution: fixed
Status: reopenedclosed

(In [8588]) Fixed #6353 (again) by making force_unicode() and smart_str() a bit more robust
in the face of funky Exception instances. This is slightly symptomatic of
problems in the calling code, but we don't want to raise a secondary exception
whilst trying to display the first one. Based on a patch from Karen Tracey.

comment:19 by Malcolm Tredinnick, 16 years ago

The above commit puts the final Exception-specific workaround as the last option after all others. I've also made smart_str handle the same issues (trying to fix the general problem, not just the specific issue). This at least does the job with both the cases Karen points out. New cases should be opened as separate tickets.

comment:20 by Jacob, 13 years ago

milestone: 1.0

Milestone 1.0 deleted

comment:21 by GitHub <noreply@…>, 8 years ago

In 26619ad7:

Removed an untested and broken branch in force_bytes() (refs #6353).

The new test crashed in the removed branch. It's unclear if the branch has
value since c6a2bd9b962af1cdf46f964589e6023046cfa8ec didn't include tests.

Note: See TracTickets for help on using tickets.
Back to Top