Error handling for MQTT5 seems inconsistent

[sophokles73]

Thanks a lot for providing this crate :-)

I am trying to use it with an MQTT 5 broker (Mosquitto) and am struggling with the error handling a little bit.

When my MQTT 5 client tries to open a connection to the wrong port, I get a Error::TcpTlsConnectFailure which seems correct and is what I also get when I try to use MQTT 3.

However, when I connect using MQTT 5 with invalid credentials, then I get a Error::Failure instead of something more specific like e.g. Error::ConnectReturn(ConnectReturnCode), which is what I get when connecting using MQTT 3.

When publishing a message using MQTT 5 to a topic that the client is not authorized to publish to, I also get a Error::Failure. I would have expected to get at least a Error::ReasonCode(ReasonCode) with the reason code sent by the broker.

Taking a look at token::on_failure5 I do not see any reference to the reason code that is (supposedly?) provided in ffi::MQTTAsync_failureData5::reasonCode.

After some debugging, it seems that in scenario 1, ffi::MQTTAsync_failureData5::code contains value -1 which results in the correct Error being created.
In scenario 2, ffi::MQTTAsync_failureData5::code contains value 135 (which I believe to be the MQTT 5 reason code returned in the CONNACK packet) which leads to the wrong Error being created because it is not an MQTT 3 error code.
In scenario 3, ffi::MQTTAsync_failureData5::code contains 0 and ffi::MQTTAsync_failureData5::reasonCode contains 135, which also leads to the wrong error being created.

My feeling is that the paho C library does not fill the code and reasonCode fields consistently. FMPOV it should populate the code field for generic client side errors that are agnostic of the protocol version, e.g. Can not connect to socket, and for all other MQTT 3 errors. But it should populate (only) the reasonCode field, if MQTT 5 is used and the request has failed with a server side error. The token::on_failure5 function could then easily create the correct type of Error, based on which field is populated.

[fpagliughi]

Agreed, the error reporting is poor.

My main complaint with the Paho C lib is that it doesn't report to the caller the reason why the underlying TCP/TLS connection to the server failed (address lookup failure, server not accepting connection, TLS credentials failed, etc). It logs these different problems, but doesn't return the reason to the app. We usually just get a connection failure, -1.

But I do believe that there are a number of error conditions that we could catch in the wrapper library, but are missing. I plan to make an effort to catch all these and do a better job overall with error reporting. I'm guessing that I may want the freedom to break the existing API, so I will do this as part of the next major version change v0.14, in the next month or two.