Fix bug in utils.load_json_file when pyjson5 encounters errors

hab/utils.py

@@ -297,7 +297,11 @@ def load_json_file(filename):
             data = json.load(fle)
         # Include the filename in the traceback to make debugging easier
         except _JsonException as e:
-            # pyjson5 is installed
+            # pyjson5 is installed add filename to the traceback
+            if e.result is None:
+                # Depending on the exception result may be None, convert it
+                # into a empty dict so we can add the filename
+                e.args = e.args[:1] + ({},) + e.args[2:]
             e.result["filename"] = str(filename)
             raise e.with_traceback(sys.exc_info()[2]) from None
         except ValueError as e:

tests/test_parsing.py

@@ -20,6 +20,64 @@
 from hab.parsers import Config, DistroVersion, FlatConfig
 
 
+class TestLoadJsonFile:
+    """Tests various conditions when using `hab.utils.load_json_file` to ensure
+    expected output.
+    """
+
+    def test_missing(self, tmpdir):
+        """If the file doesn't exist, the exception includes the missing filename."""
+        path = Path(tmpdir) / "missing.json"
+        with pytest.raises(
+            FileNotFoundError, match="No such file or directory:"
+        ) as excinfo:
+            utils.load_json_file(path)
+        assert Path(excinfo.value.filename) == path
+
+    def test_binary(self, tmpdir):
+        """If attempting to read a binary file, filename is included in exception.
+
+        This is a problem we run into rarely where a text file gets
+        replaced/generated with a binary file containing noting but a lot of null bytes.
+        """
+        path = Path(tmpdir) / "binary.json"
+        # Create a binary test file containing multiple binary null values.
+        with path.open("wb") as fle:
+            fle.write(b"\x00" * 32)
+
+        # Detect if using pyjson5 or not
+        native_json = False
+        try:
+            import pyjson5
+        except ImportError:
+            native_json = True
+        if native_json:
+            exc_type = json.decoder.JSONDecodeError
+        else:
+            exc_type = pyjson5.pyjson5.Json5IllegalCharacter
+
+        with pytest.raises(exc_type) as excinfo:
+            utils.load_json_file(path)
+
+        if native_json:
+            # If built-in json was used, check that filename was appended to the message
+            assert f'Filename("{path}")' in str(excinfo.value)
+        else:
+            # If pyjson5 was used, check that the filename was added to the result dict
+            assert f"{{'filename': {str(path)!r}}}" in str(excinfo.value)
+
+    def test_config_load(self, uncached_resolver):
+        cfg = Config({}, uncached_resolver)
+
+        # Loading a directory raises a FileNotFoundError
+        with pytest.raises(FileNotFoundError):
+            cfg.load(".")
+
+        # Loading a non-existent file path raises a FileNotFoundError
+        with pytest.raises(FileNotFoundError):
+            cfg.load("invalid_path.json")
+
+
 def test_distro_parse(config_root, resolver):
     """Check that a distro json can be parsed correctly"""
     forest = {}
@@ -632,14 +690,6 @@ def test_misc_coverage(resolver):
     cfg.version = "1.0"
     assert cfg.version == Version("1.0")
 
-    # Loading a directory raises a FileNotFoundError
-    with pytest.raises(FileNotFoundError):
-        cfg.load(".")
-
-    # Loading a non-existent file path raises a FileNotFoundError
-    with pytest.raises(FileNotFoundError):
-        cfg.load("invalid_path.json")
-
 
 @pytest.mark.parametrize(
     "dirname,uri",