From 86292584f967e91cc7d4e4346a3bd67ac5559b74 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Tue, 22 Apr 2025 15:43:54 -0400 Subject: [PATCH] GH-9985: Fix NPE in ReqHandlerRetryAdvice with MetricsRetryListener Fixes: #9985 Issue link: https://github.com/spring-projects/spring-integration/issues/9985 When `RequestHandlerRetryAdvice` is supplied with a `RetryTemplate` and `MetricsRetryListener`, the `NullPointerException` is thrown from the Micrometer `Timer`, where `name` tag cannot be `null`. In fact, the `RetryCallback` implementation in the `RequestHandlerRetryAdvice` does not provide any `label`. * Fix `RequestHandlerRetryAdvice` to produce label based on the `componentName` or just class name of the AOP caller as fallback (cherry picked from commit 30355f243c799f50733f0c245da4424f19e02bc1) --- .../advice/RequestHandlerRetryAdvice.java | 14 ++++-- .../advice/AdvisedMessageHandlerTests.java | 46 ++++++++++++++++++- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/advice/RequestHandlerRetryAdvice.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/advice/RequestHandlerRetryAdvice.java index cdf50b3baa..f75d252367 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/handler/advice/RequestHandlerRetryAdvice.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/advice/RequestHandlerRetryAdvice.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.integration.handler.advice; +import org.springframework.integration.handler.AbstractReplyProducingMessageHandler; import org.springframework.integration.support.ErrorMessageUtils; import org.springframework.messaging.Message; import org.springframework.messaging.MessagingException; @@ -78,7 +79,7 @@ public class RequestHandlerRetryAdvice extends AbstractRequestHandlerAdvice { @Override protected Object doInvoke(ExecutionCallback callback, Object target, Message message) { - IntegrationRetryCallback retryCallback = new IntegrationRetryCallback(message, callback); + IntegrationRetryCallback retryCallback = new IntegrationRetryCallback(message, callback, target); RetryState retryState = this.retryStateGenerator.determineRetryState(message); try { return this.retryTemplate.execute(retryCallback, this.recoveryCallback, retryState); @@ -113,7 +114,7 @@ public class RequestHandlerRetryAdvice extends AbstractRequestHandlerAdvice { } - private record IntegrationRetryCallback(Message messageToTry, ExecutionCallback callback) + private record IntegrationRetryCallback(Message messageToTry, ExecutionCallback callback, Object target) implements RetryCallback { @Override @@ -121,6 +122,13 @@ public class RequestHandlerRetryAdvice extends AbstractRequestHandlerAdvice { return this.callback.cloneAndExecute(); } + @Override + public String getLabel() { + return this.target instanceof AbstractReplyProducingMessageHandler.RequestHandler requestHandler + ? requestHandler.getAdvisedHandler().getComponentName() + : this.target.getClass().getName(); + } + } } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/handler/advice/AdvisedMessageHandlerTests.java b/spring-integration-core/src/test/java/org/springframework/integration/handler/advice/AdvisedMessageHandlerTests.java index a585ea036d..be570a7b7f 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/handler/advice/AdvisedMessageHandlerTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/handler/advice/AdvisedMessageHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * Copyright 2002-2025 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,9 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.aopalliance.aop.Advice; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; @@ -58,6 +61,7 @@ import org.springframework.messaging.support.GenericMessage; import org.springframework.retry.RetryContext; import org.springframework.retry.policy.SimpleRetryPolicy; import org.springframework.retry.support.DefaultRetryState; +import org.springframework.retry.support.MetricsRetryListener; import org.springframework.retry.support.RetryTemplate; import org.springframework.scheduling.TaskScheduler; import org.springframework.test.annotation.DirtiesContext; @@ -993,6 +997,46 @@ public class AdvisedMessageHandlerTests { assertThat(((ErrorMessage) error).getOriginalMessage().getPayload()).isEqualTo("foo"); } + @Test + public void retryAdviceWithMetricsListener() { + AbstractReplyProducingMessageHandler handler = new AbstractReplyProducingMessageHandler() { + + @Override + protected Object handleRequestMessage(Message requestMessage) { + throw new RuntimeException("intentional"); + } + }; + + MeterRegistry meterRegistry = new SimpleMeterRegistry(); + + RequestHandlerRetryAdvice advice = new RequestHandlerRetryAdvice(); + RetryTemplate retryTemplate = new RetryTemplate(); + retryTemplate.registerListener(new MetricsRetryListener(meterRegistry)); + advice.setRetryTemplate(retryTemplate); + advice.setBeanFactory(mock(BeanFactory.class)); + advice.afterPropertiesSet(); + + List adviceChain = new ArrayList<>(); + adviceChain.add(advice); + handler.setAdviceChain(adviceChain); + handler.setBeanName("testEndpoint"); + handler.setBeanFactory(mock(BeanFactory.class)); + handler.afterPropertiesSet(); + + Message message = new GenericMessage<>("Hello, world!"); + assertThatExceptionOfType(MessagingException.class) + .isThrownBy(() -> handler.handleMessage(message)) + .withRootCauseInstanceOf(RuntimeException.class) + .withStackTraceContaining("intentional"); + + Timer retryTimer = meterRegistry.find(MetricsRetryListener.TIMER_NAME) + .tag("name", "testEndpoint") + .tag("retry.count", "3") + .timer(); + + assertThat(retryTimer.count()).isEqualTo(1); + } + private interface Bar { Object handleRequestMessage(Message message) throws Throwable;