Catch corner case error conditions when parsing files

 - Make connect() return False
 - Rather than throwing an exception

melee/slpfilestreamer.py

@@ -7,18 +7,21 @@
 from enum import Enum
 import numpy as np
 
+
 # pylint: disable=too-few-public-methods
 class EventType(Enum):
-    """ Replay event types """
+    """Replay event types"""
+
     GECKO_CODES = 0x10
     PAYLOADS = 0x35
     GAME_START = 0x36
     PRE_FRAME = 0x37
     POST_FRAME = 0x38
     GAME_END = 0x39
-    FRAME_START = 0x3a
-    ITEM_UPDATE = 0x3b
-    FRAME_BOOKEND = 0x3c
+    FRAME_START = 0x3A
+    ITEM_UPDATE = 0x3B
+    FRAME_BOOKEND = 0x3C
+
 
 class SLPFileStreamer:
     def __init__(self, path):
@@ -50,46 +53,56 @@ def _is_new_frame(self, event_bytes):
         return False
 
     def dispatch(self, dummy):
-        """Read a single game event off the buffer
-        """
+        """Read a single game event off the buffer"""
         if self._index >= len(self._contents):
             return None
 
         if EventType(self._contents[self._index]) == EventType.PAYLOADS:
             cursor = 0x2
-            payload_size = self._contents[self._index+1]
+            payload_size = self._contents[self._index + 1]
             num_commands = (payload_size - 1) // 3
             for i in range(0, num_commands):
                 command = np.ndarray((1,), ">B", self._contents, cursor)[0]
                 command_len = np.ndarray((1,), ">H", self._contents, cursor + 0x1)[0]
-                self.eventsize[command] = command_len+1
+                self.eventsize[command] = command_len + 1
                 cursor += 3
 
             wrapper = dict()
             wrapper["type"] = "game_event"
-            wrapper["payload"] = self._contents[self._index : self._index+payload_size+1]
+            wrapper["payload"] = self._contents[
+                self._index : self._index + payload_size + 1
+            ]
             self._index += payload_size + 1
             return wrapper
 
         event_size = self.eventsize[self._contents[self._index]]
 
         # Check to see if a new frame has happened for an old file type
-        if self._is_new_frame(self._contents[self._index : self._index+event_size]):
+        if self._is_new_frame(self._contents[self._index : self._index + event_size]):
             wrapper = dict()
             wrapper["type"] = "frame_end"
             wrapper["payload"] = b""
             return wrapper
 
         wrapper = dict()
         wrapper["type"] = "game_event"
-        wrapper["payload"] = self._contents[self._index : self._index+event_size]
+        wrapper["payload"] = self._contents[self._index : self._index + event_size]
         self._index += event_size
 
         return wrapper
 
     def connect(self):
-        with open(self._path, mode='rb') as file:
-            full = ubjson.loadb(file.read())
+        with open(self._path, mode="rb") as file:
+            full = None
+            try:
+                full = ubjson.loadb(file.read())
+            except ubjson.decoder.DecoderException:
+                return False
+            # This is annoying and sometimes happens when there's an error parsing
+            if not isinstance(full, dict):
+                return False
+            if "raw" not in full:
+                return False
             raw = full["raw"]
             self._contents = raw
             try:

test.py

@@ -3,18 +3,22 @@
 
 import melee
 
+
 class SLPFile(unittest.TestCase):
     """
     Test cases that can be run automatically in the Github cloud environment
     In particular, there are no live dolphin tests here.
     """
+
     def test_read_file(self):
         """
         Load and parse SLP file
         """
-        console = melee.Console(system="file",
-                                allow_old_version=False,
-                                path="test_artifacts/test_game_1.slp")
+        console = melee.Console(
+            system="file",
+            allow_old_version=False,
+            path="test_artifacts/test_game_1.slp",
+        )
         self.assertTrue(console.connect())
         framecount = 0
         while True:
@@ -33,14 +37,13 @@ def test_read_file(self):
                 self.assertEqual(gamestate.players[1].percent, 17)
                 self.assertEqual(gamestate.players[2].percent, 0)
 
-
     def test_read_old_file(self):
         """
         Load and parse old SLP file
         """
-        console = melee.Console(system="file",
-                                allow_old_version=True,
-                                path="test_artifacts/test_game_2.slp")
+        console = melee.Console(
+            system="file", allow_old_version=True, path="test_artifacts/test_game_2.slp"
+        )
         self.assertTrue(console.connect())
         framecount = 0
         while True:
@@ -65,7 +68,33 @@ def test_framedata(self):
         """
         framedata = melee.FrameData()
         self.assertTrue(framedata.is_attack(melee.Character.FALCO, melee.Action.DAIR))
-        self.assertFalse(framedata.is_attack(melee.Character.FALCO, melee.Action.STANDING))
+        self.assertFalse(
+            framedata.is_attack(melee.Character.FALCO, melee.Action.STANDING)
+        )
+
+    def test_corrupt_file(self):
+        """Load a corrupt SLP file and make sure we don't crash"""
+        console = melee.Console(
+            system="file",
+            allow_old_version=True,
+            path="test_artifacts/corrupt_game_1.slp",
+        )
+        self.assertFalse(console.connect())
+
+        console = melee.Console(
+            system="file",
+            allow_old_version=True,
+            path="test_artifacts/corrupt_game_2.slp",
+        )
+        self.assertFalse(console.connect())
+
+        console = melee.Console(
+            system="file",
+            allow_old_version=True,
+            path="test_artifacts/corrupt_game_3.slp",
+        )
+        self.assertFalse(console.connect())
+
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     unittest.main()

test_artifacts/corrupt_game_1.slp

@@ -0,0 +1 @@
+This is not an SLP file at all!