miguelgrinberg / simple-websocket

Simple WebSocket server and client for Python.
MIT License
79 stars 17 forks source link

BrokenPipeError in _thread #29

Closed sanchay-hai closed 1 year ago

sanchay-hai commented 1 year ago

Hello,

We occasionally get BrokenPipeErrors from _thread which kills our server. Here is the stack trace:

  File "/usr/local/lib/python3.11/threading.py", line 975, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/local/lib/python3.11/site-packages/simple_websocket/ws.py", line 169, in _thread
    self.connected = self._handle_events()
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/simple_websocket/ws.py", line 249, in _handle_events
    self.sock.send(out_data)
BrokenPipeError: [Errno 32] Broken pipe

Not sure if there's anything we can do on the client side to handle this as it is happening in a thread not in our control. Maybe you need a try / catch in the thread so the thread doesn't die?

Related note, could you perhaps name the thread, right now it shows up as _thread, not the most descriptive name.

We're using simple-websocket==0.10.1

Thanks!

miguelgrinberg commented 1 year ago

Yes, good catch on this, the call to handle_events() should be inside a try/catch to prevent the thread from crashing. The broken pipe errors are really unpredictable, they happen when the client unexpectedly goes away, and there is really nothing that can be done. The server needs to prevent the thread from crashing and just assume the client is gone. I will put a fix for this.

sanchay-hai commented 1 year ago

If it helps, this is our current workaround

# Override ws._thread to catch BrokenPipeError and ignore it. Otherwise it kills our server
def override_thread_ws():
    orig_thread = Server._thread

    def overriden(self, *args, **kwargs):
        try:
            orig_thread(self, *args, **kwargs)
        except BrokenPipeError:
            LOGGER.warning(f"BrokenPipeError in simple_websocket _thread {self}")
            # this actually closes the connection, which allows us to clean up the call
            self.close()
            self.event.set()

    Server._thread = overriden
miguelgrinberg commented 1 year ago

@sanchay-hai Can I ask you to install the main branch of this package and confirm that there are no more issues with broken pipe errors in the background thread? Once you report success I'll issue a new release. Thanks.

sanchay-hai commented 1 year ago

Thank you! will try it out when I get the chance