Particular / NServiceBus.AzureServiceBus

The Azure ServiceBus Transport
https://docs.particular.net/nservicebus/azure-service-bus/
Other
13 stars 17 forks source link

Bug: Serialization interop support - bypass BrokeredMessage underlying serialization #49

Closed kevinhillinger closed 9 years ago

kevinhillinger commented 9 years ago

Using Azure Service Bus as a broker between an Android device and on premise NServiceBus... The serialization type is JSON. Android is using the ASB REST API to send the binary payload to the ASB queue.

A hard exception is hit in the AzureServiceBus Transport code. The problem is line 15 in the ToTransportMessage method in BrokeredMessageConverter:

var rawMessage = message.GetBody<byte[]>() ?? new byte[0];

The exception is thrown within Microsoft.ServiceBus: "There was an error deserializing the object of type System.Byte[]". The reason for this error is that when any object type is used with BrokeredMessage (incuding byte[]), the underlying DataContractSerializer is enlisted with an XmlBindary[Writer/Reader].

This problem is that this destroys interoperability, and also removes control of serialization from NServiceBus.

I've created a fix for it here.

The resolution is to use Stream whenever creating a BrokeredMessage and when retrieving the body. This also has the added benefit of giving NSB full control over serialization both sending and receiving.

kevinhillinger commented 9 years ago

I've added a working example of the changes I made in the fix branch:

https://github.com/kevinhillinger/NServiceBus.AzureAsb.Serialization.Interop

yvesgoeleven commented 9 years ago

Thank you @kevinhillinger

We have ran into this issue ourselves before and are aware of it. However, we cannot just change it as that would break wire compatibility for existing production deployments that may still have messages on their queues while upgrading, and there is no way to ask a brokered message how the data has been serialized at the sending end so we cannot switch between the serializations dynamically either.

In our next major however, we will introduce some changes that will allow you to control these serialization aspects through the configuration, so that both your use case can be supported as well as existing deployments can continue to run as they are

yvesgoeleven commented 9 years ago

Duplicate of https://github.com/Particular/NServiceBus.AzureServiceBus/issues/32

kevinhillinger commented 9 years ago

Understood. Given interop requirements with Android, what's a suggested solution?

kevinhillinger commented 9 years ago

So this isn't an academic exercise. I've got a new Particular customer that I'm installing NSB. What's the workaround for the new customer with serialization requirements across platforms?

yvesgoeleven commented 9 years ago

There are multiple options

kevinhillinger commented 9 years ago

What's the roadmap for the version with the correction?

In my situation, the second option isn't ideal, and I was hoping not to have to use a fork...but the ASB transport seems pretty stable, and if we're not talking a year+ to have to wait, I'll have to choose that option.

kevinhillinger commented 9 years ago

What about option 3: I work on an extension point for the input/output type of the body and it gets added to azure service bus configuration officially?

Wouldn't it only benefit Particular to support interoperability now?

yvesgoeleven commented 9 years ago

I'm actively working on the next major, but cannot share dates at the moment

One thing that could be done in the mean time, if you cannot wait until we release the next major, is to extract the 3 offending lines as Func's in a public static class, so that you can change the implemenations at startup time... then release that as a minor, since it only adds to the public api and doesn't change/remove we can do that. But it will be removed again in next major where it is integrated properly

kevinhillinger commented 9 years ago

Thanks, Yves. That would work. Is that something that I need to submit as a pull request?

yvesgoeleven commented 9 years ago

@kevinhillinger You can find the new extension points in version 6.3.2 on nuget

BrokeredMessageBodyConversion.ExtractBody is the Func callback to go from brokered message to byte[] BrokeredMessageBodyConversion.InjectBody is the other direction

You can substitute them with your memorystream based implementation (anywhere before the bus is started is fine)

kevinhillinger commented 9 years ago

Thank you, Yves. The responsiveness to resolving this is much appreciated!

I've created an example here: NServiceBus.AzureServiceBus.Serialization.Interop.