1315970 - Exception in .parse method reading file with incorrect encoding

Status: RESOLVED FIXED

(Localization Infrastructure and Tools :: compare-locales, defect)

Description

[Francesco Lodolo [:flod]]

Attachment: mailTurboMenu.properties

When running

entities, map = file_parser.parse()

On a file with wrong encoding, with 1.2 I get an exception, while 1.1 would simply log the error.

Traceback (most recent call last):
  File "/Users/flodolo/github/tmx_maker/tests/test_string_extraction.py", line 181, in testBrokenEnconding
    extraction.extractStrings()
  File "/Users/flodolo/github/tmx_maker/tmx_products/tmx_products.py", line 139, in extractStrings
    entities, map = file_parser.parse()
  File "/Users/flodolo/github/tmx_maker/libraries/compare-locales/compare_locales/parser.py", line 224, in parse
    for e in self:
  File "/Users/flodolo/github/tmx_maker/libraries/compare-locales/compare_locales/parser.py", line 237, in walk
    contents = ctx.contents
AttributeError: 'NoneType' object has no attribute 'contents'

Attached is the file generating this error.

Comment 1

[Axel Hecht [:Pike]]

This is a regression from bug 1290883.

Comment 2

[Axel Hecht [:Pike]]

Attachment: bug 1315970, failed loading should return empty parse lits,

When hitting an encoding error, or other IO problems, we should
return an empty parse result.
We used to do that, but regressed that behavior in bug 1290883.

For encoding errors, we could also choose to not error, but
often all strings are returning garbage in that case, not
just the ones with code points triggering the failure.

Review commit: https://reviewboard.mozilla.org/r/91706/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91706/

Comment 3

[Francesco Lodolo [:flod]]

Comment on attachment 8809037 [details]
bug 1315970, failed loading should return empty parse lits,

https://reviewboard.mozilla.org/r/91706/#review91594

Thanks, it makes sense and works as expected now.

Comment 4

[Axel Hecht [:Pike]]

https://hg.mozilla.org/l10n/compare-locales/rev/5735bcdca4f339e2aa08030774a18a3ae66dfcdc, marking FIXED.

Thanks for testing and the review.