Wrap underlying checked exception with IllegalStateException. Add tests.

docs/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-openfeign</artifactId>
-		<version>3.1.1-SNAPSHOT</version>
+		<version>3.1.2-SNAPSHOT</version>
 	</parent>
 	<artifactId>spring-cloud-openfeign-docs</artifactId>
 	<packaging>jar</packaging>

pom.xml

@@ -4,14 +4,14 @@
 		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
 	<modelVersion>4.0.0</modelVersion>
 	<artifactId>spring-cloud-openfeign</artifactId>
-	<version>3.1.1-SNAPSHOT</version>
+	<version>3.1.2-SNAPSHOT</version>
 	<packaging>pom</packaging>
 	<name>Spring Cloud OpenFeign</name>
 	<description>Spring Cloud OpenFeign</description>
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-build</artifactId>
-		<version>3.1.1-SNAPSHOT</version>
+		<version>3.1.1</version>
 		<relativePath/>
 	</parent>
 	<scm>
@@ -26,7 +26,7 @@
 	<properties>
 		<main.basedir>${basedir}</main.basedir>
 		<jackson.version>2.11.3</jackson.version>
-		<spring-cloud-commons.version>3.1.1-SNAPSHOT</spring-cloud-commons.version>
+		<spring-cloud-commons.version>3.1.2-SNAPSHOT</spring-cloud-commons.version>
 
 		<!-- Plugin versions -->
 		<maven-eclipse-plugin.version>2.10</maven-eclipse-plugin.version>

spring-cloud-openfeign-core/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-openfeign</artifactId>
-		<version>3.1.1-SNAPSHOT</version>
+		<version>3.1.2-SNAPSHOT</version>
 		<relativePath>..</relativePath> <!-- lookup parent from repository -->
 	</parent>
 	<artifactId>spring-cloud-openfeign-core</artifactId>

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<Object> asSupplier(final Method method, final Object[] args) {
 		final RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
 		return () -> {

spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java

@@ -69,6 +69,7 @@ import org.springframework.util.StringUtils;
  * @author Sam Kruglov
  * @author Jasbir Singh
  * @author Hyeonmin Park
+ * @author Felix Dittrich
  */
 public class FeignClientFactoryBean
 		implements FactoryBean<Object>, InitializingBean, ApplicationContextAware, BeanFactoryAware {
@@ -132,7 +133,6 @@ public class FeignClientFactoryBean
 		// @formatter:on
 
 		configureFeign(context, builder);
-		applyBuildCustomizers(context, builder);
 
 		return builder;
 	}
@@ -374,6 +374,7 @@ public class FeignClientFactoryBean
 		Client client = getOptional(context, Client.class);
 		if (client != null) {
 			builder.client(client);
+			applyBuildCustomizers(context, builder);
 			Targeter targeter = get(context, Targeter.class);
 			return targeter.target(this, builder, context, target);
 		}
@@ -443,6 +444,9 @@ public class FeignClientFactoryBean
 			}
 			builder.client(client);
 		}
+
+		applyBuildCustomizers(context, builder);
+
 		Targeter targeter = get(context, Targeter.class);
 		return (T) targeter.target(this, builder, context, new HardCodedTarget<>(type, name, url));
 	}

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignBuilderCustomizerTests.java

@@ -17,35 +17,56 @@
 package org.springframework.cloud.openfeign;
 
 import java.lang.reflect.Field;
+import java.util.stream.Stream;
 
