aProcessingErrorMessages in handleIncomingSBD

[iansmirlis]

Hello and Merry Christmas

I have a question regarding the recent change in v2.6.0 of adding the aProcessingErrorMessages parameter because it got me a bit confused. Maybe I've missed something.

In my implementation I assume that when handleIncomingSBD() is called, everything is already checked, so I proceed to store the message in my database and then call the createPeppolReportingItemForReceivedMessage() to store the reporting item to the back-end.

Thus, at this point I expect no aProcessingErrorMessages at this point. Should I? Or this is added just to add more aProcessingErrorMessages in the handleIncomingSBD?

Thanks!

[phax]

Hello and Merry Christmas to you too :)

Prior to v2.6.0 no real error handling was possible inside the custom SPI handler. Only throwing an Exception was a possibility to reject messages. However, based on the issue #198 having fine grained control over the error messages becomes more and more important. The generic AS4 SPI receiving handler already has the possibility to create custom error messages, but the Peppol specific SPI handler did not have the possibility. By adding this possibility it was just an alignment with the "generic" level.

A real-world use case is the message validation, and the message sender does not support MLR, then you need to reject synchronosouly. And now you can customize the error messages etc. properly.

Hope that makes sense :)

[iansmirlis]

Ok, yeah that makes sense.

So my assumption is correct, the moment were the function handleIncomingSBD() is called, the parameter aProcessingErrorMessages should be empty. If there are errors in the message that have been handled before, handleIncomingSBD() will be not called.

On the other hand I am free to add error messages in aProcessingErrorMessages. By adding at least one, the sender will get a rejection with my error message(s). Is that correct? Or I should also throw an exception?

[phax]

Yes, your assumptions are correct:)

[iansmirlis]

Ok. Don't want to pretend the smart-ass here, but if I were you I would add a constructor to an existing exception that causes the message to be rejected, with the messages collection. I.e.

if(!supportMLR) throw new InvalidMessageException(aProcessingErrorMessage); or

if(!supportMLR) aProcessingErrorMessages.add(...) if(!dontLikeThisMessage) aProcessingErrorMessages.add(...) ... if(aProcessingErrorMessages.Any()) throw new InvalidMessageException(aProcessingErrorMessages);

and the internal caller should catch this and return them to the sender.

But that's me.

[phax]

I agree with your proposal, under the assumption that there is just one processor for the incoming messages. Using the SPI pattern, there are potentially multiple different receivers so I decided to go for the error list to be filled instead. Otherwise I would needed to do some additional exception checking in the SPI caller, collecting the errors in one list.

In general, coming from the C world, I like the "return value" style of programming because I deem unhandled exceptions as the bigger problem. Without exceptions, the programming flow is also easier to verify and imho more consistent. That's why it is how it is :)

Thanks a lot for taking the time and presenting your ideas here - it is really appreciated!

[iansmirlis]

In general, coming from the C world, I like the "return value" style of programming because I deem unhandled exceptions as the bigger problem. Without exceptions, the programming flow is also easier to verify and imho more consistent. That's why it is how it is :)

haha that explains a lot :)

I am coming from the C++ world, where the error handling flow should not mess up with the rest logic flow. But yeah I see exactly your point.