[async-mqtt5] Review questions

From: Ruben Perez (rubenperez038_at_[hidden])
Date: 2024-10-18 15:56:29

Hi Ivica,

First of all, thanks for submitting your library to Boost.

I've been reading the discussion and reference and have some questions for you:

1. The limitation listed on
https://spacetime.mireo.com/async-mqtt5/async_mqtt5/auto_reconnect.html
about concealing configuration issues looks concerning, as it can be
very frustrating for the user. As far as I know, Boost.Redis solved it
by including logging as part of its API, and Boost.MySQL did it by
providing an extra diagnostics output argument, populated on operation
cancellation, that informs the user of potential causes to the
problem. Neither of us are very happy with our solution, to be frank.
But I think it's a point that might be worth solving before going into
Boost. As an option, I have been looking into abusing Asio handler
tracking (https://live.boost.org/doc/libs/1_86_0/doc/html/boost_asio/overview/core/handler_tracking.html)
to log custom events. BOOST_ASIO_HANDLER_OPERATION might be a
possibility. What are your thoughts regarding this?

2. Quoting the allocators section
(https://spacetime.mireo.com/async-mqtt5/async_mqtt5/asio_compliance/allocators.html):
"[...] associates the allocator of the first packet in the queue to
the low-level Boost.Asio function async_write on the transport layer".
Does this mean that the queue might use the allocator bound to the
first async_publish operation that is issued? If this is the case,
wouldn't it be violating the requirement that all memory allocated by
an associated allocator is deallocated before the async operation
completes?

3. Quoting the executors section
(https://spacetime.mireo.com/async-mqtt5/async_mqtt5/asio_compliance/executors.html):
"The same executor must execute mqtt_client::async_run and all the
subsequent async_xxx operations". This sounds like an unusual
restriction. What happens if it is violated? Wouldn't it make more
sense to use the executor associated with async_run for intermediate
handlers, and then the async_xxx associated executor for the final
one?

4. I noticed that async_mqtt5 does not implement persistent and clean
sessions. I'm no MQTT expert, so I can't evaluate whether these
features are relevant or not - I just know that they exist. Is there a
reason why you chose to not implement them? If so, I'd suggest adding
these reasons somewhere in the docs.

5. According to what I read in the docs, message payloads might
contain UTF-8 encoded text or just data blobs, depending on the
payload_format_indicator property passed to publish. However,
std::string is always used for payloads. As I understand it, if I want
to send a binary blob, I need to copy my blob to a string (probably
with a reinterpret_cast in the way), and then setting
payload_format_indicator. Would it make sense to expose a simpler API
taking a blob (e.g. a std::vector<unsigned char>) to make this use
case safer?

6. I can't find any docs (other than the examples) on how to use TLS.
>From the simplest example
(https://spacetime.mireo.com/async-mqtt5/async_mqtt5/hello_world_over_tls.html)
it looks like setting up TLS requires defining some non-trivial
template specializations. I'd like these to be documented. On the
other hand, the specializations seem to be using only Boost.Asio
types, rather than user-defined types. What would happen if two
libraries separately define these hooks because they need MQTT over
TLS? Wouldn't it make more sense to provide an optional header
providing these functions?

7. What's the use of the TlsContext type in mqtt_client? It looks very
coupled to asio::ssl::stream.

8. I've noticed that async_receive returns with an error code whenever
a reconnection occurs, which forces the user to re-subscribe to
topics. This contrasts with how async_publish works, where
reconnection is transparent to the user. Is there a reason for this
difference? Would it make sense to implement automatic resubscription
on reconnection?

Regards,
Ruben.