Skip to content

Support Configurable Queue Client Encoding Strategy and Cross-Encoding Handling#1230

Merged
nytian merged 9 commits into
mainfrom
nytian/as-default-to-base64
Jul 10, 2025
Merged

Support Configurable Queue Client Encoding Strategy and Cross-Encoding Handling#1230
nytian merged 9 commits into
mainfrom
nytian/as-default-to-base64

Conversation

@nytian

@nytian nytian commented Jun 26, 2025

Copy link
Copy Markdown
Contributor

This PR adds support for configurable encoding strategies in Azure Storage Queue clients, enabling compatibility between UTF8 and Base64 encodings.

  • Settings Update: Added num QueueClientEncodingStrategy to AzureStorageOrchestrationServiceSettings for user configuration.
  • Client Configuration: Applied encoding strategy in AzureStorageClient, with a MessageDecodingFailed handler to manage cross-encoding scenarios, especially for base64 received utf8 message.
  • Improved DeserializeQueueMessageAsync to support both encoding types with fallback logic.

@nytian nytian requested a review from cgillum June 26, 2025 19:59

@cgillum cgillum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I haven't finished reviewing the full PR yet, but here are some initial comments.

public async Task CrossEncodingCompatibility_Base64ToUtf8()
{
string testName = "Base64ToUtf8";
string input = "世界! 🌍 Test with émojis and spéciål chàracters: ñáéíóúü";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would any of these tests have failed prior to this fix?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We originally supported UTF-8 clients receiving either UTF-8 or Base64 messages. However, if a Base64 client receives a UTF-8 message, it should fail unless with the error handling implemented in AzureStorageClient.cs.

Comment thread src/DurableTask.AzureStorage/QueueClientEncodingStrategy.cs Outdated

@cgillum cgillum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added a few more comments after finishing my review.

Comment thread src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceSettings.cs Outdated
Comment thread src/DurableTask.AzureStorage/MessageManager.cs Outdated
Comment thread src/DurableTask.AzureStorage/MessageManager.cs Outdated
// so that the client can successfully process it on the next attempt.
if (this.Settings.QueueClientEncodingStrategy == QueueClientEncodingStrategy.Base64)
{
options.MessageDecodingFailed += async (QueueMessageDecodingFailedEventArgs args) =>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems this handler will try to handle the decoding error automatically. But it seems we also have exception handling logic in other places. Which exception handling logic will actually get triggered?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This one is only for the case when a base64 queue client receives a message sent by a utf8 queue client. I added more comments to make it more clear.

Comment thread src/DurableTask.AzureStorage/Storage/AzureStorageClient.cs
Comment thread src/DurableTask.AzureStorage/MessageManager.cs Outdated
@nytian nytian requested a review from cgillum July 8, 2025 19:24
Comment thread src/DurableTask.AzureStorage/MessageManager.cs Outdated
@nytian nytian requested a review from cgillum July 9, 2025 18:01
Comment thread src/DurableTask.AzureStorage/MessageManager.cs Outdated
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.

2 participants