alexanderwallin / node-gettext

A JavaScript implementation of gettext, a localization framework.
Other
188 stars 44 forks source link

Recipe to prevent race conditions with setLocale on server-side? #67

Open aldipower opened 1 year ago

aldipower commented 1 year ago

Hi! I a found a potential problem, which could occur on high traffic sites with the concept of setLocale. Does anybody worked around this already?

I give you a code example straight away - imagine hundreds of simultaneous requests per second:

const Gettext = require('node-gettext');

// This is a "singleton" context
const gettext = new Gettext();

gettext.addTranslations('de', 'messages', JSON.parse(translationFile));

// This creates a new sub-context on each request
app.get('/:lang', function (req, res) {
    const locale = req.params.lang;

    // This set the locale in the upper context which is shared by each request!
    gettext.setLocale(locale);

    // Do some heavy work here ....

    // Again we are accessing the upper gettext-context
    res.send(gettext.gettext('Translation probably with the wrong locale!'));
});

If you read the code, you will see, this leads to race conditions on heavy requested sites! On low traffic sites you probably won't notice. But if your site gets more popular, s*it hits the fan.

How could we prevent this?

We could contextualize the setLocale itself?

For example:

gettext.setLocale(locale, "my-context-id");

res.send(gettext.gettext("my-context-id")('Translation probably with the wrong locale!'));

What do you think?

aldipower commented 1 year ago

I've found a workaround for the race condition. Performance wise it does not feel ideal, but at least it works. :)

If you initialize Gettext in each request, you won't have races.

const Gettext = require('node-gettext');

app.get('/:lang', function (req, res) {
    const gettext = new Gettext();

    gettext.addTranslations('de', 'messages', JSON.parse(translationFile));

    const locale = req.params.lang;

    gettext.setLocale(locale);

    // Do some heavy work here ....

    res.send(gettext.gettext('Translation probably with the wrong locale!'));
});
icastillejogomez commented 1 year ago

The problem with that is JSON.parse statement block the libuv event loop and has implies a performance degradation on each request. Maybe to improve this workaround we should parse the translationFile once outside the controller.

alexey-sh commented 1 year ago

@aldipower What about the following workaround?

// locales.js

const availableLocales = ['en', 'de'];
const locales = {};

availableLocales.forEach((localeKey) => {
  const gettext = require('node-gettext');
  gettext.addTranslations(localeKey, 'messages', JSON.parse(translationFile));
  gettext.setLocale(locale);
  locales[localeKey] = gettext;
});
module.exports = locales;
// api.js

const locales = require('./locales.js');

app.get('/:lang', function (req, res) {
    const gettext = locales[req.params.lang];

    res.send(gettext.gettext('yay'));
});
icastillejogomez commented 1 year ago

You have to create a singleton instance per request. This library doesn't offer you any other solution

alexanderwallin commented 1 year ago

You could also create a map of gettext instances based on locale, and then retrieve the correct one in the request.

const de = new Gettext()
de.addTranslations('de', 'messages', deTranslations)
de.setLocale('de')
// etc for other locales

const gettexts = {
  de: de,
  en_EN: en_EN,
}

app.get('/:lang', function(req, res) {
  const gettext = gettexts[req.params.lang]
})