+import feign.Client;
 import feign.Feign;
 import feign.Logger;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.mockito.ArgumentCaptor;
 
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
+import org.springframework.context.annotation.Primary;
 import org.springframework.core.annotation.Order;
 import org.springframework.util.ReflectionUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doCallRealMethod;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 
 /**
  * @author Matt King
  * @author Sam Kruglov
+ * @author Felix Dittrich
+ * @author Olga Maciaszek-Sharma
  */
 class FeignBuilderCustomizerTests {
 
+	private static final Targeter targeterSpy = spy(DefaultTargeter.class);
+
+	private static final Client defaultClient = mock(Client.class);
+
 	@Test
 	void testBuilderCustomizer() {
+		ArgumentCaptor<Feign.Builder> feignBuilderCaptor = ArgumentCaptor.forClass(Feign.Builder.class);
+		doCallRealMethod().when(targeterSpy).target(any(), feignBuilderCaptor.capture(), any(), any());
+
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
 				FeignBuilderCustomizerTests.SampleConfiguration2.class);
 
 		FeignClientFactoryBean clientFactoryBean = context.getBean(FeignClientFactoryBean.class);
-		FeignContext feignContext = context.getBean(FeignContext.class);
+		clientFactoryBean.getTarget();
 
-		Feign.Builder builder = clientFactoryBean.feign(feignContext);
+		Assertions.assertNotNull(feignBuilderCaptor.getValue());
+		Feign.Builder builder = feignBuilderCaptor.getValue();
 		assertFeignBuilderField(builder, "logLevel", Logger.Level.HEADERS);
 		assertFeignBuilderField(builder, "decode404", true);
 
@@ -62,13 +83,17 @@ class FeignBuilderCustomizerTests {
 
 	@Test
 	void testBuildCustomizerOrdered() {
+		ArgumentCaptor<Feign.Builder> feignBuilderCaptor = ArgumentCaptor.forClass(Feign.Builder.class);
+		doCallRealMethod().when(targeterSpy).target(any(), feignBuilderCaptor.capture(), any(), any());
+
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
 				FeignBuilderCustomizerTests.SampleConfiguration3.class);
 
 		FeignClientFactoryBean clientFactoryBean = context.getBean(FeignClientFactoryBean.class);
-		FeignContext feignContext = context.getBean(FeignContext.class);
+		clientFactoryBean.getTarget();
 
-		Feign.Builder builder = clientFactoryBean.feign(feignContext);
+		Assertions.assertNotNull(feignBuilderCaptor.getValue());
+		Feign.Builder builder = feignBuilderCaptor.getValue();
 		assertFeignBuilderField(builder, "logLevel", Logger.Level.FULL);
 		assertFeignBuilderField(builder, "decode404", true);
 
@@ -77,15 +102,19 @@ class FeignBuilderCustomizerTests {
 
 	@Test
 	void testBuildCustomizerOrderedWithAdditional() {
+		ArgumentCaptor<Feign.Builder> feignBuilderCaptor = ArgumentCaptor.forClass(Feign.Builder.class);
+		doCallRealMethod().when(targeterSpy).target(any(), feignBuilderCaptor.capture(), any(), any());
+
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
 				FeignBuilderCustomizerTests.SampleConfiguration3.class);
 
 		FeignClientFactoryBean clientFactoryBean = context.getBean(FeignClientFactoryBean.class);
 		clientFactoryBean.addCustomizer(builder -> builder.logLevel(Logger.Level.BASIC));
 		clientFactoryBean.addCustomizer(Feign.Builder::doNotCloseAfterDecode);
-		FeignContext feignContext = context.getBean(FeignContext.class);
+		clientFactoryBean.getTarget();
 
-		Feign.Builder builder = clientFactoryBean.feign(feignContext);
+		Assertions.assertNotNull(feignBuilderCaptor.getValue());
+		Feign.Builder builder = feignBuilderCaptor.getValue();
 		assertFeignBuilderField(builder, "logLevel", Logger.Level.BASIC);
 		assertFeignBuilderField(builder, "decode404", true);
 		assertFeignBuilderField(builder, "closeAfterDecode", false);
@@ -93,16 +122,41 @@ class FeignBuilderCustomizerTests {
 		context.close();
 	}
 
-	private static FeignClientFactoryBean defaultFeignClientFactoryBean() {
+	@ParameterizedTest(name = "should use custom HttpClient with config: {0}")
+	@MethodSource("testConfiguration")
+	void testBuildCustomizerWithCustomHttpClient(Class configClass) {
+		ArgumentCaptor<Feign.Builder> feignBuilderCaptor = ArgumentCaptor.forClass(Feign.Builder.class);
+		doCallRealMethod().when(targeterSpy).target(any(), feignBuilderCaptor.capture(), any(), any());
+		Client customClientMock = mock(Client.class);
+
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(configClass);
+		FeignClientFactoryBean clientFactoryBean = context.getBean(FeignClientFactoryBean.class);
+		clientFactoryBean.addCustomizer(builder -> builder.client(customClientMock));
+		clientFactoryBean.getTarget();
+
+		Assertions.assertNotNull(feignBuilderCaptor.getValue());
+		Feign.Builder builder = feignBuilderCaptor.getValue();
+		assertFeignBuilderField(builder, "client", customClientMock);
+
+		context.close();
+	}
+
+	private static FeignClientFactoryBean defaultFeignClientFactoryBean(String url) {
 		FeignClientFactoryBean feignClientFactoryBean = new FeignClientFactoryBean();
 		feignClientFactoryBean.setContextId("test");
 		feignClientFactoryBean.setName("test");
 		feignClientFactoryBean.setType(FeignClientFactoryTests.TestType.class);
 		feignClientFactoryBean.setPath("");
-		feignClientFactoryBean.setUrl("http://some.absolute.url");
+		if (url != null) {
+			feignClientFactoryBean.setUrl(url);
+		}
 		return feignClientFactoryBean;
 	}
 
+	private static Stream<Class> testConfiguration() {
+		return Stream.of(SampleConfiguration3.class, LoadBalancedSampleConfiguration.class);
+	}
+
 	@Configuration(proxyBeanMethods = false)
 	@Import(FeignClientsConfiguration.class)
 	protected static class SampleConfiguration2 {
@@ -129,7 +183,12 @@ class FeignBuilderCustomizerTests {
 
 		@Bean
 		FeignClientFactoryBean feignClientFactoryBean() {
-			return defaultFeignClientFactoryBean();
+			return defaultFeignClientFactoryBean("http://some.absolute.url");
+		}
+
+		@Bean
+		Targeter targeter() {
+			return targeterSpy;
 		}
 
 	}
@@ -167,7 +226,29 @@ class FeignBuilderCustomizerTests {
 
 		@Bean
 		FeignClientFactoryBean feignClientFactoryBean() {
-			return defaultFeignClientFactoryBean();
+			return defaultFeignClientFactoryBean("http://some.absolute.url");
+		}
+
+		@Bean
+		Targeter targeter() {
+			return targeterSpy;
+		}
+
+		@Bean
+		Client client() {
+			return defaultClient;
+		}
+
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	@Import(SampleConfiguration3.class)
+	protected static class LoadBalancedSampleConfiguration {
+
+		@Primary
+		@Bean
+		FeignClientFactoryBean feignClientFactoryBean() {
+			return defaultFeignClientFactoryBean(null);
 		}
 
 	}

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
@@ -62,6 +64,9 @@ class CircuitBreakerTests {
 	@Autowired
 	TestClient testClient;
 
+	@Autowired
+	ExceptionClient exceptionClient;
+
 	@Autowired
 	TestClientWithFactory testClientWithFactory;
 
@@ -108,6 +113,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 {
 
@@ -119,6 +135,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 {
 
@@ -156,6 +184,25 @@ class CircuitBreakerTests {
 
 	}
 
+	static class ExceptionThrowingFallbackFactory implements FallbackFactory<ExceptionClient> {
+
+		@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
@@ -173,7 +220,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 {
 
@@ -225,6 +272,11 @@ class CircuitBreakerTests {
 			return new TestFallbackFactory();
 		}
 
+		@Bean
+		ExceptionThrowingFallbackFactory exceptionThrowingFallbackFactory() {
+			return new ExceptionThrowingFallbackFactory();
+		}
+
 	}
 
 }

spring-cloud-openfeign-dependencies/pom.xml

@@ -6,11 +6,11 @@
 	<parent>
 		<artifactId>spring-cloud-dependencies-parent</artifactId>
 		<groupId>org.springframework.cloud</groupId>
-		<version>3.1.0</version>
+		<version>3.1.2-SNAPSHOT</version>
 		<relativePath/>
 	</parent>
 	<artifactId>spring-cloud-openfeign-dependencies</artifactId>
-	<version>3.1.1-SNAPSHOT</version>
+	<version>3.1.2-SNAPSHOT</version>
 	<packaging>pom</packaging>
 	<name>spring-cloud-openfeign-dependencies</name>
 	<description>Spring Cloud OpenFeign Dependencies</description>

spring-cloud-starter-openfeign/pom.xml

@@ -5,7 +5,7 @@
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-openfeign</artifactId>
-		<version>3.1.1-SNAPSHOT</version>
+		<version>3.1.2-SNAPSHOT</version>
 		<relativePath>..</relativePath>
 	</parent>
 	<artifactId>spring-cloud-starter-openfeign</artifactId>