-
Notifications
You must be signed in to change notification settings - Fork 589
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
Fix Issue #633: Resubmit Selected Messages In Batch Mode #764
Conversation
…without running into message body corruption.
outboundMessage = bodyType == BodyType.ByteArray ? | ||
message.CloneWithByteArrayBodyType(messageText, messagesSplitContainer.Visible) : | ||
message.Clone(message.GetBody<Stream>(), messagesSplitContainer.Visible); | ||
if (messageText.GetType() == typeof(string)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this will always be true since GetMessageText returns a string. Have you tested with the other body types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I second that. The issue here is serialization/deserialization with the legacy SDK (track 0).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review! I did not, since we were only dealing with string inputs. I will debug further and see what I can do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @Anshuljkt!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added back the ByteArray check, but have a question: what is the downside of using the messageText directly for the clone, instead of using the GetBody method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought of a slightly different solution: avoid overriding the bodyType using the GetMessageText method. This way, the user selection in the resubmission dialog will apply. What do you think about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought of a slightly different solution: avoid overriding the bodyType using the GetMessageText method. This way, the user selection in the resubmission dialog will apply. What do you think about this?
It would require you to set that for every message, right? I think that people expect bodyType to be automatically set.
When is that serialization text created?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this scenario, we would set the bodyType for all messages in the batch based on the user input from the dialog box. If the expectation is that the user input is overridden, this solution would not work.
I am not sure where the serialization text comes from... (assuming you are referring to the @�strin3http://schemas.microsoft.com/2003/10/Serialization/�
string)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I am. I suggest the way to fix this is to avoid getting that string. It is probably due to reading a string message using GetBody<Stream>()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anshuljkt, I tested that and it worked so I created a PR for that, #776.
Thanks for your help, @ErikMogensen! Closing this PR as you have addressed the issue. |
To address issue #633 (Resubmit Selected Messages In Batch Mode issue)
I have added a check on the messageType's value and parsed it as a string if it is one. Also removed the added serialization text
"@\u0006string\b3http://schemas.microsoft.com/2003/10/Serialization/�G"
to prevent messages from becoming corrupted when resubmitting.Using this build, you should be able to resubmit batches of messages without running into message body corruption.