diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreakerInvocationHandler.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreakerInvocationHandler.java index da76d175..a85cf471 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreakerInvocationHandler.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignCircuitBreakerInvocationHandler.java @@ -17,6 +17,7 @@ package org.springframework.cloud.openfeign; import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.LinkedHashMap; @@ -29,6 +30,7 @@ import feign.Target; import org.springframework.cloud.client.circuitbreaker.CircuitBreaker; import org.springframework.cloud.client.circuitbreaker.CircuitBreakerFactory; +import org.springframework.cloud.client.circuitbreaker.NoFallbackAvailableException; import org.springframework.web.context.request.RequestAttributes; import org.springframework.web.context.request.RequestContextHolder; @@ -95,15 +97,29 @@ class FeignCircuitBreakerInvocationHandler implements InvocationHandler { try { return this.fallbackMethodMap.get(method).invoke(fallback, args); } - catch (Exception e) { - throw new IllegalStateException(e); + catch (Exception exception) { + unwrapAndRethrow(exception); } + return null; }; return circuitBreaker.run(supplier, fallbackFunction); } return circuitBreaker.run(supplier); } + private void unwrapAndRethrow(Exception exception) { + if (exception instanceof InvocationTargetException || exception instanceof NoFallbackAvailableException) { + Throwable underlyingException = exception.getCause(); + if (underlyingException instanceof RuntimeException) { + throw (RuntimeException) underlyingException; + } + if (underlyingException != null) { + throw new IllegalStateException(underlyingException); + } + throw new IllegalStateException(exception); + } + } + private Supplier asSupplier(final Method method, final Object[] args) { final RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes(); return () -> { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerTests.java index 87676464..da6dfa05 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/circuitbreaker/CircuitBreakerTests.java @@ -16,6 +16,7 @@ package org.springframework.cloud.openfeign.circuitbreaker; +import java.io.IOException; import java.util.function.Function; import org.apache.commons.logging.Log; @@ -47,6 +48,7 @@ import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RestController; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * @author Spencer Gibb @@ -63,6 +65,9 @@ class CircuitBreakerTests { @Autowired TestClient testClient; + @Autowired + ExceptionClient exceptionClient; + @Autowired TestClientWithFactory testClientWithFactory; @@ -109,6 +114,17 @@ class CircuitBreakerTests { assertThat(testClientWithFactory.getException()).isEqualTo("Fixed response"); } + @Test + void testRuntimeExceptionUnwrapped() { + assertThatExceptionOfType(UnsupportedOperationException.class) + .isThrownBy(() -> exceptionClient.getRuntimeException()); + } + + @Test + void testCheckedExceptionWrapped() { + assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> exceptionClient.getCheckedException()); + } + @FeignClient(name = "test", url = "http://localhost:${server.port}/", fallback = Fallback.class) protected interface TestClient { @@ -120,6 +136,18 @@ class CircuitBreakerTests { } + @FeignClient(name = "exceptionClient", url = "http://localhost:${server.port}/", + fallbackFactory = ExceptionThrowingFallbackFactory.class) + protected interface ExceptionClient { + + @GetMapping("/runtimeException") + Hello getRuntimeException(); + + @GetMapping("/runtimeException") + Hello getCheckedException() throws IOException; + + } + @Component static class Fallback implements TestClient { @@ -157,6 +185,25 @@ class CircuitBreakerTests { } + static class ExceptionThrowingFallbackFactory implements FallbackFactory { + + @Override + public ExceptionClient create(Throwable cause) { + return new ExceptionClient() { + @Override + public Hello getRuntimeException() { + throw new UnsupportedOperationException("Not implemented!"); + } + + @Override + public Hello getCheckedException() throws IOException { + throw new IOException(); + } + }; + } + + } + static class FallbackWithFactory implements TestClientWithFactory { @Override @@ -174,7 +221,7 @@ class CircuitBreakerTests { @Configuration(proxyBeanMethods = false) @EnableAutoConfiguration @RestController - @EnableFeignClients(clients = { TestClient.class, TestClientWithFactory.class }) + @EnableFeignClients(clients = { TestClient.class, TestClientWithFactory.class, ExceptionClient.class }) @Import(NoSecurityConfiguration.class) protected static class Application implements TestClient { @@ -226,6 +273,11 @@ class CircuitBreakerTests { return new TestFallbackFactory(); } + @Bean + ExceptionThrowingFallbackFactory exceptionThrowingFallbackFactory() { + return new ExceptionThrowingFallbackFactory(); + } + } }