From 4e8b524048a81cff09f2186f252bc1d1561024e4 Mon Sep 17 00:00:00 2001 From: "Shanyou Yu (Sean Yu)" Date: Mon, 29 May 2023 11:21:10 +0800 Subject: [PATCH] fix custom fallback exception (#1020) --- CHANGELOG.md | 1 + .../exception/FallbackWrapperException.java | 27 ++++ .../PolarisCircuitBreakerFallbackFactory.java | 5 +- ...sFeignCircuitBreakerInvocationHandler.java | 13 +- ...CircuitBreakerRestTemplateInterceptor.java | 70 ++++++---- ...risCircuitBreakerFeignIntegrationTest.java | 4 +- ...PolarisCircuitBreakerNameResolverTest.java | 125 ++++++++++++++++++ ...uitBreakerRestTemplateIntegrationTest.java | 7 +- 8 files changed, 214 insertions(+), 38 deletions(-) create mode 100644 spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/exception/FallbackWrapperException.java create mode 100644 spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerNameResolverTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 99b394d8f..78d6330ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,3 +10,4 @@ - [docs:add release GitHub Action.](https://github.com/Tencent/spring-cloud-tencent/pull/1006) - [docs:update Polaris test environment ip.](https://github.com/Tencent/spring-cloud-tencent/pull/1010) - [feat:sct-all package is now available as a shaded uber-jar.](https://github.com/Tencent/spring-cloud-tencent/pull/1016) +- [fix:fix custom fallback exception.](https://github.com/Tencent/spring-cloud-tencent/pull/1020) diff --git a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/exception/FallbackWrapperException.java b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/exception/FallbackWrapperException.java new file mode 100644 index 000000000..39b3e0b17 --- /dev/null +++ b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/exception/FallbackWrapperException.java @@ -0,0 +1,27 @@ +/* + * Tencent is pleased to support the open source community by making Spring Cloud Tencent available. + * + * Copyright (C) 2019 THL A29 Limited, a Tencent company. All rights reserved. + * + * Licensed under the BSD 3-Clause License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://opensource.org/licenses/BSD-3-Clause + * + * Unless required by applicable law or agreed to in writing, software distributed + * under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR + * CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package com.tencent.cloud.polaris.circuitbreaker.exception; + +public class FallbackWrapperException extends RuntimeException { + + public FallbackWrapperException(Throwable cause) { + super(cause); + } + +} + diff --git a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerFallbackFactory.java b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerFallbackFactory.java index 0bb6b9644..b513d1138 100644 --- a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerFallbackFactory.java +++ b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerFallbackFactory.java @@ -25,6 +25,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import com.tencent.cloud.polaris.circuitbreaker.exception.FallbackWrapperException; import com.tencent.polaris.api.pojo.CircuitBreakerStatus; import com.tencent.polaris.circuitbreak.client.exception.CallAbortedException; import feign.Request; @@ -87,11 +88,11 @@ public class PolarisCircuitBreakerFallbackFactory implements FallbackFactory { return decoder.decode(response, method.getGenericReturnType()); } catch (IOException e) { - throw new IllegalStateException(e); + throw new FallbackWrapperException(e); } } } - throw new IllegalStateException(t); + throw new FallbackWrapperException(t); } } } diff --git a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisFeignCircuitBreakerInvocationHandler.java b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisFeignCircuitBreakerInvocationHandler.java index ab16bf625..abcb47586 100644 --- a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisFeignCircuitBreakerInvocationHandler.java +++ b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisFeignCircuitBreakerInvocationHandler.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.function.Function; import java.util.function.Supplier; +import com.tencent.cloud.polaris.circuitbreaker.exception.FallbackWrapperException; import feign.InvocationHandlerFactory; import feign.Target; import feign.codec.Decoder; @@ -119,7 +120,13 @@ public class PolarisFeignCircuitBreakerInvocationHandler implements InvocationHa return fallback.fallback(method); }; } - return circuitBreaker.run(supplier, fallbackFunction); + try { + return circuitBreaker.run(supplier, fallbackFunction); + } + catch (FallbackWrapperException e) { + // unwrap And Rethrow + throw e.getCause(); + } } private void unwrapAndRethrow(Exception exception) { @@ -129,9 +136,9 @@ public class PolarisFeignCircuitBreakerInvocationHandler implements InvocationHa throw (RuntimeException) underlyingException; } if (underlyingException != null) { - throw new IllegalStateException(underlyingException); + throw new FallbackWrapperException(underlyingException); } - throw new IllegalStateException(exception); + throw new FallbackWrapperException(exception); } } diff --git a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/resttemplate/PolarisCircuitBreakerRestTemplateInterceptor.java b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/resttemplate/PolarisCircuitBreakerRestTemplateInterceptor.java index 49afb2210..8cb26902f 100644 --- a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/resttemplate/PolarisCircuitBreakerRestTemplateInterceptor.java +++ b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/main/java/com/tencent/cloud/polaris/circuitbreaker/resttemplate/PolarisCircuitBreakerRestTemplateInterceptor.java @@ -20,6 +20,7 @@ package com.tencent.cloud.polaris.circuitbreaker.resttemplate; import java.io.IOException; import java.lang.reflect.Method; +import com.tencent.cloud.polaris.circuitbreaker.exception.FallbackWrapperException; import com.tencent.polaris.api.pojo.CircuitBreakerStatus; import com.tencent.polaris.circuitbreak.client.exception.CallAbortedException; @@ -63,39 +64,50 @@ public class PolarisCircuitBreakerRestTemplateInterceptor implements ClientHttpR @Override public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException { - return circuitBreakerFactory.create(request.getURI().getHost() + "#" + request.getURI().getPath()).run( - () -> { - try { - ClientHttpResponse response = execution.execute(request, body); - ResponseErrorHandler errorHandler = restTemplate.getErrorHandler(); - if (errorHandler.hasError(response)) { - errorHandler.handleError(request.getURI(), request.getMethod(), response); + try { + return circuitBreakerFactory.create(request.getURI().getHost() + "#" + request.getURI().getPath()).run( + () -> { + try { + ClientHttpResponse response = execution.execute(request, body); + ResponseErrorHandler errorHandler = restTemplate.getErrorHandler(); + if (errorHandler.hasError(response)) { + errorHandler.handleError(request.getURI(), request.getMethod(), response); + } + return response; } - return response; - } - catch (IOException e) { - throw new IllegalStateException(e); - } - }, - t -> { - if (StringUtils.hasText(polarisCircuitBreaker.fallback())) { - CircuitBreakerStatus.FallbackInfo fallbackInfo = new CircuitBreakerStatus.FallbackInfo(200, null, polarisCircuitBreaker.fallback()); - return new PolarisCircuitBreakerHttpResponse(fallbackInfo); - } - if (!PolarisCircuitBreakerFallback.class.toGenericString().equals(polarisCircuitBreaker.fallbackClass().toGenericString())) { - Method method = ReflectionUtils.findMethod(PolarisCircuitBreakerFallback.class, "fallback"); - PolarisCircuitBreakerFallback polarisCircuitBreakerFallback = applicationContext.getBean(polarisCircuitBreaker.fallbackClass()); - return (PolarisCircuitBreakerHttpResponse) ReflectionUtils.invokeMethod(method, polarisCircuitBreakerFallback); - } - if (t instanceof CallAbortedException) { - CircuitBreakerStatus.FallbackInfo fallbackInfo = ((CallAbortedException) t).getFallbackInfo(); - if (fallbackInfo != null) { + catch (IOException e) { + throw new IllegalStateException(e); + } + }, + t -> { + if (StringUtils.hasText(polarisCircuitBreaker.fallback())) { + CircuitBreakerStatus.FallbackInfo fallbackInfo = new CircuitBreakerStatus.FallbackInfo(200, null, polarisCircuitBreaker.fallback()); return new PolarisCircuitBreakerHttpResponse(fallbackInfo); } + if (!PolarisCircuitBreakerFallback.class.toGenericString().equals(polarisCircuitBreaker.fallbackClass().toGenericString())) { + Method method = ReflectionUtils.findMethod(PolarisCircuitBreakerFallback.class, "fallback"); + PolarisCircuitBreakerFallback polarisCircuitBreakerFallback = applicationContext.getBean(polarisCircuitBreaker.fallbackClass()); + return (PolarisCircuitBreakerHttpResponse) ReflectionUtils.invokeMethod(method, polarisCircuitBreakerFallback); + } + if (t instanceof CallAbortedException) { + CircuitBreakerStatus.FallbackInfo fallbackInfo = ((CallAbortedException) t).getFallbackInfo(); + if (fallbackInfo != null) { + return new PolarisCircuitBreakerHttpResponse(fallbackInfo); + } + } + throw new FallbackWrapperException(t); } - throw new IllegalStateException(t); - } - ); + ); + } + catch (FallbackWrapperException e) { + // unwrap And Rethrow + Throwable underlyingException = e.getCause(); + if (underlyingException instanceof RuntimeException) { + throw (RuntimeException) underlyingException; + } + throw new IllegalStateException(underlyingException); + } + } } diff --git a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerFeignIntegrationTest.java b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerFeignIntegrationTest.java index cdde3388f..ea77505eb 100644 --- a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerFeignIntegrationTest.java +++ b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerFeignIntegrationTest.java @@ -111,7 +111,7 @@ public class PolarisCircuitBreakerFeignIntegrationTest { Utils.sleepUninterrupted(2000); assertThatThrownBy(() -> { echoService.echo(null); - }).isInstanceOf(Exception.class); + }).isInstanceOf(InvocationTargetException.class); assertThatThrownBy(() -> { fooService.echo("test"); }).isInstanceOf(NoFallbackAvailableException.class); @@ -244,7 +244,7 @@ public class PolarisCircuitBreakerFeignIntegrationTest { @Override public String echo(@RequestParam("str") String param) throws InvocationTargetException { if (param == null) { - throw new InvocationTargetException(new Exception()); + throw new InvocationTargetException(new Exception(), "test InvocationTargetException"); } return "echo fallback"; } diff --git a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerNameResolverTest.java b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerNameResolverTest.java new file mode 100644 index 000000000..52e9e4650 --- /dev/null +++ b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/feign/PolarisCircuitBreakerNameResolverTest.java @@ -0,0 +1,125 @@ +/* + * Tencent is pleased to support the open source community by making Spring Cloud Tencent available. + * + * Copyright (C) 2019 THL A29 Limited, a Tencent company. All rights reserved. + * + * Licensed under the BSD 3-Clause License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://opensource.org/licenses/BSD-3-Clause + * + * Unless required by applicable law or agreed to in writing, software distributed + * under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR + * CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ + +package com.tencent.cloud.polaris.circuitbreaker.feign; + +import java.lang.reflect.Method; + +import com.tencent.cloud.common.metadata.MetadataContext; +import com.tencent.cloud.common.util.ApplicationContextAwareUtils; +import com.tencent.cloud.common.util.ReflectionUtils; +import com.tencent.cloud.rpc.enhancement.config.RpcEnhancementReporterProperties; +import feign.Target; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import org.springframework.context.ApplicationContext; +import org.springframework.web.bind.annotation.RequestMapping; + +import static com.tencent.polaris.test.common.Consts.NAMESPACE_TEST; +import static com.tencent.polaris.test.common.Consts.SERVICE_PROVIDER; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +/** + * @author sean yu + */ +@ExtendWith(MockitoExtension.class) +public class PolarisCircuitBreakerNameResolverTest { + + private static MockedStatic mockedApplicationContextAwareUtils; + + @BeforeAll + static void beforeAll() { + mockedApplicationContextAwareUtils = Mockito.mockStatic(ApplicationContextAwareUtils.class); + mockedApplicationContextAwareUtils.when(() -> ApplicationContextAwareUtils.getProperties(anyString())) + .thenReturn("unit-test"); + ApplicationContext applicationContext = mock(ApplicationContext.class); + RpcEnhancementReporterProperties reporterProperties = mock(RpcEnhancementReporterProperties.class); + doReturn(reporterProperties) + .when(applicationContext).getBean(RpcEnhancementReporterProperties.class); + mockedApplicationContextAwareUtils.when(ApplicationContextAwareUtils::getApplicationContext) + .thenReturn(applicationContext); + } + + @AfterAll + static void afterAll() { + mockedApplicationContextAwareUtils.close(); + } + + @BeforeEach + void setUp() { + MetadataContext.LOCAL_NAMESPACE = NAMESPACE_TEST; + MetadataContext.LOCAL_SERVICE = SERVICE_PROVIDER; + } + + + @Test + public void test() { + Target target = mock(Target.class); + doReturn("test-svc").when(target).name(); + Method method = ReflectionUtils.findMethod(PolarisCircuitBreakerNameResolverTest.class, "mockRequestMapping"); + PolarisCircuitBreakerNameResolver resolver = new PolarisCircuitBreakerNameResolver(); + String polarisCircuitBreakerName = resolver.resolveCircuitBreakerName("test", target, method); + assertThat(polarisCircuitBreakerName).isEqualTo("Test#test-svc"); + } + + @Test + public void test2() { + Target target = mock(Target.class); + doReturn("test-svc").when(target).name(); + Method method = ReflectionUtils.findMethod(PolarisCircuitBreakerNameResolverTest.class, "mockRequestMapping2"); + PolarisCircuitBreakerNameResolver resolver = new PolarisCircuitBreakerNameResolver(); + String polarisCircuitBreakerName = resolver.resolveCircuitBreakerName("test", target, method); + assertThat(polarisCircuitBreakerName).isEqualTo("Test#test-svc#/"); + } + + @Test + public void test3() { + Target target = mock(Target.class); + doReturn("test-svc").when(target).name(); + Method method = ReflectionUtils.findMethod(PolarisCircuitBreakerNameResolverTest.class, "mockRequestMapping3"); + PolarisCircuitBreakerNameResolver resolver = new PolarisCircuitBreakerNameResolver(); + String polarisCircuitBreakerName = resolver.resolveCircuitBreakerName("test", target, method); + assertThat(polarisCircuitBreakerName).isEqualTo("Test#test-svc#/"); + } + + + @RequestMapping + public void mockRequestMapping() { + + } + + @RequestMapping(path = "/") + public void mockRequestMapping2() { + + } + + @RequestMapping("/") + public void mockRequestMapping3() { + + } + +} diff --git a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/resttemplate/PolarisCircuitBreakerRestTemplateIntegrationTest.java b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/resttemplate/PolarisCircuitBreakerRestTemplateIntegrationTest.java index 1d12a2dc0..2080b63c8 100644 --- a/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/resttemplate/PolarisCircuitBreakerRestTemplateIntegrationTest.java +++ b/spring-cloud-starter-tencent-polaris-circuitbreaker/src/test/java/com/tencent/cloud/polaris/circuitbreaker/resttemplate/PolarisCircuitBreakerRestTemplateIntegrationTest.java @@ -75,6 +75,7 @@ import org.springframework.web.util.DefaultUriBuilderFactory; import static com.tencent.polaris.test.common.Consts.NAMESPACE_TEST; import static com.tencent.polaris.test.common.TestUtils.SERVER_ADDRESS_ENV; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; import static org.springframework.test.web.client.match.MockRestRequestMatchers.method; import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; @@ -142,12 +143,14 @@ public class PolarisCircuitBreakerRestTemplateIntegrationTest { assertThat(defaultRestTemplate.getForObject("http://localhost:18001/example/service/b/info", String.class)).isEqualTo("fallback"); mockServer.verify(); mockServer.reset(); + assertThatThrownBy(() -> { + restTemplateFallbackFromPolaris.getForObject("/example/service/b/info", String.class); + }).isInstanceOf(IllegalStateException.class); assertThat(restTemplateFallbackFromCode.getForObject("/example/service/b/info", String.class)).isEqualTo("\"this is a fallback class\""); Utils.sleepUninterrupted(2000); assertThat(restTemplateFallbackFromCode2.getForObject("/example/service/b/info", String.class)).isEqualTo("\"this is a fallback class\""); Utils.sleepUninterrupted(2000); - assertThat(restTemplateFallbackFromCode3.getForEntity("/example/service/b/info", String.class) - .getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(restTemplateFallbackFromCode3.getForEntity("/example/service/b/info", String.class).getStatusCode()).isEqualTo(HttpStatus.OK); Utils.sleepUninterrupted(2000); assertThat(restTemplateFallbackFromCode4.getForObject("/example/service/b/info", String.class)).isEqualTo("fallback"); Utils.sleepUninterrupted(2000);