Michael Cooper [:mythmon] Reporter
	Description • 11 years ago

Right now, if a translator puts a string in Verbatim that messes with a format string, any page that uses that string raises an unhelpful 500. An example of a instance in which this has happened is bug 841392, in which a localizer submitted the translation "{0} antwoorden" for the string "{num} answers". This seems like an easy mistake to make, and we shouldn't punish localizers or users for this.

This might require patching Tower, or writing our own wrappers around gettext and company, or writing a custom string formatter. Since I think this generally happens *after* the call to gettext, we might just have to deal with it in string formatting instead of in the look up phase.

Options

* If a string format fails, just ignore that variable. For example, 'There are {num} {animals}.'.format(num=5) would return 'There are 5 {animals}' without raising an error (though we would probably log a warning or something.

* If a string format fails, go back and get the English version of the text. So in the Polish example in the first paragraph, would render as '11 replies' because '{0} antwoorden' did not properly format. Considering the order of calls to format and gettext, I'm not sure this is possible.

* Some third option?

	Will Kahn-Greene [:willkg] ET needinfo? me
	Comment 1 • 11 years ago

Whatever solution we pick has to have the following properties:

1. the site must continue to be usable
2. we must be alerted as soon as possible so that we can fix the string

I think anything short of that just creates new problems.


When I fixed this in Miro, I did two things:

1. create gettext wrappers that, like the Python logging module functions, also took the values to be interpolated as arguments. The wrappers would take the msgid, convert it, and then interpolate the variables. If any of those pieces failed, it'd return the msgid interpolated.

https://github.com/pculture/miro/blob/master/tv/lib/gtcache.py#L180

2. change the entire codebase to use our gettext wrappers

It wasn't a trivial project. I had the advantage that Miro was using the Python gettext stuff directly and wasn't going through several layers of middlemen.


I think it's a lot more difficult here since we have a bunch of different ways that values get interpolated.

We could modify the f and fe filters in Jingo (that's where it was erroring out in the problem today) such that if they throw an exception, we just return the string *without* the bits interpolated.

Speaking to Mike's options, I can't think of how we could do the first one var-by-var--it'd have to be all or nothing. It's essentially what I'm suggesting by modifying the f and fe filters.

I have no idea how to do the second one without seriously modifying the plumbing and changing the entire site. That'd be a huge project. So huge that I don't think it'd be worth it.

	Michael Cooper [:mythmon] Reporter
	Comment 2 • 11 years ago

Doing formatting var-by-var: https://gist.github.com/mythmon/a24456ba39eed0fae26f.

Python uses string.Format by default, but we can make a custom subclass that defines whatever get_value() behavior we want, like not throwing errors. I did this in another project, but couldn't find it again, so I wrote up a quick sketch. If we are already modifying |f and |fe, this wouldn't be too hard to do. Maybe we could even monkeypatch... no, lets not do that.

	Kadir Topal [:atopal]
	Comment 3 • 11 years ago

Let's invest some time to figure out what would be needed to fix the root cause of the breakage. I'm assuming that this would also benefit other apps that use the .po files like MDN and AMO.

	Will Kahn-Greene [:willkg] ET needinfo? me
	Comment 4 • 11 years ago

Just to be clear, we can't fix the root cause since the root cause is bad l10n strings.

Further, strings get translated in the code through a series of different paths. The problem step is when the string is interpolated--not when it's translated. That's a step that happens in a bunch of different ways.

Like I said in comment #1, I don't think it's possible to alleviate it across the board without turning this into a *huge* project, but we can probably alleviate the problem in specific code paths.

This is going to need a few days of research. Research should answer the following questions:

1. What are the different codepath variations for a string to be translated and then interpolated?

2. Can we reduce the number of codepath variations by changing the code so that there are fewer variations?

3. At which points can we tweak something such that interpolation doesn't kick up Python string formatting errors?

	Ricky Rosario [:rrosario, :r1cky]
	Comment 5 • 11 years ago

(In reply to Will Kahn-Greene [:willkg] from comment #4)
> This is going to need a few days of research.
2pts or 3?

Let's do this because these issues are getting annoying.

	Will Kahn-Greene [:willkg] ET needinfo? me
	Comment 6 • 11 years ago

I suspect it involves dives into tower, jinja, jingo, and the kitsune code to get a clear picture and answer the questions I asked above in comment #4.

My vote is to give it 3 points because that's far more likely enough time to wade through everything, get a clear view of the problem domain, and have a series of possible things we could implement.

	Ricky Rosario [:rrosario, :r1cky]
	Comment 7 • 11 years ago

3pts it is!! ^^

	Ricky Rosario [:rrosario, :r1cky]
	Comment 8 • 11 years ago

I guess this isn't really P1 since the site isn't really broken with it. It would just alleviate unexpected breakage at 9pm on a thursday night.

	Ricky Rosario [:rrosario, :r1cky]
	Comment 9 • 11 years ago

Moving this to the backlog to ponder it more. There may be other solutions might adding the linter to our deploy process.

	Ricky Rosario [:rrosario, :r1cky]
	Comment 10 • 11 years ago

See bug 850215 for the strings breaking emails instead of the site

	Ricky Rosario [:rrosario, :r1cky]
	Comment 11 • 11 years ago

Q1 is in the past. Moving to the future.

	Kadir Topal [:atopal]
	Comment 12 • 11 years ago

Pretty sure Dennis took care of this. \o/ Thanks, Will!