paolosalvatori / ServiceBusExplorer

The Service Bus Explorer allows users to connect to a Service Bus namespace and administer messaging entities in an easy manner. The tool provides advanced features like import/export functionality or the ability to test topic, queues, subscriptions, relay services, notification hubs and events hubs.
MIT License
2.01k stars 584 forks source link

Feature Request: Re-submit a single message with binary content created with Azure.Messaging.ServiceBus nuget package #712

Closed KinNeko-De closed 1 year ago

KinNeko-De commented 1 year ago

In the Service Bus Explorer you can already "repair and resubmit" messages from a dead letter queue.

The content of the service bus message is a byte array (in C#). You can use Protobuf to serialize an object and put it into the message.

As soon as you Resubmit the message with the Service Bus Explorer, the content of the message is not parsable by protobuf anymore. The reason for that is that the protobuf message is converted to a string and from that back to binary content. This change the content of the message.

Requirement: Service Bus Messages can be resubmitted if they contain binary (example protobuf) content. The content of the message is not changed.

Repro: I have created a repro for the azure service bus explorer because I requested the feature there also : https://github.com/KinNeko-De/repro-servicebusexplorer-protobuf

PoC: I have created a fork and hacked a solution into the existing code to resubmit messages without changing the content. https://github.com/KinNeko-De/ServiceBusExplorer I added a small checkbox in the existing dialog. I do not think this should be the final solution, but i am not so familiar with WPF.

An alternative solution would be add a to a specific menu item like repairAndResubmitMessageToolStripMenuItem.

I am open to other suggestions but will need some more time to implement this :)

Priority: The priority is not so high. Resubmit messages in production is not a common task. If so multiple messages have to be resubmitted. The most common usage is manual testing during development. For that, I have automated tests that send the same message again.

Ps.: I have currently no access to a service bus instance which makes it hard to test this.

SeanFeldman commented 1 year ago

I have currently no access to a service bus instance which makes it hard to test this.

A free account might help.

I added a small checkbox in the existing dialog. I do not think this should be the final solution, but i am not so familiar with WPF.

An alternative solution would be add a to a specific menu item like repairAndResubmitMessageToolStripMenuItem.

The tool is handling NServiceBus old encoding of the messages w/o any UI changes. Maybe the way it was done is the way to handle this case. Not sure if that's possible. A byte array should stay a byte array w/o conversion. Is that conversation to string and back happening during cloning of the original message?

A PR is a good way to have that discussion and see the changes.

KinNeko-De commented 1 year ago

A free account might help. From Azure

Start with USD200* Azure credit
You'll have 30 days to use it

I want to use this maybe in the near future for a project of me.

Is that conversation to string and back happening during cloning of the original message?

That is the main functionality of the repair and resubmit form. 3 handlings (i see 4 of them inside of MessageForm) reads out the message from the textbox as a string and converts it back to a message. The fourth one does not and does something similar to my code, but it must do something different

The tool is handling NServiceBus old encoding of the messages w/o any UI changes I do not see that NServiceBus specific behavior inside the code. There are comments that say cloning a message is not an option for body type ByteArray. Maybe this is where NServiceBus old encoding is handled. BodyType.ByteArray is only set in AttemptToReadByteArrayBody which fails for protobuf messages. I have to find out the exact error message for that. I will do that on Monday.

Protobuf messages are displayed inside the textbox. The last handling inside of ServiceBusHelper succeeds. That is great because at least strings are readable. BodyType for this kind of message is Stream. I used message.Clone(GetBodyStream) to clone the message. So I think the BodyType makes sense.

I did the changes some months ago, so I do not exactly know where the code goes through. I will try this on Monday too and will execute the repro. You gave me an idea (thx for that).. maybe it is caused by some difference between Windows.Azure.ServiceBus 6.2.2 and Azure.Messaging.ServiceBus 7.12.0 which I use. Thanks to you, I now know where the code should go through.

I will then start a pull request and stop this discussion here :)

KinNeko-De commented 1 year ago

