Skip to content

Commit

Permalink
translation: skip strings which can not be serialized
Browse files Browse the repository at this point in the history
When string serialization fails, skip the string, unmark it as pending
and continue in other pending strings. The error is stored as a Change
object and the string is marked as needing editing.

This makes it possible to recover from the situation, but the string can
be lost as it is not serialized, the change will disappear on next upstream
update.

Right now this happens frequently with Fluent, but can happen with other
formats as well.

Fixes #9775
  • Loading branch information
nijel committed Oct 5, 2023
1 parent 8319239 commit 9183158
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 7 deletions.
1 change: 1 addition & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Not yet released.
* Configure VCS API credentials as a Python dict from environment variables.
* Improved accuracy of checks on plural messages.
* Enage page better shows stats.
* Strings which can not be saved to a file no longer block other strings to be written.

**Bug fixes**

Expand Down
12 changes: 10 additions & 2 deletions weblate/formats/ttkit.py
Original file line number Diff line number Diff line change
Expand Up @@ -2045,10 +2045,18 @@ def fixup(self, store):

class FluentUnit(MonolingualSimpleUnit):
def set_target(self, target: str | list[str]):
old_target = self.unit.target
old_source = self.unit.source
super().set_target(target)
self.unit.source = target
# This triggers serialization discovering any syntax issues
self.unit.to_entry()
try:
# This triggers serialization discovering any syntax issues
self.unit.to_entry()
except Exception:
# Restore previous content
self.unit.target = old_target
self.unit.source = old_source
raise

@cached_property
def flags(self):
Expand Down
91 changes: 91 additions & 0 deletions weblate/trans/migrations/0004_alter_change_action.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# Copyright © Michal Čihař <[email protected]>
#
# SPDX-License-Identifier: GPL-3.0-or-later

# Generated by Django 4.2.5 on 2023-10-05 10:40

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("trans", "0003_alter_project_access_control"),
]

operations = [
migrations.AlterField(
model_name="change",
name="action",
field=models.IntegerField(
choices=[
(0, "Resource update"),
(1, "Translation completed"),
(2, "Translation changed"),
(5, "New translation"),
(3, "Comment added"),
(4, "Suggestion added"),
(6, "Automatic translation"),
(7, "Suggestion accepted"),
(8, "Translation reverted"),
(9, "Translation uploaded"),
(13, "New source string"),
(14, "Component locked"),
(15, "Component unlocked"),
(17, "Committed changes"),
(18, "Pushed changes"),
(19, "Reset repository"),
(20, "Merged repository"),
(21, "Rebased repository"),
(22, "Failed merge on repository"),
(23, "Failed rebase on repository"),
(28, "Failed push on repository"),
(24, "Parse error"),
(25, "Removed translation"),
(26, "Suggestion removed"),
(27, "Search and replace"),
(29, "Suggestion removed during cleanup"),
(30, "Source string changed"),
(31, "New string added"),
(32, "Bulk status change"),
(33, "Changed visibility"),
(34, "Added user"),
(35, "Removed user"),
(36, "Translation approved"),
(37, "Marked for edit"),
(38, "Removed component"),
(39, "Removed project"),
(41, "Renamed project"),
(42, "Renamed component"),
(43, "Moved component"),
(44, "New string to translate"),
(45, "New contributor"),
(46, "New announcement"),
(47, "New alert"),
(48, "Added new language"),
(49, "Requested new language"),
(50, "Created project"),
(51, "Created component"),
(52, "Invited user"),
(53, "Received repository notification"),
(54, "Replaced file by upload"),
(55, "License changed"),
(56, "Contributor agreement changed"),
(57, "Screnshot added"),
(58, "Screnshot uploaded"),
(59, "String updated in the repository"),
(60, "Add-on installed"),
(61, "Add-on configuration changed"),
(62, "Add-on uninstalled"),
(63, "Removed string"),
(64, "Removed comment"),
(65, "Resolved comment"),
(66, "Explanation updated"),
(67, "Removed category"),
(68, "Renamed category"),
(69, "Moved category"),
(70, "Could not save string"),
],
default=2,
),
),
]
3 changes: 3 additions & 0 deletions weblate/trans/models/change.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ class Change(models.Model, UserDisplayMixin):
ACTION_REMOVE_CATEGORY = 67
ACTION_RENAME_CATEGORY = 68
ACTION_MOVE_CATEGORY = 69
ACTION_SAVE_FAILED = 70

ACTION_CHOICES = (
# Translators: Name of event in the history
Expand Down Expand Up @@ -431,6 +432,8 @@ class Change(models.Model, UserDisplayMixin):
(ACTION_RENAME_CATEGORY, gettext_lazy("Renamed category")),
# Translators: Name of event in the history
(ACTION_MOVE_CATEGORY, gettext_lazy("Moved category")),
# Translators: Name of event in the history
(ACTION_SAVE_FAILED, gettext_lazy("Could not save string")),
)
ACTIONS_DICT = dict(ACTION_CHOICES)
ACTION_STRINGS = {
Expand Down
12 changes: 8 additions & 4 deletions weblate/trans/models/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -2019,15 +2019,19 @@ def commit_files(

return True

def handle_parse_error(
self, error, translation=None, filename=None, reraise: bool = True
):
"""Handler for parse errors."""
def get_parse_error_message(self, error) -> str:
error_message = getattr(error, "strerror", "")
if not error_message:
error_message = getattr(error, "message", "")
if not error_message:
error_message = str(error).replace(self.full_path, "")
return error_message

def handle_parse_error(
self, error, translation=None, filename=None, reraise: bool = True
):
"""Handler for parse errors."""
error_message = self.get_parse_error_message(error)
if filename is None:
filename = self.template if translation is None else translation.filename
self.trigger_alert("ParseError", error=error_message, filename=filename)
Expand Down
9 changes: 8 additions & 1 deletion weblate/trans/models/translation.py
Original file line number Diff line number Diff line change
Expand Up @@ -766,10 +766,17 @@ def update_units(self, units, store, author_name, author_id):
pounit.set_explanation(unit.explanation)
pounit.set_source_explanation(unit.source_unit.explanation)
except Exception as error:
self.component.handle_parse_error(error, self, reraise=False)
report_error(
cause="Could not update unit", project=self.component.project
)
unit.state = STATE_FUZZY
# Use update instead of hitting expensive save()
Unit.objects.filter(pk=unit.pk).update(state=STATE_FUZZY)
unit.change_set.create(
action=Change.ACTION_SAVE_FAILED,
target=self.get_parse_error_message(error),

This comment has been minimized.

Copy link
@henry-torproject

henry-torproject Oct 11, 2023

Contributor

@nijel Thanks for putting this patch together.

I was testing this out, and I noticed that this line in throwing

AttributeError: 'Translation' object has no attribute 'get_parse_error_message'

Was it meant to be self.component.get_parse_error_message(error)?

This comment has been minimized.

Copy link
@nijel

nijel Oct 11, 2023

Author Member

Yes, thanks for spotting. I was too lazy to construct a testcase for this :-(. Fixed in 7b374ca

)
clear_pending.append(unit.pk)
continue

updated = True
Expand Down

0 comments on commit 9183158

Please sign in to comment.