adafruit / Adafruit_IO_Python

Adafruit IO Python Client Library
Other
225 stars 99 forks source link

disconnect errors not recoverable #11

Closed mgk closed 8 years ago

mgk commented 8 years ago

I have a publisher only application on an RPi2 that is event based and sends messages infrequently. It sometimes gets disconnected (possibly after inactivity as suggested here although my client runs for hours possibly days before I see the disconnect).

Whatever the cause of the disconnect clients should be able to reconnect, but adding an on_disconnect() callback does not work when there is a connection error as the Adafruit IO library throws a RuntimeError as shown below. Further, this error cannot be caught as it happens in the background thread managing the MQTT communication.

The net effect is that a disconnect when using loop_background() crashes the client.

Exception in thread Thread-1:
Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 810, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 763, in run
    self.__target(*self.__args, **self.__kwargs)
  File "/home/pi/tpenv/local/lib/python2.7/site-packages/paho/mqtt/client.py", line 2287, in _thread_main
    self.loop_forever()
  File "/home/pi/tpenv/local/lib/python2.7/site-packages/paho/mqtt/client.py", line 1261, in loop_forever
    rc = self.loop(timeout, max_packets)
  File "/home/pi/tpenv/local/lib/python2.7/site-packages/paho/mqtt/client.py", line 811, in loop
    rc = self.loop_read(max_packets)
  File "/home/pi/tpenv/local/lib/python2.7/site-packages/paho/mqtt/client.py", line 1075, in loop_read
    return self._loop_rc_handle(rc)
  File "/home/pi/tpenv/local/lib/python2.7/site-packages/paho/mqtt/client.py", line 1382, in _loop_rc_handle
    self.on_disconnect(self, self._userdata, rc)
  File "/home/pi/tpenv/local/lib/python2.7/site-packages/Adafruit_IO/mqtt_client.py", line 80, in _mqtt_disconnect
    raise RuntimeError('Unexpected disconnect with rc: {0}'.format(rc))
RuntimeError: Unexpected disconnect with rc: 1
tdicola commented 8 years ago

Thanks for raising the issue and apologies it took so long to be looked at. Great point about the exception causing trouble--I think my original intent was to only explode like this with an exception if something truly catastrophic happened and a retry couldn't work, but it looks like it causes more harm than good. I just changed it to instead log (to a debug level) the RC and move on to call the on disconnect handler. That way client code can at least check they're not connected and try to go on to connect again. Thanks again for raising the issue!

mgk commented 8 years ago

:+1: Yeah on reading the code it felt right to me too (as per your original intent). It was not until the disconnects happened that I started thinking about it the other way.