Better error handling for gettext and gettext_lazy issues

[Manuel Saelices]

It's very common get confused in use of gettext and gettext_lazy functions. An usual error was to use gettext at import time, for example, in any of this cases:

• In a parameter of settings.py file:

  from django.utils.translation import ugettext as _
  ...
  LANGUAGE_CHOICES = (('es', _('Spanish')),
                      ('en', _('English')),)
  

• In class or field definition of a models.py file:

  from django.db import models
  from django.utils.translation import ugettext as _
  
  class Author(models.Model):
      name = models.CharField(_(u'Name'), max_length=100)
  

In both cases, if you get an error, the error will be an ImportError usually in another part of code... This is very difficult to debug, because the real error should be "Incorrect use of gettext. You can't use gettext at import time".

The error is due to the fact of not all applications has been imported yet, and gettext resolution need to have all possible translation files loaded (django.po files), which implies loading of all applications. This causes a circular import error problem.

In ​this thread there is a discussion about this.

We are thinking in check in every call to gettext if django apps has been loaded (using django.db.models.loading.app_cache_ready()) and take one of this options:

• raise an representative error.
• return a dummy string.

I personally prefers first one, because gettext should be always called inside functions (at run time), and never at import time. I don't imagine a case of we need use gettext before that.

[Manuel Saelices]

Previous patch pass all tests, but if you test in an apache deployment, it fails in rendering of 500 template, with this error:

Mod_python error: "PythonHandler django.core.handlers.modpython"

Traceback (most recent call last):

  File "/usr/lib/python2.5/site-packages/mod_python/apache.py", line 299, in HandlerDispatch
    result = object(req)

  File "/var/lib/python-support/python2.5/django/core/handlers/modpython.py", line 211, in handler
    return ModPythonHandler()(req)

  File "/var/lib/python-support/python2.5/django/core/handlers/modpython.py", line 184, in __call__
    response = self.get_response(request)

  File "/var/lib/python-support/python2.5/django/core/handlers/base.py", line 126, in get_response
    return self.handle_uncaught_exception(request, resolver, exc_info)

  File "/var/lib/python-support/python2.5/django/core/handlers/base.py", line 146, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)

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

  File "/home/lin/django_src/django/views/debug.py", line 113, in get_traceback_html
    return t.render(c)

  File "/var/lib/python-support/python2.5/django/template/__init__.py", line 176, in render
    return self.nodelist.render(context)

  File "/var/lib/python-support/python2.5/django/template/__init__.py", line 751, in render
    bits.append(self.render_node(node, context))

  File "/var/lib/python-support/python2.5/django/template/debug.py", line 81, in render_node
    raise wrapped

TemplateSyntaxError: Caught an exception while rendering: django.utils.translation.gettext function called at import time to perform a translation of 'Tue'. gettext function must be calledcalled after applications loading

Original Traceback (most recent call last):
  File "/var/lib/python-support/python2.5/django/template/debug.py", line 71, in render_node
    result = node.render(context)
  File "/var/lib/python-support/python2.5/django/template/debug.py", line 87, in render
    output = force_unicode(self.filter_expression.resolve(context))
  File "/var/lib/python-support/python2.5/django/template/__init__.py", line 542, in resolve
    new_obj = func(obj, *arg_vals)
  File "/var/lib/python-support/python2.5/django/template/defaultfilters.py", line 627, in date
    return format(value, arg)
  File "/var/lib/python-support/python2.5/django/utils/dateformat.py", line 264, in format
    return df.format(format_string)
  File "/var/lib/python-support/python2.5/django/utils/dateformat.py", line 29, in format
    pieces.append(force_unicode(getattr(self, piece)()))
  File "/var/lib/python-support/python2.5/django/utils/dateformat.py", line 174, in r
    return self.format('D, j M Y H:i:s O')
  File "/var/lib/python-support/python2.5/django/utils/dateformat.py", line 29, in format
    pieces.append(force_unicode(getattr(self, piece)()))
  File "/var/lib/python-support/python2.5/django/utils/encoding.py", line 49, in force_unicode
    s = unicode(s)
  File "/var/lib/python-support/python2.5/django/utils/functional.py", line 201, in __unicode_cast
    return self.__func(*self.__args, **self.__kw)
  File "/var/lib/python-support/python2.5/django/utils/translation/__init__.py", line 62, in ugettext
    return real_ugettext(message)
  File "/var/lib/python-support/python2.5/django/utils/translation/trans_real.py", line 295, in ugettext
    return do_translate(message, 'ugettext')
  File "/var/lib/python-support/python2.5/django/utils/translation/trans_real.py", line 278, in do_translate
    "called after applications loading" % message)
