glTF importer: better error messages when loading files

2 files changed, 80 insertions, 78 deletions

diff --git a/io_scene_gltf2/__init__.py b/io_scene_gltf2/__init__.py
index 4e105b69..6b4a88d0 100755
--- a/io_scene_gltf2/__init__.py
+++ b/io_scene_gltf2/__init__.py
@@ -15,7 +15,7 @@
 bl_info = {
     'name': 'glTF 2.0 format',
     'author': 'Julien Duroure, Scurest, Norbert Nopper, Urs Hanselmann, Moritz Becher, Benjamin Schmithüsen, Jim Eckerlein, and many external contributors',
-    "version": (1, 4, 36),
+    "version": (1, 4, 37),
     'blender': (2, 91, 0),
     'location': 'File > Import-Export',
     'description': 'Import-Export as glTF 2.0',
@@ -994,26 +994,28 @@ class ImportGLTF2(Operator, ImportHelper):
 
     def unit_import(self, filename, import_settings):
         import time
-        from .io.imp.gltf2_io_gltf import glTFImporter
+        from .io.imp.gltf2_io_gltf import glTFImporter, ImportError
         from .blender.imp.gltf2_blender_gltf import BlenderGlTF
 
-        self.gltf_importer = glTFImporter(filename, import_settings)
-        success, txt = self.gltf_importer.read()
-        if not success:
-            self.report({'ERROR'}, txt)
-            return {'CANCELLED'}
-        success, txt = self.gltf_importer.checks()
-        if not success:
-            self.report({'ERROR'}, txt)
+        try:
+            gltf_importer = glTFImporter(filename, import_settings)
+            gltf_importer.read()
+            gltf_importer.checks()
+
+            print("Data are loaded, start creating Blender stuff")
+
+            start_time = time.time()
+            BlenderGlTF.create(gltf_importer)
+            elapsed_s = "{:.2f}s".format(time.time() - start_time)
+            print("glTF import finished in " + elapsed_s)
+
+            gltf_importer.log.removeHandler(gltf_importer.log_handler)
+
+            return {'FINISHED'}
+
+        except ImportError as e:
+            self.report({'ERROR'}, e.args[0])
             return {'CANCELLED'}
-        print("Data are loaded, start creating Blender stuff")
-        start_time = time.time()
-        BlenderGlTF.create(self.gltf_importer)
-        elapsed_s = "{:.2f}s".format(time.time() - start_time)
-        print("glTF import finished in " + elapsed_s)
-        self.gltf_importer.log.removeHandler(self.gltf_importer.log_handler)
-
-        return {'FINISHED'}
 
     def set_debug_log(self):
         import logging
diff --git a/io_scene_gltf2/io/imp/gltf2_io_gltf.py b/io_scene_gltf2/io/imp/gltf2_io_gltf.py
index 1607979a..83969d94 100755
--- a/io_scene_gltf2/io/imp/gltf2_io_gltf.py
+++ b/io_scene_gltf2/io/imp/gltf2_io_gltf.py
@@ -22,6 +22,11 @@ from os.path import dirname, join, isfile
 from urllib.parse import unquote
 
 
+# Raise this error to have the importer report an error message.
+class ImportError(RuntimeError):
+    pass
+
+
 class glTFImporter():
     """glTF Importer class."""
 
@@ -52,21 +57,32 @@ class glTFImporter():
         ]
 
     @staticmethod
-    def bad_json_value(val):
-        """Bad Json value."""
-        raise ValueError('Json contains some unauthorized values')
+    def load_json(content):
+        def bad_constant(val):
+            raise ImportError('Bad glTF: json contained %s' % val)
+        try:
+            return json.loads(bytes(content), encoding='utf-8', parse_constant=bad_constant)
+        except ValueError as e:
+            raise ImportError('Bad glTF: json error: %s' % e.args[0])
+
+    @staticmethod
+    def check_version(gltf):
+        """Check version. This is done *before* gltf_from_dict."""
+        if not isinstance(gltf, dict) or 'asset' not in gltf:
+            raise ImportError("Bad glTF: no asset in json")
+        if 'version' not in gltf['asset']:
+            raise ImportError("Bad glTF: no version")
+        if gltf['asset']['version'] != "2.0":
+            raise ImportError("glTF version must be 2.0; got %s" % gltf['asset']['version'])
 
     def checks(self):
         """Some checks."""
-        if self.data.asset.version != "2.0":
-            return False, "glTF version must be 2"
-
         if self.data.extensions_required is not None:
             for extension in self.data.extensions_required:
                 if extension not in self.data.extensions_used:
-                    return False, "Extension required must be in Extension Used too"
+                    raise ImportError("Extension required must be in Extension Used too")
                 if extension not in self.extensions_managed:
