Listener thread died because of unable to parse a packet

[ChihayaK]

Describe the bug As the title stated, the entire listener thread terminated because parser.py raise an error.

To Reproduce Sorry but it seems happened just once and I wasn't able to reproduce the bug yet, but read though the code I found that it is caused by the header mismatched.

Expected behavior The packet should be droped and log a header error instead of the entire listener thread terminated?

Logs/output

 Exception in thread Thread-1:
 Traceback (most recent call last):
   File "/usr/lib/python3.8/threading.py", line 932, in _bootstrap_inner
     self.run()
   File "/usr/lib/python3.8/threading.py", line 870, in run
     self._target(*self._args, **self._kwargs)
   File "/.venv/lib/python3.8/site-packages/oscpy/server.py", line 338, in _run_listener
     self._listen()
   File "/.venv/lib/python3.8/site-packages/oscpy/server.py", line 386, in _listen
     for address, tags, values, offset in read_packet(
   File "/.venv/lib/python3.8/site-packages/oscpy/parser.py", line 424, in read_packet
     raise ValueError('packet is not a message or a bundle')
 ValueError: packet is not a message or a bundle

Platform (please complete the following information):

• OS: linux
• Python version: 3.8
• release 0.6.0

[sunnyking]

I have reproduced this behavior:

Python 3.8.10 (default, Jun  2 2021, 10:49:15) 
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from oscpy.server import OSCThreadServer
>>> osc = OSCThreadServer()
>>> def callback(*values):
...     print(f'got values: {values}')
... 
>>> sock = osc.listen(address='127.0.0.1', port=8000, default=True)
>>> osc.bind(b'/address', callback)
>>> # idle here and use nc to send in packet
>>>
>>> Exception in thread Thread-1:
Traceback (most recent call last):
  File "/usr/lib/python3.8/threading.py", line 932, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.8/threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "/home/ubuntu/.local/share/virtualenvs/valory-ppyBrvzu/lib/python3.8/site-packages/oscpy/server.py", line 338, in _run_listener
    self._listen()
  File "/home/ubuntu/.local/share/virtualenvs/valory-ppyBrvzu/lib/python3.8/site-packages/oscpy/server.py", line 386, in _listen
    for address, tags, values, offset in read_packet(
  File "/home/ubuntu/.local/share/virtualenvs/valory-ppyBrvzu/lib/python3.8/site-packages/oscpy/parser.py", line 424, in read_packet
    raise ValueError('packet is not a message or a bundle')
ValueError: packet is not a message or a bundle

by sending a hello world packet via nc: echo -n "hello world" | nc -4u -q1 localhost 8000

[tshirtman]

Hm, indeed it seems i purposely designed it to break in case of malformed messages, which is not particularly convenient if you can't ensure packets are always correctly formed.

the handle_message method call in _listen should probably be wrapped in a try/except block, as the other blocks before it, and optionally log the error.

[ChihayaK]

Hm, indeed it seems i purposely designed it to break in case of malformed messages, which is not particularly convenient if you can't ensure packets are always correctly formed.

the handle_message method call in _listen should probably be wrapped in a try/except block, as the other blocks before it, and optionally log the error.

I can do a pull request to implement above changes. Do you want me to do that?

[tshirtman]

That would be welcome :). though the code is a bit different currently from what i described, because i was looking at my local branch for asyncio/etc, but i'll make sure to preserve these changes before merging it (currently blocked by my inhability to run tests for async and sync in a smart way). So the place to put the try/except would be around the call to callbacks in the _listen function, i think.

[ChihayaK]

Isn't I suppose to place the try/except around the read_packet and just catching the ValueError ? From my understanding the exception was raised there?

[tshirtman]

Actually you are right, not sure why i got confused. :sweat_smile:

[ChihayaK]

Here you go, I did some testing and it seems working.

[ChihayaK]

Since the pr has been merged. Closing the issue.