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 messages are always acked even if handler returns an error #1234

Closed
CodeMonkeyLeet opened this issue Oct 26, 2021 · 5 comments
Closed
Assignees
Labels
kind/bug Something isn't working pinned Issue does not get stale
Milestone

Comments

@CodeMonkeyLeet
Copy link
Contributor

Expected Behavior

MQTT pubsub and bindings components only Ack() a message if the app's callback returns without error, as documented by the Dapr APIs.

Actual Behavior

paho.mqtt.golang lib that Dapr MQTT components depend on always Ack() the message when the handler returns, even though both the pubsub and bindings components expect to be able to explicitly Ack() only on success of the app's handler.

See eclipse-paho/paho.mqtt.golang#459 for details.

Steps to Reproduce the Problem

  1. Modify the pubsub conformance test subscription handler to always return failure.
  2. Run the pubsub conformance test on any of the MQTT profiles several times.
    • Observe that the component moves on to the next message once all retries (5) are exhausted (i.e. the MQTT wrapper handler has returned to mqtt lib caller), even across subsequent runs of the test.
    • Compare with a different pubsub component (e.g. Kafka), the first message is continuously redelivered as it is not successfully acked, even across subsequent runs of the test.

Release Note

RELEASE NOTE: FIX MQTT messages are always acked even if handler returns an error

@CodeMonkeyLeet CodeMonkeyLeet added the kind/bug Something isn't working label Oct 26, 2021
@CodeMonkeyLeet
Copy link
Contributor Author

Note that in the (breaking) v5 version of the SDK, there is an issue tracking designs for potentially addressing this: eclipse-paho/paho.golang#53

@artursouza artursouza added this to the v1.6 milestone Oct 27, 2021
@artursouza artursouza added the pinned Issue does not get stale label Oct 27, 2021
@shivamkm07
Copy link
Contributor

/assign

@shivamkm07
Copy link
Contributor

paho.mqtt.golang lib fix to support switching off auto acks: eclipse-paho/paho.mqtt.golang#578

@shivamkm07
Copy link
Contributor

@artursouza this can be closed now.

@yaron2
Copy link
Member

yaron2 commented Jan 9, 2022

@artursouza this can be closed now.

Let's keep it open to track until it's updated in Dapr and resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working pinned Issue does not get stale
Projects
None yet
Development

No branches or pull requests

4 participants