-                    return False, "Extension " + extension + " is not available on this addon version"
+                    raise ImportError("Extension %s is not available on this addon version" % extension)
 
         if self.data.extensions_used is not None:
             for extension in self.data.extensions_used:
@@ -74,86 +90,70 @@ class glTFImporter():
                     # Non blocking error #TODO log
                     pass
 
-        return True, None
-
-    def load_glb(self):
+    def load_glb(self, content):
         """Load binary glb."""
-        header = struct.unpack_from('<4sII', self.content)
-        self.format = header[0]
-        self.version = header[1]
-        self.file_size = header[2]
+        magic = content[:4]
+        if magic != b'glTF':
+            raise ImportError("This file is not a glTF/glb file")
 
-        if self.format != b'glTF':
-            return False, "This file is not a glTF/glb file"
-
-        if self.version != 2:
-            return False, "GLB version %d unsupported" % self.version
-
-        if self.file_size != len(self.content):
-            return False, "Bad GLB: file size doesn't match"
+        version, file_size = struct.unpack_from('<II', content, offset=4)
+        if version != 2:
+            raise ImportError("GLB version must be 2; got %d" % version)
+        if file_size != len(content):
+            raise ImportError("Bad GLB: file size doesn't match")
 
+        glb_buffer = None
         offset = 12  # header size = 12
 
         # JSON chunk is first
-        type_, len_, json_bytes, offset = self.load_chunk(offset)
+        type_, len_, json_bytes, offset = self.load_chunk(content, offset)
         if type_ != b"JSON":
-            return False, "Bad GLB: first chunk not JSON"
+            raise ImportError("Bad GLB: first chunk not JSON")
         if len_ != len(json_bytes):
-            return False, "Bad GLB: length of json chunk doesn't match"
-        try:
-            json_str = str(json_bytes, encoding='utf-8')
-            json_ = json.loads(json_str, parse_constant=glTFImporter.bad_json_value)
-            self.data = gltf_from_dict(json_)
-        except ValueError as e:
-            return False, e.args[0]
+            raise ImportError("Bad GLB: length of json chunk doesn't match")
+        gltf = glTFImporter.load_json(json_bytes)
 
         # BIN chunk is second (if it exists)
-        if offset < len(self.content):
-            type_, len_, data, offset = self.load_chunk(offset)
+        if offset < len(content):
+            type_, len_, data, offset = self.load_chunk(content, offset)
             if type_ == b"BIN\0":
                 if len_ != len(data):
-                    return False, "Bad GLB: length of BIN chunk doesn't match"
-                self.glb_buffer = data
+                    raise ImportError("Bad GLB: length of BIN chunk doesn't match")
+                glb_buffer = data
 
-        return True, None
+        return gltf, glb_buffer
 
-    def load_chunk(self, offset):
+    def load_chunk(self, content, offset):
         """Load chunk."""
-        chunk_header = struct.unpack_from('<I4s', self.content, offset)
+        chunk_header = struct.unpack_from('<I4s', content, offset)
         data_length = chunk_header[0]
         data_type = chunk_header[1]
-        data = self.content[offset + 8: offset + 8 + data_length]
+        data = content[offset + 8: offset + 8 + data_length]
 
         return data_type, data_length, data, offset + 8 + data_length
 
     def read(self):
         """Read file."""
-        # Check this is a file
         if not isfile(self.filename):
-            return False, "Please select a file"
+            raise ImportError("Please select a file")
 
-        # Check if file is gltf or glb
         with open(self.filename, 'rb') as f:
-            self.content = memoryview(f.read())
+            content = memoryview(f.read())
 
-        self.is_glb_format = self.content[:4] == b'glTF'
+        if content[:4] == b'glTF':
+            gltf, self.glb_buffer = self.load_glb(content)
+        else:
+            gltf = glTFImporter.load_json(content)
+            self.glb_buffer = None
 
-        # glTF file
-        if not self.is_glb_format:
-            content = str(self.content, encoding='utf-8')
-            self.content = None
-            try:
-                self.data = gltf_from_dict(json.loads(content, parse_constant=glTFImporter.bad_json_value))
-                return True, None
-            except ValueError as e:
-                return False, e.args[0]
+        glTFImporter.check_version(gltf)
 
-        # glb file
-        else:
-            # Parsing glb file
-            success, txt = self.load_glb()
-            self.content = None
-            return success, txt
+        try:
+            self.data = gltf_from_dict(gltf)
+        except AssertionError:
+            import traceback
+            traceback.print_exc()
+            raise ImportError("Couldn't parse glTF. Check that the file is valid")
 
     def load_buffer(self, buffer_idx):
         """Load buffer."""