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

Add TimeoutException to JmsInvokerClientInterceptor [SPR-12731] #17328

Closed
spring-issuemaster opened this issue Feb 18, 2015 · 1 comment

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Feb 18, 2015

Ezequiel Rosas Garcia opened SPR-12731 and commented

JmsInvokerClientInterceptor currently allows a timeout field, but it is currently not checking the return value of MessageConsumer#receive(long timeout) for null, which occurs when a timeout happens.

That leaves a NullPointerException coming from MessageConverter as the only signal for available for when a timeout happens .

MessageConsumer documentation mentions this null return value for receive(long)

Right now, It is possible to work around this issue by checking for null in a custom MessageConverter, but current MessageConverter implementations are naturally not expecting a null Message. (Except MessagingMessageConverter, though)

I would like to propose a change similar to the following to JmsInvokerClientInterceptor in order to signal a timeout situation to client code:

/*line 246*/ Message responseMessage = doExecuteRequest(session, queueToUse, requestMessage);
/* added  */ if (responseMessage == null) throw new TimeoutException()
/*line 247*/ return extractInvocationResult(responseMessage);

Affects: 4.1.4

Referenced from: commits 8fcbdae

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 19, 2015

Stéphane Nicoll commented

Good catch, thanks!

I have introduced an additional hook point onReceiveTimeout that takes the RemoteInvocation that did timeout. The default implementation throws a RemoteTimeoutException but sub-classes can override it to return a different exception.

It is also possible to return a fallback RemoteInvocationResult if that makes sense for a particular use case.

This should be available in the next couple of hours in a 4.2.0.BUILD-SNAPSHOT. Please give it a try if you can!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.