search

RubyDevInc / paho.mqtt.ruby

Eclipse Public License 1.0
31 stars 19 forks source link

Thread abort after exception #22

Open arizz96 opened 6 years ago

arizz96 commented 6 years ago

Hi, during my tests, an exception has been raised and the MQTT client died after it. This line https://github.com/RubyDevInc/paho.mqtt.ruby/blob/c37bc2ebaa61b0c021a6177f09e04582300d0284/lib/paho-mqtt.rb#L103 force the thread to be aborted if some exception is raised. Do you consider it correct? Or maybe there are some exception that should be handled in some other ways. For example, if the packed body length is over 256MB, is it ok to kill the client permanently?

I'm still investigating on my error, that raised an Invalid packet type identifier and I'm not able to find out why, but the problem is that an error like this can block the whole application logic.

p-goudet commented 6 years ago

@arizz96 I think that you are right, exceptions should be handled in different ways regarding their type. I would investigate on the different kinds of exception that could be raised and try to provide a better handler. The current implementation is quite brutal and need improvements.

arizz96 commented 6 years ago

Ok @p-goudet let me know. I can help you on this if you need.

arizz96 commented 6 years ago

@p-goudet I'm trying to put all Logger calls into a unique method avoiding duplicate code and to use custom error in the whole project, here's the code: https://github.com/arizz96/paho.mqtt.ruby/pull/2

p-goudet commented 6 years ago

The exception mentioned occurs in the packet parser. The exception handling in this part is still really basic and should be improved to avoid blocking application logic. The client has an option "persistence" that enable the client to attempt to reconnect in case of minor failure. I would try to include some exceptions that could be rescued with the persistence option.

arizz96 commented 6 years ago

@p-goudet any news?

p-goudet commented 6 years ago

Sorry for this late answer. I am still working on the exception handler for packet with invalid format (which to be the cause of error). Basically, any packet with invalid format raise a exception. For most case the invalid packet should be ignore to not block the application logic. However, in some case, it would raise a protocol violation which would shut down the current connection. If the client is configured in persistent mode, it would try to automatically try to reconnect. I am currently writing the spec (that were missing) for the packet parsing part.

p-goudet commented 6 years ago

The latest version of raise exception if the packet format is invalid (PacketFormatException), which is easier to handle. Did you found why the exception was raised? If not could you provide some example to reproduce this?