ImproperlyImplemented: django.utils.translation.gettext function called at import time to perform a translation of 'Tue'. gettext function must be calledcalled after applications loading

[Manuel Saelices]

I've found that previous real error was: TemplateDoesNotExist: 500.html but in DEBUG mode, fails in variables and context rendering.

[Manuel Saelices]

A better patch... but It's seems to have problems in apache detection of app_cache_ready

[Manuel Saelices]

A better patch... but It's seems to have problems in apache detection of app_cache_ready

[Manuel Saelices]

Now, the problem I get in apache is that always raise error, even when call gettext at run time. But the problem seems to be that app_cache_ready() doesnt work in Apache. Always returns False, even inside a django view.

[Manuel Saelices]

I think I find an error with apps installation. I don't find any code point that forces a django apps loading, and apps loading occurs if some things happends, like gettext call. For example, if in your first line of your django view, in a production server with mod_python, you call to django.db.models.loading.app_cache_ready() you get False.

I will attach a patch that fixes this problem, but changes Django core parts (django.core.handlers.base module). Ok, I can say that this core changes passes all tests and works in my dev and production server.

[Manuel Saelices]

This patch fixes apache problems, forcing apps loading just in request processing

[Malcolm Tredinnick]

This is pretty evil. :-(

We intentionally do lazy apps loading for a reason (but I cannot, for the life of me, remember why that is). I'll try to remember the reason, but I don't like forcing it like this. It's certainly not a bug that it happens lazily. I'd prefer to work around requiring it to be loaded. All that being said, your patch is a good start, so I'll have a read of it tonight.

[Manuel Saelices]

Replying to msaelices:

I've found that previous real error was: TemplateDoesNotExist: 500.html but in DEBUG mode, fails in variables and context rendering.

The final problem was Django tries to translate server_time to a spanish date, for rendering 500 template, in django.core.debug module. For fix that, I deactivated all translation in 500 error handling and I checked in do_translate that translations are activated, before raise an ImproperlyImplemented exception if is the case.

[Manuel Saelices]

Replying to msaelices:

Replying to msaelices:

I've found that previous real error was: TemplateDoesNotExist: 500.html but in DEBUG mode, fails in variables and context rendering.

The final problem was Django tries to translate server_time to a spanish date, for rendering 500 template, in django.core.debug module. For fix that, I deactivated all translation in 500 error handling and I checked in do_translate that translations are activated, before raise an ImproperlyImplemented exception if is the case.

do you refer to do in do_translate something like this?:

from django.db.models.loading import get_apps

def do_translate(message, translation_function):
    global _default, _active
    t = _active.get(currentThread(), None)
    try:
       get_apps()
    except ImportError:
       raise ImproperlyImplemented(
           "ImportError occurs, maybe it was caused because"
           "django.utils.translation.%(func_name)s function was called at "
           "import time to perform a translation of '%(message)s'. "
           "%(func_name)s function must be called after "
           "applications loading." % {'func_name': translation_function,
                                      'message': message})

[Manuel Saelices]

Replying to mtredinnick:

This is pretty evil. :-(

We intentionally do lazy apps loading for a reason (but I cannot, for the life of me, remember why that is). I'll try to remember the reason, but I don't like forcing it like this. It's certainly not a bug that it happens lazily. I'd prefer to work around requiring it to be loaded. All that being said, your patch is a good start, so I'll have a read of it tonight.

Sorry for my previous comment... was in bad thread.

About your work around... do you refer to do in do_translate something like this?:

from django.db.models.loading import get_apps

def do_translate(message, translation_function):
    global _default, _active
    t = _active.get(currentThread(), None)
    try:
       get_apps()
    except ImportError:
       raise ImproperlyImplemented(
           "ImportError occurs, maybe it was caused because"
           "django.utils.translation.%(func_name)s function was called at "
           "import time to perform a translation of '%(message)s'. "
           "%(func_name)s function must be called after "
           "applications loading." % {'func_name': translation_function,
                                      'message': message})
    ...

[Manuel Saelices]

And with respect lazy apps loading. I dont mean is a bug, but the fail may be in let apps rely on app_cache_ready, because maybe never get loaded. Maybe the reason for lazy apps loading was performance. If you count time of loading templatetags, etc. When you do a simple HttpResponseRedirect, may not have to import any apps.

[Malcolm Tredinnick]

@msaelices: how are you checking things with this patch? I realise it's pretty hard to write automated tests and I'm not really worried about that. I just want to make sure I'm covering all the possibilities you are when I make changes. So I'm using ugettext() in the __init__.py file of an app and in a model field definition. Anything else?

[Manuel Saelices]

It's very strange thing. I dont get to make fails django trunk, but I remember fixed several related issues by gettext_lazy. We also take a one of my company project and test it with actual django version and works. Also I try to update to a previous django revision (r8100) and works too.

What happening?

I will attach my test project, with uses gettext in forms.py and models.py, with two apps inside and one uses the other.

[Manuel Saelices]

Test project

[Malcolm Tredinnick]

I haven't had as much time to look at this as I would have liked, but I just gave it another read. My feeling is that it is over-engineered. Firstly, it's not always wrong to use ugettext() in a module at import time (because of dynamic import possibilities), but it usually is.

I also feel that the patch is trying to solve too large a problem. The real problem that we want to avoid is calling the translation machinery whilst apps are being imported. We have a way to tell when the app loading is finished. So probably all that we need is a way to tell is the app loading is in progress. How about adding an extra method/flag to the AppCacheclass to indicate that loading is in progress? Then we can test that before trying to access a translation and, if it's true, raise an error. The number of lines of code should be less and it catches the really nasty problem case.

Adding lots and lots of lines of code to handle an error situation that is a developer error is adding complexity that shouldn't be necessary. A clear error message to describe the problem is good, though, but I think we've gone a bit too far in trying to hold the hand of the developer in this patch.

[Manuel Saelices]

You're completely right. I'll try to make a better patch with your idea.

[Manuel Saelices]

Using a flag for no try translation while django apps is still loading

[Manuel Saelices]

@mtredinnick, I added a patch with your idea. But, I think we do not solve all kind of problems. Still there is a possibility to call ugettext before first app was tried to load. In this case, ugettext was no check any problem, and a circular import error will occurs (because this translation would begin apps loading).

What is your opinion?

[Malcolm Tredinnick]

This can wait until after 1.0. The patch still needs some work (we're aren't going to raise an exception saying Django isn't properly implemented, since that isn't true: this feature is about errors in the user's code). I'm also fairly sure it would be safe to use ugettext() before importing all models and that could be a genuine use-case (e.g. in cronjobs), but it needs testing and mistakes now would be stabilising.

Since this is only a diagnostic check and only triggered for code that is already broken, we can push it to later.

[(none)]

Milestone post-1.0 deleted

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Claude Paroz]

I think that after app loading refactor, the situation has improved a lot.
However, here is a patch to add a complementary error message to this sort of errors: ​https://github.com/django/django/pull/2838

[Aymeric Augustin]

If we're creating a specific exception for the app registry not being ready, it has no reason to inherit RuntimeError.

Furthermore, this patch changes the error message without updating the docs — which are important as this is going to be one of the pain points in 1.7.

[Aymeric Augustin]

Upon further thought, I'm in favor of introducing an AppRegistryNotReady exception and updating the documentation accordingly.

[Claude Paroz]

Pull request updated.

[Claude Paroz <claude@…>]

In 9618d68b345fe69c787f8426b07e920e647e05f3:

Fixed #8033 -- Explained app registry error during translation setup

Thanks Tim Graham and Aymeric Augustin for the review.

[Claude Paroz <claude@…>]

In f50a17785cdb67bc9d1b146d01c731cb6bfc48cf:

[1.7.x] Fixed #8033 -- Explained app registry error during translation setup

Thanks Tim Graham and Aymeric Augustin for the review.
Backport of 9618d68b34 from master.