Using customHandleAcks for MQTT <5

[fmvilas]

Description

I was wondering if there's a reason why the customHandleAcks function is explicitly disabled for MQTT versions below 5.

The reason I'm asking is to suit the following workflow:

1. Client receives a message.
2. Business logic is executed.
3. If business logic doesn't trigger an error, send ack. Otherwise, don't send it. This usually forces the broker to resend the message after a timeout period.

In short, I'd love to be able to decide if I want to ack or not. Ack'ing immediately is great for general purposes but it creates a situation where a message can be lost.

Proposal

I propose we replace this line with something like:

- this.options.customHandleAcks = (options.protocolVersion === 5 && options.customHandleAcks) ? options.customHandleAcks : function () { arguments[3](0) }
+ this.options.customHandleAcks = options.customHandleAcks || function () { arguments[3](0) }

Happy to provide a pull request if that sounds like something you'd like to have.

[fmvilas]

If that's helpful, that's what I had to do in order to make it work: master...fmvilas:master.

[fmvilas]

@YoDaMa thoughts?

Also, it looks like you may need some help with maintenance. Let me know how we can help from the AsyncAPI Initiative 😊

[YoDaMa]

@fmvilas let me double check the MQTT spec to see that this doesn't violate anything in 3.x versioning... but I like adding it optionally on face value.

Also yes, would love maintenance help. My email is in my github bio, and if you reach out we can talk more.

[scarry1992]

I had added customHandleAcks, because it is necessary by mqtt 5 spec, but 3x mqtt spec is declaring more straight behaviour for acking, so I don`t know..

[fmvilas]

I had added customHandleAcks, because it is necessary by mqtt 5 spec, but 3x mqtt spec is declaring more straight behaviour for acking, so I don`t know..

Yes, it's actually declared more straight but there's nothing saying you can't decide when exactly to ack, that's why I'm asking for this feature.

[fmvilas]

Just as an example, the Paho Java client has this feature: http://www.eclipse.org/paho/files/javadoc/index.html. See setManualAcks.

An extract for convenience:

public void setManualAcks(boolean manualAcks)

Description copied from interface: IMqttClient
If manualAcks is set to true, then on completion of the messageArrived callback the MQTT acknowledgements are not sent. You must call messageArrivedComplete to send those acknowledgements. This allows finer control over when the acks are sent. The default behaviour, when manualAcks is false, is to send the MQTT acknowledgements automatically at the successful completion of the messageArrived callback method.

[YoDaMa]

@scarry1992 let me know your thoughts on this but from a mqttv3 perspective I don't see anything in the spec that would be violated in adding this. Of course Paho also has it in Java but that doesn't necessarily mean it's right... But you've been the driver of v5 work so lmk.

[scarry1992]

Ok, I made fast review and I didn't see any violations too. But I think we have to check this situation at least in tests. So I think we could add this ability to mqtt v3 protocols.

[fmvilas]

I'm happy to update tests too. I may need guidance though, it will be my first contribution to the library.

[robertsLando]

MQTT 5.0.0 BETA is now available! Try it out and give us feedback: npm i mqtt@beta. It may fix your issues

[fmvilas]

Yay! Thanks! FYI @KhudaDad414 @Souvikns @oviecodes

[robertsLando]

@fmvilas Fixed?