Missing @RabbitHandler method triggers an infinite retry loop

[bcalmac]

In what version(s) of Spring AMQP are you seeing this issue?

2.4.6

Describe the bug

If the spring-boot application does not have a @RabbitHandler matching an incoming event then the event is re-queued and retried immediately, triggering an infinite reprocessing loop.

To Reproduce

Create a binding that sends a service an unexpected message. Say for instance, if you currently have:

@Component
@RabbitListener(queues = "foo-service")
public class FooEventListener {

    @RabbitHandler
    public void onOrderCreated(OrderCreated orderCreated) {
        ...
    }
}

then create a binding that sends some other event, say OrderUpdated to the foo-service queue.

Here is the exception as seen from our application:

org.springframework.amqp.rabbit.support.ListenerExecutionFailedException: Failed to convert message
        at org.springframework.amqp.rabbit.listener.adapter.MessagingMessageListenerAdapter.onMessage(MessagingMessageListenerAdapter.java:156)
        at org.springframework.amqp.rabbit.listener.AbstractMessageListenerContainer.doInvokeListener(AbstractMessageListenerContainer.java:1670)
        at org.springframework.amqp.rabbit.listener.AbstractMessageListenerContainer.actualInvokeListener(AbstractMessageListenerContainer.java:1589)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:344)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:198)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
        at io.opentracing.contrib.spring.rabbitmq.RabbitMqReceiveTracingInterceptor.invoke(RabbitMqReceiveTracingInterceptor.java:48)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
        at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215)
        at org.springframework.amqp.rabbit.listener.$Proxy173.invokeListener(Unknown Source)
        at org.springframework.amqp.rabbit.listener.AbstractMessageListenerContainer.invokeListener(AbstractMessageListenerContainer.java:1577)
        at org.springframework.amqp.rabbit.listener.AbstractMessageListenerContainer.doExecuteListener(AbstractMessageListenerContainer.java:1568)
        at org.springframework.amqp.rabbit.listener.AbstractMessageListenerContainer.executeListener(AbstractMessageListenerContainer.java:1512)
        at org.springframework.amqp.rabbit.listener.SimpleMessageListenerContainer.doReceiveAndExecute(SimpleMessageListenerContainer.java:993)
        at org.springframework.amqp.rabbit.listener.SimpleMessageListenerContainer.receiveAndExecute(SimpleMessageListenerContainer.java:940)
        at org.springframework.amqp.rabbit.listener.SimpleMessageListenerContainer.access$1600(SimpleMessageListenerContainer.java:84)
        at org.springframework.amqp.rabbit.listener.SimpleMessageListenerContainer$AsyncMessageProcessingConsumer.mainLoop(SimpleMessageListenerContainer.java:1317)
        at org.springframework.amqp.rabbit.listener.SimpleMessageListenerContainer$AsyncMessageProcessingConsumer.run(SimpleMessageListenerContainer.java:1223)
        at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: org.springframework.amqp.AmqpException: No method found for class com.clearcapital.lca3.model.event.AppraisalSubmittedEvent
        at org.springframework.amqp.rabbit.listener.adapter.DelegatingInvocableHandler.getHandlerForPayload(DelegatingInvocableHandler.java:207)
        at org.springframework.amqp.rabbit.listener.adapter.DelegatingInvocableHandler.getMethodFor(DelegatingInvocableHandler.java:339)
        at org.springframework.amqp.rabbit.listener.adapter.HandlerAdapter.getMethodFor(HandlerAdapter.java:117)
        at org.springframework.amqp.rabbit.listener.adapter.MessagingMessageListenerAdapter.invokeHandlerAndProcessResult(MessagingMessageListenerAdapter.java:206)
        at org.springframework.amqp.rabbit.listener.adapter.MessagingMessageListenerAdapter.onMessage(MessagingMessageListenerAdapter.java:147)
        ... 22 common frames omitted

Expected behavior

This use case is no different than all the others described in https://docs.spring.io/spring-amqp/reference/html/#exception-handling for which the ConditionalRejectingErrorHandler rejects the message without re-queuing. I would expect this to be treated the same.

Of course, this issue can be addressed on the application side in several way. The main point is that the default behavior is undesirable and leads to a less resilient system out of the box.

[garyrussell]

Yes, the CREH should treat that exception is fatal; in the meantime, you can set the container's defaultRequeueRejected to false.

	/**
	 * Set the default behavior when a message is rejected, for example because the listener
	 * threw an exception. When true, messages will be requeued, when false, they will not. For
	 * versions of Rabbit that support dead-lettering, the message must not be requeued in order
	 * to be sent to the dead letter exchange. Setting to false causes all rejections to not
	 * be requeued. When true, the default can be overridden by the listener throwing an
	 * {@link AmqpRejectAndDontRequeueException}. Default true.
	 * @param defaultRequeueRejected true to reject by default.
	 */
	public void setDefaultRequeueRejected(boolean defaultRequeueRejected) {

[artembilan]

Interesting. The NoSuchMethodException is included into a DefaultExceptionStrategy, but looks like that DelegatingInvocableHandler does this instead throw new AmqpException("No method found for " + payloadClass);.
Or we should throw from there a NoSuchMethodException, or some other more relevant one and treat it as fatal in that DefaultExceptionStrategy...

[garyrussell]

Exactly; yes.

[garyrussell]

Another work around is to add a default @RabbitHandler method that receives Object and throw an AmqpRejectAndDontRequeueException from there.

[bcalmac]

Quick related question, sorry. If I go ahead for now with:

spring.rabbitmq.listener.simple.default-requeue-rejected=false

can you confirm that I can still request that a message be re-queued by throwing a ImmediateRequeueAmqpException?

[artembilan]

That's correct.
See ContainerUtils.shouldRequeue() logic on the matter:

boolean shouldRequeue = defaultRequeueRejected ||
				throwable instanceof MessageRejectedWhileStoppingException;
		Throwable t = throwable;
		while (t != null) {
			if (t instanceof AmqpRejectAndDontRequeueException) {
				shouldRequeue = false;
				break;
			}
			else if (t instanceof ImmediateRequeueAmqpException) {
				shouldRequeue = true;
				break;
			}
			t = t.getCause();
		}

So, if you set defaultRequeueRejected to false, that ImmediateRequeueAmqpException will override it smoothly.