Skip to content
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

Spring integration service-interface gateway bridge should only forward message initiated by the gateway [INT-3660] #7619

Closed
spring-operator opened this issue Feb 26, 2015 · 5 comments
Labels
in: core status: duplicate There is already an issue similar to this. The link to it should be present type: enhancement

Comments

@spring-operator
Copy link
Contributor

Aodhagán Collins opened INT-3660 and commented

I am using Spring integration to launch Spring batch jobs. I have a number of input routes e.g. file polling and http requests. These all route to a batch-int Job Launching Gateway. The referenced Job Launcher has a task executor so job launches are asynchronous. This gateway replies on a specified channel.

This specified channel 'jobLaunchReplyChannel' is pub/sub and has a logger listening to it. This channel is also used as the reply channel for a service-interface gateway.

The issue I am having is that when jobs are requested via sources that are not the gateway (e.g. the poller) the Bridge that is added by gateway throws an exception because no reply channel is set on replies.


Affects: 3.0.4

Reference URL: http://stackoverflow.com/questions/28743577/spring-integration-service-interface-gateway-reply-channel-as-shared-pub-sub

@spring-operator
Copy link
Contributor Author

Aodhagán Collins commented

As per the stack overflow post http://stackoverflow.com/questions/28743577/spring-integration-service-interface-gateway-reply-channel-as-shared-pub-sub the solution I used was to get the gateway to add a header and then filter the replies. It would be great if the gateway did this itself.

@spring-operator
Copy link
Contributor Author

Gary Russell commented

One solution might be to have each gateway add a originator header and only process replies that originated here.

However, losing headers (especially the replyChannel ) is a common mistake and I am not keen on adding another header that would change behavior if it was lost someplace in the flow. Plus it's another header that would go in every message that's only needed for this relatively rare use case.

We'll have to think about it some more; perhaps another solution would be some way to configure endpoints that don't expect a reply to insert nullChannel into the replyChannel header.

Another workaround would be to add a <router/> in the reply channel flow and route messages with no replyChannel header to nullChannel. Or simply <filter/> on the lack of a replyChannel - I'm not sure why you needed to add a header to filter on.

@spring-operator
Copy link
Contributor Author

Aodhagán Collins commented

The thinking for adding the header to filter on was to future proof as I may add other gateways in the future so it would not be correct in that case to forward any message with a reply-channel. In fact that could cause undesirable behaviour in that the message could be forwarded to the reply-channel by all the BridgeHandlers added by the individual Gateways.

@spring-operator
Copy link
Contributor Author

Artem Bilan commented

Adding filter (unconditionally) to the replyMessageCorrelator flow does not make sense. We would filter all the messages without replyChannel header, including those who are really replies but with lost replyChannel header by some wrong logic. Although that might be an option on the gateway - filter-replies.

The idea with the nullChannel as a replyChannel header for all the messages from the Inbound Channel Adapters is good, but it provides some overhead with an extra header carrying (which can be lost downstream as well).
Although this might be an option as well there. However I've provided simple one already on that SO question:

<header-enricher>
  <reply-channel ref="nullChannel"/>
</header-enricher>

This way can be just documented for this complex (unusual) scenario.

The scenario with publish-subscribe-channel as a reply-channel configuration for several gateways is unusual and indeed brings a bunch of unexpected behaviors. You've just made a trap for yourself there! In most cases we really don't need that reply-channel configuration on the gateway at all. The simple relying on the replyChannel header is fully enough. But anyway, if you even made that configuration, the reply message comes to the proper originator gateway, just because all the logic is based on the TemporaryReplyChannel. And even if you have several gateways subscribed to the same publish-subscribe-channel for replies, only one of those message copies reaches its "native" gateway. All others indeed will be sent to the same TemporaryReplyChannel, but there we have a logic:

else if (alreadyReceivedReply) {
	errorDescription = "Reply message received but the receiving thread has already received a reply";
}
...
if (errorDescription != null) {
	if (logger.isWarnEnabled()) {
		logger.warn(errorDescription + ":" + message);
	}
	if (this.throwExceptionOnLateReply) {
		throw new MessageDeliveryException(message, errorDescription);
	}
}

So, you get a WARN in logs and MessageDeliveryException thrown but only in case of this.throwExceptionOnLateReply == true.

I would suggest to stay away from such a scenario to avoid unexpected WARNs and exceptions.

Nevertheless I see another solution like supply the TemporaryReplyChannel with the Object originator property and improve our replyMessageCorrelator logic to check that property before sending to that TemporaryReplyChannel. Only the problem that we might not able to provide changes to the TemporaryReplyChannel any more - Spring Framework 5.0 is very close to its GA point...
Although we may consider to come up with our own TemporaryReplyChannel implementation copying all the sendAndReceive logic like we have already with the MessagingGatewaySupport.doSendAndReceiveMessageReactive().

@artembilan artembilan modified the milestones: General Backlog, Backlog Feb 14, 2023
@artembilan
Copy link
Member

I find this as a duplication of #3985.
Same problem - different report.

@artembilan artembilan closed this as not planned Won't fix, can't repro, duplicate, stale May 12, 2023
@artembilan artembilan removed this from the Backlog milestone May 12, 2023
@artembilan artembilan added the status: duplicate There is already an issue similar to this. The link to it should be present label May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: duplicate There is already an issue similar to this. The link to it should be present type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants