diff --git a/Lib/fontTools/ufoLib/errors.py b/Lib/fontTools/ufoLib/errors.py index f4a181c41..e05dd438b 100644 --- a/Lib/fontTools/ufoLib/errors.py +++ b/Lib/fontTools/ufoLib/errors.py @@ -1,3 +1,6 @@ +from __future__ import annotations + + class UFOLibError(Exception): pass @@ -7,7 +10,12 @@ class UnsupportedUFOFormat(UFOLibError): class GlifLibError(UFOLibError): - pass + def _add_note(self, note: str) -> None: + # Loose backport of PEP 678 until we only support Python 3.11+, used for + # adding additional context to errors. + # TODO: Replace with https://docs.python.org/3.11/library/exceptions.html#BaseException.add_note + (message, *rest) = self.args + self.args = ((message + "\n" + note), *rest) class UnsupportedGLIFFormat(GlifLibError): diff --git a/Lib/fontTools/ufoLib/glifLib.py b/Lib/fontTools/ufoLib/glifLib.py index 49e74bfcd..6dee9db30 100755 --- a/Lib/fontTools/ufoLib/glifLib.py +++ b/Lib/fontTools/ufoLib/glifLib.py @@ -416,17 +416,33 @@ class GlyphSet(_UFOBaseIO): if validate is None: validate = self._validateRead text = self.getGLIF(glyphName) - tree = _glifTreeFromString(text) - formatVersions = GLIFFormatVersion.supported_versions( - self.ufoFormatVersionTuple - ) - _readGlyphFromTree( - tree, - glyphObject, - pointPen, - formatVersions=formatVersions, - validate=validate, - ) + try: + tree = _glifTreeFromString(text) + formatVersions = GLIFFormatVersion.supported_versions( + self.ufoFormatVersionTuple + ) + _readGlyphFromTree( + tree, + glyphObject, + pointPen, + formatVersions=formatVersions, + validate=validate, + ) + except GlifLibError as glifLibError: + # Re-raise with a note that gives extra context, describing where + # the error occurred. + fileName = self.contents[glyphName] + try: + glifLocation = f"'{self.fs.getsyspath(fileName)}'" + except fs.errors.NoSysPath: + # Network or in-memory FS may not map to a local path, so use + # the best string representation we have. + glifLocation = f"'{fileName}' from '{str(self.fs)}'" + + glifLibError._add_note( + f"The issue is in glyph '{glyphName}', located in {glifLocation}." + ) + raise def writeGlyph( self, diff --git a/Tests/ufoLib/glifLib_test.py b/Tests/ufoLib/glifLib_test.py index ba5b1e7a4..8f48168d8 100644 --- a/Tests/ufoLib/glifLib_test.py +++ b/Tests/ufoLib/glifLib_test.py @@ -138,7 +138,8 @@ class GlyphSetTests(unittest.TestCase): def testReadGlyphInvalidXml(self): """Test that calling readGlyph() to read a .glif with invalid XML raises a library error, instead of an exception from the XML dependency that is - used internally.""" + used internally. In addition, check that the raised exception describes + the glyph by name and gives the location of the broken .glif file.""" # Create a glyph set with three empty glyphs. glyph_set = GlyphSet(self.dstDir) @@ -150,10 +151,16 @@ class GlyphSetTests(unittest.TestCase): invalid_xml = b"" Path(self.dstDir, glyph_set.contents["c"]).write_bytes(invalid_xml) - # Confirm that reading /c raises an appropriate error. + # Confirm that reading /a and /b is fine... glyph_set.readGlyph("a", _Glyph()) glyph_set.readGlyph("b", _Glyph()) - with pytest.raises(GlifLibError, match="GLIF contains invalid XML"): + + # ...but that reading /c raises a descriptive library error. + expected_message = ( + r"GLIF contains invalid XML\.\n" + r"The issue is in glyph 'c', located in '.*c\.glif.*\." + ) + with pytest.raises(GlifLibError, match=expected_message): glyph_set.readGlyph("c", _Glyph())