vert-x3 / vertx-mqtt

Vert.x MQTT
Apache License 2.0
184 stars 86 forks source link

Manual ACK of QoS 1 2 message #232

Closed dometec closed 1 year ago

dometec commented 1 year ago

The problem I found in VertX MQTT Client implementation is that acknowledgment of the message is done by the library before passing the message to the application logic. So the application does not have the ability to mark the message as elaborated. In this way, MQTT messages at QoS1 or QoS2 can be lost before they can be elaborated, for example for a crash of the client, since the broker has received the acks for those messages, and for it, are successfully delivered.

I extended the message handler of the VertX client and I give the opportunity to application logic to ack the message.

A more detailed example is there: https://github.com/dometec/vertx-mqtt-flowcontrol-example

I'm sorry to not provide a unit test for it, but I think that it's not so easy...

vietj commented 1 year ago

@dometec I think the ack() method should be directly on the MqttPublishMessage interface and we don't need a sub interface for that.

In addition we don't need the second handler too, instead we should have an option in MqttClientOptions that says wether to auto ack the messages and the default value should be true.

ppatierno commented 1 year ago

@vietj +1 on your comments ;-)

vietj commented 1 year ago

ping @dometec

dometec commented 1 year ago

ok @vietj let me a few days and I'll prepare a new PR

ppatierno commented 1 year ago

@dometec there is no need for a new PR you can just update this one ;-)

dometec commented 1 year ago

Hi @ppatierno yes, unfortunately by modifying my commit on my branch this PR was automatically closed, I create a new one.