After debugging the code i have to change the analysis To keep track of that i will do it here because i do not want to do that in a pull request. I see no other options in github.

  1. The message can not be parsed as a byte array.
  2. The message type is stream.
  3. if you resubmit multiple messages, then inside of MessageForm the brockeredMessage is null. In this case the following code is executed. `var messageText = serviceBusHelper.GetMessageText(message, MainForm.SingletonMainForm.UseAscii, out bodyType);

                                // For body type ByteArray cloning is not an option. When cloned, supplied body can be only of a string or stream types, but not byte array :(
                                outboundMessage = bodyType == BodyType.ByteArray ?
                                                  message.CloneWithByteArrayBodyType(messageText, messagesSplitContainer.Visible) :
                                                  message.Clone(message.GetBody<Stream>(), messagesSplitContainer.Visible);`

As "message.GetBody" is used, it does not destroy the message. I assume that brokeredMessage is the editable message in the form.

  1. If you resubmit one message, then inside of MessageForm the brockeredMessage is not null. In this case the following code is executed. outboundMessage = bodyType == BodyType.ByteArray ? brokeredMessage.CloneWithByteArrayBodyType(txtMessageText.Text, messagesSplitContainer.Visible) : brokeredMessage.Clone(txtMessageText.Text, messagesSplitContainer.Visible);

There the txtMessageText.Text is used and that destroy the message.

I will try to create a pull request for that as soon as i have a solution

ErikMogensen commented 1 year ago

@KinNeko-De, it would be wonderful if you implemented that in a separate class using the latest SDK for ServiceBus (Track 2). Preferably that class should reside in the ServiceBus project. It may be easier to find a solution using that SDK since the messages are byte arrays. Or less difficult... 😄

KinNeko-De commented 1 year ago

My first approach was to try to read this as readonlymemory but that failed with Windows.Azure.ServiceBus package. Azure.Messaging.ServiceBus uses readonlymemoryinternally as far as I understand the code. I have to try to parse ArraySegment tomorrow.

But after understanding the existing UI and code I have already 2 different solutions. The first one is to add a new menu analogous to "Resubmit selected message in batch mode" so that you explicitly can not change the message. Pro: No behavior-breaking change Cons: more complex in usage. The second is to never allow editing the message in case of the last try case where the content of the message is changed during parsing. Pro: The user has nothing to decide. Cons: It is a kind of behavior change because a user can have use cases where editing the message can be useful.

I will do 2 different forks for that as pull requests are easier to understand that my text :)

In both cases, I would start tomorrow with writing unit tests against ServiceBusHelper.GetMessageText(BrokeredMessage messageToRead, bool useAscii, out BodyType bodyType) to show the current behavior. That removes the need to have a real service bus to implement this.

I found the ServiceBus project. Thanks for the hint. I will try to reference and use that in my unit tests.


Implementing the ServiceBusHelper2 new in that project is a big task. There are 6 methods of GetMessageText and I found no unit tests that ensure that the new implementation act in all cases like the old implementation (something like NServiceBus old encoding). Also, the existing implementation is also valid for other languages like java. It is hard to not break that behavior.

I am also not completely sure that it is possible to reimplement all the try catches using Azure.Messaging.ServiceBus as the methods were changed. You can only get the Body as BinaryData or Stream.

Don't get me wrong. You all did a great job in implementing all that and I use this tool in multiple companies. The online version in azure portal is not that feature rich and destroys all protobuf (and I think all binary) messages. But I would set up the concept new with a new major version (maybe also 7 :D) with breaking changes and maybe with a dotnet core version in the background. I can not invest so much time in this project. And personally, I hate JSON inside of messages and so my motivation is not that high to support manually editing this. I am currently between changing companies, so I try to solve this for my current colleagues to make their life easier. We had these problems for multiple years and I have now the time to get deeper into it. But this can change at any time. Sorry.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

KinNeko-De commented 1 year ago

The issue is not stale. There is a pull request #722 with a suggested work around

KinNeko-De commented 1 year ago

722 merged