Return a client response from filter function instead of throwing an exception. Fixes #386 (#430)

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunction.java

@@ -2,9 +2,11 @@ package org.springframework.cloud.client.loadbalancer.reactive;
 
 import java.net.URI;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.loadbalancer.LoadBalancerClient;
-import org.springframework.util.Assert;
+import org.springframework.http.HttpStatus;
 import org.springframework.web.reactive.function.client.ClientRequest;
 import org.springframework.web.reactive.function.client.ClientResponse;
 import org.springframework.web.reactive.function.client.ExchangeFilterFunction;
@@ -14,9 +16,13 @@ import reactor.core.publisher.Mono;
 
 /**
  * @author Spencer Gibb
+ * @author Ryan Baxter
  */
 public class LoadBalancerExchangeFilterFunction implements ExchangeFilterFunction {
 
+	private static Log logger = LogFactory
+			.getLog(LoadBalancerExchangeFilterFunction.class);
+
 	private final LoadBalancerClient loadBalancerClient;
 
 	public LoadBalancerExchangeFilterFunction(LoadBalancerClient loadBalancerClient) {
@@ -27,10 +33,18 @@ public class LoadBalancerExchangeFilterFunction implements ExchangeFilterFunctio
 	public Mono<ClientResponse> filter(ClientRequest request, ExchangeFunction next) {
 		URI originalUrl = request.url();
 		String serviceId = originalUrl.getHost();
-		Assert.state(serviceId != null, "Request URI does not contain a valid hostname: " + originalUrl);
+		if(serviceId == null) {
+			String msg = String.format("Request URI does not contain a valid hostname: %s", originalUrl.toString());
+			logger.warn(msg);
+			return Mono.just(ClientResponse.create(HttpStatus.BAD_REQUEST).body(msg).build());
+		}
 		//TODO: reactive lb client
-
 		ServiceInstance instance = this.loadBalancerClient.choose(serviceId);
+		if(instance == null) {
+			String msg = String.format("Load balancer does not contain an instance for the service %s", serviceId);
+			logger.warn(msg);
+			return Mono.just(ClientResponse.create(HttpStatus.SERVICE_UNAVAILABLE).body(msg).build());
+		}
 		URI uri = this.loadBalancerClient.reconstructURI(instance, originalUrl);
 		ClientRequest newRequest = ClientRequest.method(request.method(), uri)
 				.headers(headers -> headers.addAll(request.headers()))

spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunctionTests.java

@@ -22,9 +22,11 @@ import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperti
 import org.springframework.cloud.client.loadbalancer.LoadBalancerClient;
 import org.springframework.cloud.client.loadbalancer.LoadBalancerRequest;
 import org.springframework.context.annotation.Bean;
+import org.springframework.http.HttpStatus;
 import org.springframework.test.context.junit4.SpringRunner;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.reactive.function.client.ClientResponse;
 import org.springframework.web.reactive.function.client.WebClient;
 import org.springframework.web.util.UriComponentsBuilder;
 
@@ -33,6 +35,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen
 
 /**
  * @author Spencer Gibb
+ * @author Ryan Baxter
  */
 @RunWith(SpringRunner.class)
 @SpringBootTest(webEnvironment = RANDOM_PORT)
@@ -68,6 +71,26 @@ public class LoadBalancerExchangeFilterFunctionTests {
 		assertThat(value).isEqualTo("Hello World");
 	}
 
+	@Test
+	public void testNoInstance() {
+		ClientResponse clientResponse = WebClient.builder()
+				.baseUrl("http://foobar")
+				.filter(lbFunction)
+				.build()
+				.get().exchange().block();
+		assertThat(clientResponse.statusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE);
+	}
+
+	@Test
+	public void testNoHostName() {
+		ClientResponse clientResponse = WebClient.builder()
+				.baseUrl("http:///foobar")
+				.filter(lbFunction)
+				.build()
+				.get().exchange().block();
+		assertThat(clientResponse.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
+	}
+
 	@EnableDiscoveryClient
 	@EnableAutoConfiguration
 	@SpringBootConfiguration
@@ -106,6 +129,9 @@ public class LoadBalancerExchangeFilterFunctionTests {
 				@Override
 				public ServiceInstance choose(String serviceId) {
 					List<ServiceInstance> instances = discoveryClient.getInstances(serviceId);
+					if(instances.size() == 0) {
+						return null;
+					}
 					int instanceIdx = random.nextInt(instances.size());
 					return instances.get(instanceIdx);
 				}