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

AbstractMethodMessageHandler does not rethrow Errors [SPR-16912] #21451

Closed
spring-projects-issues opened this issue Jun 6, 2018 · 2 comments
Closed
Assignees
Labels
in: messaging Issues in messaging modules (jms, messaging) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jun 6, 2018

Bernhard Streit opened SPR-16912 and commented

We're using Spring Cloud AWS to receive messages from SQS.

Now, due to a bug, one of our services threw an OutOfMemoryError.

Having a redrive policy to a dead-letter queue set on the SQS queue, I assumed that with deletion policy NO_REDRIVE, the messages would get returned to the queue.

But that is not happening, as the AbstractMethodMessageHandler, that re-throws exceptions in case they occur, does not do that with errors (to be more precise, with all Throwables that are not of type Exception):

try {
	[...]

}
catch (Exception ex) {
	processHandlerMethodException(handlerMethod, ex, message);
}
catch (Throwable ex) {
	if (logger.isErrorEnabled()) {
		logger.error("Error while processing message " + message, ex);
	}
}

That has the effect that the SimpleMessageListenerContainer of Spring Cloud AWS Messaging assumes the execution finished without any error, and hence deletes the message from the queue:

 try {
        executeMessage(queueMessage);
        applyDeletionPolicyOnSuccess(receiptHandle);
} catch (MessagingException messagingException) {
        applyDeletionPolicyOnError(receiptHandle, messagingException);
}

executeMessage() returns silently, as the throwable was swallowed, and hence, applyDeletionPolicyOnSuccess() is executed next.

In case of an Exception, it would execute applyDeletionPolicyOnError, but not in case of an error.

Question: Is the Spring Framework violating the contract, by swallowing the Error? IMHO it should be re-thrown, just like ordinary Exceptions are.

Or is the SimpleMessageListenerContainer wrong to assume that if executeMessage has returned normally, the processing went fine without any problem? If yes, how can it figure out that an error has occurred?
 


Affects: 4.3.17, 5.0.6

Issue Links:

Referenced from: commits 1d6f717, f39adcf, 0c8cfa0, 646d7f9

Backported to: 4.3.18

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

We're rethrowing Errors and other non-Exceptions as MessageHandlingException now, sending it through the common exception resolution algorithm. This is on master now; I'll backport it to 5.0.x and 4.3.x tomorrow after a few additional tests (in particular for cause resolution).

@spring-projects-issues
Copy link
Collaborator Author

Bernhard Streit commented

Perfect, thanks for the quick reaction :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: messaging Issues in messaging modules (jms, messaging) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants