Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#6353 closed (fixed)

debug.technical_500_response fails on exceptions having localized text in unicode

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

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@…> 6 years ago.
6353-2.diff (1.3 KB) - added by guettli 6 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 6 years ago by guettli

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to worksforme
  • Status changed from new to closed

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 follow-up: Changed 6 years ago by Peter <Melvyn>

  • Resolution worksforme deleted
  • Status changed from closed to reopened

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)

comment:3 in reply to: ↑ 2 Changed 6 years ago by Karen Tracey <kmtracey@…>

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 Changed 6 years ago by jd <jdemoor@…>

  • 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 Changed 6 years ago by Peter <Melvyn>

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 Changed 6 years ago by Karen Tracey <kmtracey@…>

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.

Changed 6 years ago by Karen Tracey <kmtracey@…>

comment:7 follow-up: Changed 6 years ago by guettli

  • Triage Stage changed from Unreviewed to Accepted

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.

Changed 6 years ago by guettli

comment:8 in reply to: ↑ 7 Changed 6 years ago by Karen Tracey <kmtracey@…>

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 Changed 6 years ago by guettli

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 Changed 6 years ago by anonymous

  • Cc alex@… added

comment:11 Changed 6 years ago by mbarkhau

  • Owner changed from nobody to mtredinnick
  • Status changed from reopened to new

I get the following trace:

http://dpaste.com/70921/

Bisect shows the error was introduced in r7927

comment:12 Changed 6 years ago by mtredinnick

  • Component changed from Uncategorized to Core framework
  • milestone set to 1.0
  • Needs tests unset

comment:13 Changed 6 years ago by jacob

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:14 Changed 6 years ago by Karen Tracey <kmtracey@…>

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 Changed 6 years ago by mtredinnick

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 Changed 6 years ago by mtredinnick

*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 Changed 6 years ago by kmtracey

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 Changed 6 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from reopened to closed

(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 Changed 6 years ago by mtredinnick

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 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.