Fixed #32 and #33 (#34)

* Fixed #32 and #33

Author: joente

README.md

@@ -29,6 +29,7 @@
     * [join](#join)
     * [leave](#leave)
     * [emit](#emit)
+  * [Failed packages](#failed-packages)
 ---------------------------------------
 
 ## Installation
@@ -550,3 +551,27 @@ Emit an event to a room.
 Future which should be awaited. The result of the future will
 be set to `None` when successful.
 
+
+## Failed packages
+
+```python
+set_package_fail_file(fn: str = '') -> None
+```
+
+Configure a file name to dump the last failed package.
+
+Only the MessagePack data will be stored in this file, not the package
+header. This is useful for debugging packages which fail to unpack.
+Note that only a single fail file can be used which is active (or not) for
+all clients.
+
+When empty (`''`), a failed package will *not* be dumped to file.
+
+```python
+from thingsdb.client import set_package_fail_file
+
+set_package_fail_file('/tmp/thingsdb-invalid-data.mp')
+
+# When a package is received which fails to unpack, the data from this package
+# will be stored to file.
+```

thingsdb/client/__init__.py

@@ -1 +1,2 @@
 from .client import Client
+from .package import set_package_fail_file

thingsdb/client/package.py

@@ -1,5 +1,24 @@
 import struct
 import msgpack
+import logging
+from typing import Optional
+
+
+_fail_file = ''
+
+
+def set_package_fail_file(fn: Optional[str] = ''):
+    """Configure a file name to dump the last failed package.
+
+    Only the MessagePack data will be dumped in this file, not the package
+    header. This is useful for debugging packages which fail to unpack.
+    Note that only a single fail file can be used which is active (or not) for
+    all clients.
+
+    When empty, a failed package will not be dumped to file.
+    """
+    global _fail_file
+    _fail_file = fn
 
 
 class Package(object):
@@ -20,6 +39,18 @@ def extract_data_from(self, barray: bytearray) -> None:
                 barray[self.__class__.st_package.size:self.total],
                 raw=False) \
                 if self.length else None
+        except Exception as e:
+            if _fail_file:
+                try:
+                    with open(_fail_file, 'wb') as f:
+                        f.write(
+                            barray[self.__class__.st_package.size:self.total])
+                except Exception:
+                    logging.exception('')
+                else:
+                    logging.warn(
+                        f'Wrote the content from {self} to `{_fail_file}`')
+            raise e
         finally:
             del barray[:self.total]
 

thingsdb/client/protocol.py

@@ -194,6 +194,8 @@ def data_received(self, data: bytes) -> None:
             except Exception:
                 logging.exception('')
                 # empty the byte-array to recover from this error
+                logging.error(
+                    f'Exception above came from package: {self.package}')
                 self._buffered_data.clear()
             else:
                 tp = self.package.tp

thingsdb/room/roombase.py

@@ -152,8 +152,17 @@ def on_leave(self) -> None:
         pass
 
     async def _on_first_join(self):
-        await self.on_join()
-        self._wait_join.set_result(None)
+        fut = self._wait_join
+        self._wait_join = None
+        # Instead of using finally to set the result, we could also catch the
+        # exception and choose to set the exception to the future. (And only
+        # set the future result to None on success). That implementation
+        # would result in getting an exception from the join() method when the
+        # wait argument is used.
+        try:
+            await self.on_join()
+        finally:
+            fut.set_result(None)
 
     def _on_join(self, _data):
         loop = self.client.get_event_loop()

thingsdb/version.py

@@ -1 +1 @@
-__version__ = '1.0.0'
+__version__ = '1.0.1'