Support Configurable Queue Client Encoding Strategy and Cross-Encoding Handling #1230
Conversation
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
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: ñáéíóúü"; |
There was a problem hiding this comment.
Would any of these tests have failed prior to this fix?
There was a problem hiding this comment.
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.
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
Added a few more comments after finishing my review.
src/DurableTask.AzureStorage/AzureStorageOrchestrationServiceSettings.cs
Outdated
Show resolved
Hide resolved
| // so that the client can successfully process it on the next attempt. | ||
| if (this.Settings.QueueClientEncodingStrategy == QueueClientEncodingStrategy.Base64) | ||
| { | ||
| options.MessageDecodingFailed += async (QueueMessageDecodingFailedEventArgs args) => |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
…e/durabletask into nytian/as-default-to-base64
This PR adds support for configurable encoding strategies in Azure Storage Queue clients, enabling compatibility between UTF8 and Base64 encodings.