Add more producer error handling options

[Nephery]

Today the binder only sends publisher errors to error channels. We should consider adding other error handling e.g. maybe adding error queue support?

https://github.com/SolaceProducts/solace-spring-cloud/issues/34#issuecomment-721854525 Also I think we need specific error handling depending on the JCSMPErrorResponseException. For example if the message is too large then we won't be able to republish it to the error queue anyway, but if it was invalid topic syntax sending to an error queue might make more sense. There are other issues such as the MAX_MESSAGE_USAGE_EXCEEDED and SPOOL_OVER_QUOTA that a retry could make sense as space may have been freed up by consumers.

---

Note: October 21, 2021

Ignore all comments from https://github.com/SolaceProducts/solace-spring-cloud/issues/35#issuecomment-747422919 to https://github.com/SolaceProducts/solace-spring-cloud/issues/35#issuecomment-767538156. These comments are irrelevant to this issue.

[GreenRover]

I currently ran into same situation. The solution of the errorChannel is not very elegant.

But i have seen that this is more a spring cloud stream bug caused by:

com.solace.spring.cloud.stream.binder.outbound.JCSMPOutboundMessageHandler.handleMessage called by: org.springframework.cloud.stream.binder.AbstractMessageChannelBinder.handleMessageInternal is completely wrong designed.

Or more specific, the whole async sending is not a god plan! org.springframework.integration.channel.AbstractSubscribableChannel.doSend

This caused me to do the sending by my own: Using:

@Autowired
private final BinderFactory binderFactory;

SolaceMessageChannelBinder session = binderFactory.getBinder("main_session", PublishSubscribeChannel.class);

But here i stuck again, because it is not possible to access the jcsmpSession. And the JCSMPOutboundMessageHandler supports only async sending.

@Nephery do you have any plans to add synchronous sending? Or even better do you already any architecture designs?

[Mrc0113]

@GreenRover - completely agree that the current solution is not very elegant. That said, I don't think we want to move to a synchronous send as it would kill performance (and is also why JCSMP doesn't support it). Since we're using Persistent messaging you'd essentially be waiting for the message to be sent, reach the broker and receive and ACK before being able to send the next message on that same producer flow. And that round-trip network time can be brutal!

It looks like the Spring team is adding a new preferred way of handling publisher confirms (using a correlation data header) with the Rabbit Binder in the 3.1 release. They currently have a release candidate out for it so we'll probably want to explore that and maybe talk with the Spring team and brainstorm what options we should add here.

FYI @Nephery.

[GreenRover]

@Mrc0113 Hi Marc, nice to hear. I am highly interested in news according to this issue. Please keep us up to date! At least to put through the exceptions from jcsmp woulp help mutch.

[GreenRover]

@Mrc0113 I will start to investigate into this now. Will use https://github.com/SolaceDev/solace-spring-cloud/tree/client-ack as base branch. Any idea when client-ack make its way into 2.0.0 ?

[Nephery]

@GreenRover The client-ack branch is already merged with PR #46. Please use stage-2.0.0 for your base branch.

[GreenRover]

As we will not find any good solution in: https://github.com/spring-cloud/spring-cloud-stream/issues/2091 we may end up with:

Possible solution Z:

Add a binder option that enables:

Use a future in: com.solace.spring.cloud.stream.binder.outbound.JCSMPOutboundMessageHandler#handleMessage

to block and wait for the async JCSMPStreamingPublishEventHandler.responseReceived

Or any better ideas?

[GreenRover]

@Mrc0113 I implemented the "publisher confirms" from rabbit binder to solace https://github.com/SolaceProducts/solace-spring-cloud/pull/61 and a options to be totally blocking.

[PhilippeKhalife]

@Nephery to check the relevance of this issue given the most recent features.

[Nephery]

@PhilippeKhalife This issue is still relevant, though the conversation tangented to implementing publisher confirmations. I've updated the issue description to clarify what this issue is for.