Provide more flexible message parsing in message pump

[Schreder-Hyperion]

Is your feature request related to a problem? Please describe.
When adding new properties to the messaging model we do not always remember to update message consumers' projects

Because of that, the messages are no longer being processed by the defined message handler because it shows the following verbose level log:

Message handler 'X' is not able to process the message because the incoming message cannot be deserialized to the message that the message handler can handle

As a result, the message is moved on and handled by the fallback message handler.

Describe the solution you'd like
To avoid causing disrupting behavior it would be nice to have a flag that toggles LAX parsing, for example in the extension method: AddServiceBusQueueMessagePump adding a parameter i.e.: laxParsingEnabled = false so that existing implementations are not directly impacted but that we could easily switch on.

It would also be nice to see the above message being presented as an Error in the logs, making it easier to intercept it without having to toggle Logging levels.

Additional context
Our message contract:

{
  "Imei": "10.34.0.16:4096",
  "FirmwareVersion": "efefad25-0d9f-4745-88d9-99b040b80c54",
  "DeviceId": "352554100004969",
  "Ip": "3.32.16.5",
  "MessageReceivedDateTime": "2021-06-15T14:02:35.190Z"
}

Received message payload:

{
  "Imei": "10.34.0.16:4096",
  "FirmwareVersion": "efefad25-0d9f-4745-88d9-99b040b80c54",
  "DeviceId": "352554100004969",
  "Ip": "3.32.16.5",
+ "Status": null,
  "MessageReceivedDateTime": "2021-06-15T14:02:35.190Z"
}

[tomkerkhove]

@stijnmoreels Let's see if we can provide some more flexible parsing here and if we can introduce options for the pump to opt-in for this.

I agree that this should have been an error to raise awareness, or even an exception and stop the whole processing instead of triggering the fallback processing.

Only caveat would be if we allow this so that 2 message handlers can have the same filter criteria and the message handler that has the exact message type wins. If that is the case, I'd provide some configuration here.

[stijnmoreels]

First of all, thanks @Schreder-Hyperion for your issue!

It would also be nice to see the above message being presented as an Error in the logs, making it easier to intercept it without having to toggle Logging levels.

I agree that this should have been an error to raise awareness, or even an exception and stop the whole processing instead of triggering the fallback processing.

The reason this message is written as a trace/verbose message and not an error/exception, is because in many cases the message handler is expected not to match with the incoming message. Especially when you have many message handlers, all with different message types, message context predicates... then all the preceding message handlers will write this message, saying the message is not for them.
You could argue that we should write this as an error when the last message handler in the line can't handle the message. But even then, people may seem confused as the problem could lie with a incorrect preceding message handler registration.

To avoid causing disrupting behavior it would be nice to have a flag that toggles LAX parsing, for example in the extension method: AddServiceBusQueueMessagePump adding a parameter i.e.: laxParsingEnabled = false so that existing implementations are not directly impacted but that we could easily switch on.

Let's see if we can provide some more flexible parsing here and if we can introduce options for the pump to opt-in for this.

Let me first of all say that we introduced an IMessageBodySerializer model that you use to implement your own parsing. We have included this strict parsing as a fallback for incoming messages, but I understand that in more complex scenario's a different approach needs to be in order.
So, as I understand it, this 'lax parsing' you require is already present via implementing your own. That way, you can make sure that this missing property is not the reason the message handler isn't matched with the message.
https://messaging.arcus-azure.net/features/message-pumps/customization#bring-your-own-deserialization

[tomkerkhove]

I agree that custom serialization is a great way to extend, but in this case I think lax serialization and ignore the non-defined fields should be something we build into the message pump.

The reason this message is written as a trace/verbose message and not an error/exception, is because in many cases the message handler is expected not to match with the incoming message. Especially when you have many message handlers, all with different message types, message context predicates... then all the preceding message handlers will write this message, saying the message is not for them.

You could argue that we should write this as an error when the last message handler in the line can't handle the message. But even then, people may seem confused as the problem could lie with a incorrect preceding message handler registration.

The real question is, will people ever have the same matching predicate with different handlers and messages? If so, fine. But there are also cases where it should be the case.

So what I think we should do is:

1. Provide capability to opt-in for lax message parsing out-of-the-box
2. Provide capability to opt-in for breaking when the predicate matches but can't deserialize it correctly
   • One might argue this should be the default, but this is not backwards compatible

[stijnmoreels]

@Schreder-Hyperion , with the v1.0 release, your suggestion is part of the publicly available package! Thanks again.