Skip to content

Commit d00be71

Browse files
garyrussellartembilan
authored andcommitted
GH-2437: Fix Fatal When No Matching RabbitHandler
Resolves #2437 Failure to find a `@RabbitHandler` method for the payload must be treated as a fatal exception to avoid an infinite loop. Also, fix CREH cause traversal to stop when **any** fatal exception is found. Previously, traversal only stopped for the original subset of exceptions. **cherry-pick to 2.4.x** # Conflicts: # spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/ConditionalRejectingErrorHandler.java # spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/adapter/DelegatingInvocableHandler.java
1 parent 6ed749e commit d00be71

File tree

3 files changed

+89
-10
lines changed

3 files changed

+89
-10
lines changed

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/ConditionalRejectingErrorHandler.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2020 the original author or authors.
2+
* Copyright 2014-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.amqp.rabbit.listener;
1818

19+
import java.lang.reflect.UndeclaredThrowableException;
1920
import java.util.List;
2021
import java.util.Map;
2122

@@ -200,9 +201,9 @@ public static class DefaultExceptionStrategy implements FatalExceptionStrategy {
200201
@Override
201202
public boolean isFatal(Throwable t) {
202203
Throwable cause = t.getCause();
203-
while (cause instanceof MessagingException
204-
&& !(cause instanceof org.springframework.messaging.converter.MessageConversionException)
205-
&& !(cause instanceof MethodArgumentResolutionException)) {
204+
while ((cause instanceof MessagingException || cause instanceof UndeclaredThrowableException)
205+
&& !isCauseFatal(cause)) {
206+
206207
cause = cause.getCause();
207208
}
208209
if (t instanceof ListenerExecutionFailedException && isCauseFatal(cause)) {

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/adapter/DelegatingInvocableHandler.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015-2022 the original author or authors.
2+
* Copyright 2015-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -46,6 +46,7 @@
4646
import org.springframework.messaging.handler.annotation.support.PayloadMethodArgumentResolver;
4747
import org.springframework.messaging.handler.invocation.InvocableHandlerMethod;
4848
import org.springframework.util.Assert;
49+
import org.springframework.util.ReflectionUtils;
4950
import org.springframework.util.concurrent.ListenableFuture;
5051
import org.springframework.validation.Validator;
5152

@@ -206,7 +207,9 @@ protected InvocableHandlerMethod getHandlerForPayload(Class<? extends Object> pa
206207
if (handler == null) {
207208
handler = findHandlerForPayload(payloadClass);
208209
if (handler == null) {
209-
throw new AmqpException("No method found for " + payloadClass);
210+
ReflectionUtils.rethrowRuntimeException(
211+
new NoSuchMethodException("No listener method found in " + this.bean.getClass().getName()
212+
+ " for " + payloadClass));
210213
}
211214
this.cachedHandlers.putIfAbsent(payloadClass, handler); //NOSONAR
212215
setupReplyTo(handler);
@@ -284,7 +287,7 @@ protected boolean matchHandlerMethod(Class<? extends Object> payloadClass, Invoc
284287
MethodParameter methodParameter = new MethodParameter(method, 0);
285288
if ((methodParameter.getParameterAnnotations().length == 0
286289
|| !methodParameter.hasParameterAnnotation(Header.class))
287-
&& methodParameter.getParameterType().isAssignableFrom(payloadClass)) {
290+
&& methodParameter.getParameterType().isAssignableFrom(payloadClass)) {
288291
if (this.validator != null) {
289292
this.payloadMethodParameters.put(handler, methodParameter);
290293
}
@@ -302,7 +305,7 @@ private boolean findACandidate(Class<? extends Object> payloadClass, InvocableHa
302305
MethodParameter methodParameter = new MethodParameter(method, i);
303306
if ((methodParameter.getParameterAnnotations().length == 0
304307
|| !methodParameter.hasParameterAnnotation(Header.class))
305-
&& methodParameter.getParameterType().isAssignableFrom(payloadClass)) {
308+
&& methodParameter.getParameterType().isAssignableFrom(payloadClass)) {
306309
if (foundCandidate) {
307310
throw new AmqpException("Ambiguous payload parameter for " + method.toGenericString());
308311
}
@@ -362,8 +365,7 @@ private static final class PayloadValidator extends PayloadMethodArgumentResolve
362365

363366
@Override
364367
@Nullable
365-
public Message<?> toMessage(Object payload, @Nullable
366-
MessageHeaders headers) {
368+
public Message<?> toMessage(Object payload, @Nullable MessageHeaders headers) {
367369
return null;
368370
}
369371

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright 2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.amqp.rabbit.listener.adapter;
18+
19+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
20+
import static org.mockito.Mockito.mock;
21+
22+
import java.lang.reflect.Method;
23+
import java.lang.reflect.UndeclaredThrowableException;
24+
import java.util.ArrayList;
25+
import java.util.List;
26+
27+
import org.junit.jupiter.api.Test;
28+
29+
import org.springframework.beans.factory.config.BeanExpressionContext;
30+
import org.springframework.beans.factory.config.BeanExpressionResolver;
31+
import org.springframework.format.support.DefaultFormattingConversionService;
32+
import org.springframework.messaging.converter.GenericMessageConverter;
33+
import org.springframework.messaging.handler.annotation.support.DefaultMessageHandlerMethodFactory;
34+
import org.springframework.messaging.handler.annotation.support.MessageHandlerMethodFactory;
35+
import org.springframework.messaging.handler.invocation.InvocableHandlerMethod;
36+
37+
/**
38+
* @author Gary Russell
39+
* @since 2.4.12
40+
*
41+
*/
42+
public class DelegatingInvocableHandlerTests {
43+
44+
@Test
45+
void multiNoMatch() throws Exception {
46+
List<InvocableHandlerMethod> methods = new ArrayList<>();
47+
Object bean = new Multi();
48+
Method method = Multi.class.getDeclaredMethod("listen", Integer.class);
49+
methods.add(messageHandlerFactory().createInvocableHandlerMethod(bean, method));
50+
BeanExpressionResolver resolver = mock(BeanExpressionResolver.class);
51+
BeanExpressionContext context = mock(BeanExpressionContext.class);
52+
DelegatingInvocableHandler handler = new DelegatingInvocableHandler(methods, bean, resolver, context);
53+
assertThatExceptionOfType(UndeclaredThrowableException.class).isThrownBy(() ->
54+
handler.getHandlerForPayload(Long.class))
55+
.withCauseExactlyInstanceOf(NoSuchMethodException.class)
56+
.withStackTraceContaining("No listener method found in");
57+
}
58+
59+
private MessageHandlerMethodFactory messageHandlerFactory() {
60+
DefaultMessageHandlerMethodFactory defaultFactory = new DefaultMessageHandlerMethodFactory();
61+
DefaultFormattingConversionService cs = new DefaultFormattingConversionService();
62+
defaultFactory.setConversionService(cs);
63+
GenericMessageConverter messageConverter = new GenericMessageConverter(cs);
64+
defaultFactory.setMessageConverter(messageConverter);
65+
defaultFactory.afterPropertiesSet();
66+
return defaultFactory;
67+
}
68+
69+
public static class Multi {
70+
71+
void listen(Integer in) {
72+
}
73+
74+
}
75+
76+
}

0 commit comments

Comments
 (0)