iain / http_accept_language

Ruby on Rails plugin. Fishes out the Accept-Language header into an array.
http://rails-i18n.org/
782 stars 120 forks source link

Wrong language occasionally - potential threading issue? #68

Open mrgordon opened 6 years ago

mrgordon commented 6 years ago

We're seeing a frustrating issue where this gem works as expected most the time but sometimes in production we end up translating to the wrong language. This is on a very high volume web app where each server runs 16-32 threads of the Rails app under Unicorn.

With settings like

config.i18n.enforce_available_locales = true
config.i18n.default_locale = :en
config.i18n.fallbacks = [:en]
config.i18n.available_locales = [:en, :ja, :ko, :ru]

(note I think that fallbacks is deprecated so I plan to remove it but its been there for a long time)

99.9% of the time we will get the behavior we want but users habitually complain that they somehow get Russian despite having an accept language string like en-US,en;q=0.5 that we can see in our logs.

We previously experienced this issue more frequently before updating the gem to include this fix: https://github.com/iain/http_accept_language/commit/47b02bededf87c9d9923006bff6e6006645043ed

My best idea for why this is going awry is that there is another threading issue similar to the one I linked above. Perhaps the memoization at https://github.com/iain/http_accept_language/blob/master/lib/http_accept_language/railtie.rb#L14

Does this seem reasonable?

The only deviation from a normal install of your gem that we have is we do the following in an initializer to ensure engines also get the locale set in lieu of using the HttpAcceptLanguage::AutoLocale concern.

# The locale is set this way so that ALL ActionController::Base instances (ie                                                                                                                                             
# parent app + any engines) set the locale properly                                                                                                                                                                       
ActiveSupport.on_load :action_controller do
  prepend_before_filter { I18n.locale = http_accept_language.compatible_language_from(I18n.available_locales) || I18n.default_locale }
end

It almost always works (I can't reproduce the issue but it has been reported by multiple users) so threading does seem like a likely candidate.

Any ideas?

tappleby commented 5 years ago

Not sure if its the same issue but I ended up switching to I18n.with_locale to fix some issues with language occasionally "leaking" between tests.

around_action :switch_locale

def switch_locale(&action)
  locale = http_accept_language.compatible_language_from(I18n.available_locales) || I18n.default_locale
  I18n.with_locale(locale, &action)
end
KelseyDH commented 4 years ago

Replacing that @http_accept_language instance variable with a RequestStore variable could be a prudent solution, that goes further in promoting thread safety than Thread.current: https://github.com/ElMassimo/request_store_rails

However it's a pretty Railsy addition to this gem, which further suggests it would be a good idea to break this gem up into its parser component, and its rails plugin.