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

BugFix(MQTT): send an event in ReceiveMessageHandler before completin… #3116

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

tmahmood-microsoft
Copy link
Contributor

@tmahmood-microsoft tmahmood-microsoft commented Feb 15, 2023

For MQTT, after receiving C2D message, the implementation was making an additional call to CompleteAsync(). This did not cause an issue if message handler was eventually only making call to Complete/Abandon/RejectAsync() but if an event was sent within message handler, the ACKs within CompleteAsync() get mismatched. The mismatch was due to awaited event call causing delay in ACK for the first lock token. i.e:

Flow with no event within message handler:

  • Add message lock token to queue.
  • Remove lock token from queue and complete ACK.
  • Add message lock token again to the queue.
  • Remove lock token from queue and complete ACK.

Flow with event within message handler:

  • Add message lock token to queue.
  • Add message lock token again to the queue.
  • Remove first lock token from queue but expects the second lock token.
  • Remove second lock token from queue but expects the first lock token.

@tmahmood-microsoft
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tmahmood-microsoft
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

// Messages with QoS = 1 need to be Acknowledged otherwise it results in mismatched Ack to IoT Hub
// causing next message being replayed and all subsequent messages being queued.
await CompleteIncomingMessageAsync(message).ConfigureAwait(false);
await TaskHelpers.CompletedTask.ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why await this instead of removing the async keyword from the method?

timstewartm pushed a commit to timstewartm/azure-iot-sdk-csharp that referenced this pull request May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants