Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MQTT Manual Acknowledgement #450

Open
t-moe opened this issue Dec 4, 2023 · 5 comments
Open

MQTT Manual Acknowledgement #450

t-moe opened this issue Dec 4, 2023 · 5 comments
Labels
feature-request A feature should be added or improved. p3 This is a minor priority issue

Comments

@t-moe
Copy link

t-moe commented Dec 4, 2023

Describe the issue

When looking at the api doc of mqtt5.Mqtt5Client::on('messageReceived',...) and mqtt.MqttClientConnection::subscribe its not clear to me how errors in the callback are handled. Is the message acknowledged anyways, even if an error occurred during processing?

Lets say I temporarily cannot process the data in my client. Should I throw an error from my message callback?
E.g. I would expect that if I throw an error in my callback, that the callback is invoked again, with the same message (at least if using QoS=1)

Links

https://aws.github.io/aws-iot-device-sdk-js-v2/node/classes/mqtt5.Mqtt5Client.html#MESSAGE_RECEIVED
https://aws.github.io/aws-iot-device-sdk-js-v2/node/classes/mqtt.MqttClientConnection.html#subscribe

@t-moe t-moe added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Dec 4, 2023
@bretambrose
Copy link
Contributor

The SDK clients do not support bridging.

I've thought about it some and if we did add bridging support I don't think it would necessarily match your expectations. The way I envision it working is something along the lines of:

"Here's a publish and a function object. Call the function when you want the client to send the ack."

Seems sensible for Qos 1, but I haven't had reason to think about Qos 2 much (given IoT Core's lack of support there).

@bretambrose bretambrose removed the needs-triage This issue or PR still needs to be triaged. label Dec 4, 2023
@t-moe
Copy link
Author

t-moe commented Dec 4, 2023

Thanks for your answer. It seems that the sdk clients always auto acknowledge the messages then. This was unclear to me from the documentation.

But I like your suggestion. This is also how rumqttc does it:

  • you can call setManualAcks(true) to disable automatic acknowledgement
  • after receiving a message you can then invoke ack(&msg) on the client.

Adding this to aws-iot-device-sdk could probably be done in a non api breaking change, if manualAcks=false per default.

Could this be added?

@bretambrose bretambrose added feature-request A feature should be added or improved. and removed documentation This is a problem with documentation. labels Dec 4, 2023
@bretambrose
Copy link
Contributor

bretambrose commented Dec 4, 2023

In theory, yes. It would likely result in configuration that takes a differently-typed publish callback. It's a pretty significant amount of work, so not likely to happen soon. Let's leave this open for others to add their interest; this isn't the first time someone has asked about bridging functionality.

I should also add that if it does get added, it is more likely to be added to the MQTT5 client than the MQTT311 client (which is a lot more brittle and hard to extend)

@t-moe t-moe changed the title MQTT Acknowledgement MQTT Manual Acknowledgement Dec 4, 2023
@t-moe
Copy link
Author

t-moe commented Dec 4, 2023

I see that this takes some work to implement, if you want to pass an additional argument to the publish callback. Thats why I linked the rumqttc API, because there the callback signature stays the same no matter if you are using manual or automatic acknowledgements ;).

Anyway, thanks for considering adding support for this, in whichever form suits you best.

@jmklix jmklix added the p3 This is a minor priority issue label Dec 4, 2023
@bretambrose
Copy link
Contributor

Tracking issue here: awslabs/aws-c-mqtt#368

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p3 This is a minor priority issue
Projects
None yet
Development

No branches or pull requests

3 participants