Incorrect language appears on after (multiple) reloads

[EnTeQuAk]

Original issue: https://github.com/mozilla/addons/issues/412

I can reproduce this on the mobile site, not on the desktop-site so I assume it's a potential addons-frontend problem. It seems, that sometimes (for me it appears to change on every request) the precedence of the language in the url and the browser-language changes and the browser-language somehow takes over, or not…

For me, the description changes indeed to German almost after every other request and then again to russian. My Accept-Language: en,en-US;q=0.7,de;q=0.3 should not show German at all but prefer English content so this is even more confusing.

See https://github.com/mozilla/addons/issues/412 for a few more details.

I can't see anything weird in the console or network traffic that would explain the behavior, yet.

[muffinresearch]

For me, the description changes indeed to German almost after every other request and then again to russian. My Accept-Language: en,en-US;q=0.7,de;q=0.3 should not show German at all but prefer English content so this is even more confusing.

Accept-language headers shouldn't come into this at all since the url for the site has the locale as ru. This is because the general policy is to not second guess the users locale if defined by url. It's only looked up by headers if a locale is not present in the url of the original request to the addons-frontend service.

Looking at an api request directly e.g. https://addons.mozilla.org/api/v3/addons/addon/ublock-origin/?lang=ru the description of the language seems to change in the response every once in a while. There seems to be multiple languages shown randomly.

On that basis it seems to be an API issue.

[muffinresearch]

@diox would you have some time to look into this?

[diox]

It's definitely a server-side issue. We've spent some time on it with @eviljeff and @EnTeQuAk but we've been unable to reproduce on local/dev/stage so far, and we can't figure it out yet.

Random bits of information:

• Request goes through the LocaleAndAppURLMiddleware, and bypasses most of it because it's an API request. The important bits should be those 2 lines https://github.com/mozilla/addons-server/blob/master/src/olympia/amo/middleware.py#L91-L92
• Once a language has been activated the translation app should use it. See here: https://github.com/mozilla/addons-server/blob/aae227acc5b1406ef15f13c3f8629782a37ce986/src/olympia/translations/transformer.py#L47
• Then the serializer, seeing that there is a request.GET['lang'], should only return one translation, the one retrieved automatically by the translation transformer: https://github.com/mozilla/addons-server/blob/aae227acc5b1406ef15f13c3f8629782a37ce986/src/olympia/api/fields.py#L81
• I suspected the translation table might be in a weird state, but it has a UNIQUE constraint on ('id', 'locale') which should limit problems... Still, it would be nice to see what it looks like.
• It's happening for other add-ons in prod, like firebug. Not sure yet if it's happening for other locales
• I can reproduce even by adding random query string parameters, so it does not look like http caching, unless somehow a proxy is caching things without looking at the query string.

[muffinresearch]

Bumping this up to a p2 because it would nice get this one sorted so that the localization doesn't randomly appear broken for people.

[EnTeQuAk]

I can look at this tomorrow (Friday) and see if I can fix this somehow, mat already found quite a bit of useful stuff. Until then it's free for someone else to look into :)

[diox]

It's definitely caching related. We reproduced on stage with @EnTeQuAk and after he cleared the cache globally on stage and requested russian, I got a russian response despite asking for french.

My wild guess is that somehow the bit we do to include the locale in the database query is not working somehow, or that cache-machine does not care. It's still strange that we haven't seen this for non-api requests, though, so there is still a bit we're missing.

[diox]

Played with this locally:

>>> from django.utils.translation import activate
>>> activate('ru')
>>> from django.utils import translation
>>> translation.get_language()
'ru'

>>> a = Addon.objects.get(slug='mantherbit-charis-opoish')
15:25:48 django.db.backends:DEBUG (0.012) SELECT `addons`.`id` FROM `addons` WHERE (("ru"="ru") AND NOT (`addons`.`status` = 11) AND `addons`.`slug` = 'mantherbit-charis-opoish'); args=(11, 'mantherbit-charis-opoish') :/home/mat/.virtualenvs/olympia/local/lib/python2.7/site-packages/django/db/backends/utils.py:89
(...)

>>> from olympia.addons.views import AddonViewSet
>>> view = AddonViewSet.as_view({'get': 'retrieve'})
>>> from django.http import HttpRequest
>>> request = HttpRequest()
>>> request.method = 'GET'
>>> view(request, pk='mantherbit-charis-opoish')
15:26:28 django.db.backends:DEBUG (0.000) SELECT `addons`.`id` FROM `addons` WHERE (("None"="None") AND NOT (`addons`.`status` = 11) AND `addons`.`slug` = 'mantherbit-charis-opoish'); args=(11, 'mantherbit-charis-opoish') :/home/mat/.virtualenvs/olympia/local/lib/python2.7/site-packages/django/db/backends/utils.py:89
>>> translation.get_language()
'ru'

That's... bad. Very bad. The "None"="None" comes from https://github.com/mozilla/addons-server/blob/87efb2789b0c58f07f5cf6e18f67ee2f03f57deb/src/olympia/amo/models.py#L102-L110 and should be using get_language()... Maybe it's reset somehow inside DRF ? That seems weird, but I'll take a look.

Edit: even weirder, I don't see this every time...

[diox]

The findings in the previous comment seem to be a red herring, for 2 reasons:

• Unless it's redirecting, LocaleAndAppURLMiddleware will set request.LANG and activate() a language in every case. It falls back to prefixer.get_language() which falls back to prefixer.lang_from_accept_header() which falls back to settings.LANGUAGE_CODE. So en-US should have been activated for requests with no valid lang.
• cache-machine has its own thing to include the locale in the key. The <lang>=<lang> thing is presumably useful for MySQL query cache ?

The cache-machine key generation depends on a with_locale parameter that may end up being False in some situations, so it's worth investigating. I still don't understand why this issue would only come up for API requests though.

Edit: Actually with_locale is False when generating the key for query caching in cache-machine, it's only True for the cached_with helper, making the <lang>=<lang> mandatory if we want caching to depend on the language.

[diox]

Unsetting queryset on AddonViewSet, making get_queryset return Addon.objects.all() instead of calling super() (and adding a base_name to the addons.register() call in api_urls.py) seems to fix it.

DRF does warn about self.queryset caching, but that warning is for django's internal queryset results caching, which DRF avoids in its get_queryset() implementation by essentially calling .all() on the self.queryset. It's not enough for us, because cache-machine will not re-execute the query in that case!

The solution is really to avoid the queryset on view classes entirely (and not just DRF, though I don't think we have other class-based views at the moment) and implement get_queryset() every time instead. I'll work on that.

[ValentinaPC]

Verified this on AMO-dev FF53(Android 7.1.2). Refreshing the page while in add-on details, will not change the language that was set. Tried with different languages. Postfix video:

[muffinresearch]

@diox thanks for investigating and finding a fix and @EnTeQuAk for assisting with the debug efforts.