Noissue - Improve error reporting for broken locale files

cms/sources.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-present eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
@@ skipping 156 matching lines @@
             pass
         return self.has_file(self.locale_filename(locale, page))
 
     def read_locale(self, locale, page):
         default_locale = self.read_config().get('general', 'defaultlocale')
         result = collections.OrderedDict()
         if locale != default_locale:
             result.update(self.read_locale(default_locale, page))
 
         if self.has_locale(locale, page):
-            filedata = self.read_file(self.locale_filename(locale, page))[0]
-            localedata = json.loads(filedata)
-            for key, value in localedata.iteritems():
-                result[key] = value['message']
+            filename = self.locale_filename(locale, page)
+            filedata, filepath = self.read_file(filename)
+            try:
+                localedata = json.loads(filedata)
+                for key, value in localedata.iteritems():
+                    result[key] = value['message']
+            except (AttributeError, ValueError):

[Vasily Kuznetsov, 2017/11/07 18:57:35]

ValueError if JSON parsing fails, AttributeError if `localedata` doesn't have
`iteritems` (that happens when it's not a dict).

[mathias, 2017/11/07 20:13:30]

What about the KeyError when 'message' is not in the value dict?

[Vasily Kuznetsov, 2017/11/27 18:35:23]

That's a bit of a different error, but sure, let's raise a comprehensible error
for that one too.

+                raise ValueError(
+                    'The content of translations file for page "{}", '
+                    'language "{}" ({}) is not a valid JSON dictionary'
+                    .format(page, locale, filepath)
+                )
 
         return result
 
     #
     # Template helpers
     #
 
     @staticmethod
     def template_filename(template):
         return 'templates/%s.tmpl' % template
@@ skipping 243 matching lines @@
             'resolve_link',
             'read_config',
             'read_template',
             'read_locale',
             'read_include',
             'exec_file',
         ]:
             setattr(source, fname, _memoize(getattr(source, fname)))
 
     return source