Merge pull request #555 from mmanciop/hystrix-webclient-timeouts

Treat HttpClient timeouts the same as Hystrix timeouts

Author: Ryan Baxter

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactory.java

@@ -36,22 +36,24 @@
 import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.cloud.gateway.filter.GatewayFilter;
 import org.springframework.cloud.gateway.filter.GatewayFilterChain;
-import org.springframework.http.HttpStatus;
+import org.springframework.cloud.gateway.support.TimeoutException;
+import org.springframework.core.annotation.AnnotatedElementUtils;
 import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
+import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.reactive.DispatcherHandler;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.util.UriComponentsBuilder;
 
-import static com.netflix.hystrix.exception.HystrixRuntimeException.FailureType.TIMEOUT;
 import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_REQUEST_URL_ATTR;
 import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.containsEncodedParts;
 
 /**
  * Depends on `spring-cloud-starter-netflix-hystrix`, {@see http://cloud.spring.io/spring-cloud-netflix/}
  * @author Spencer Gibb
+ * @author Michele Mancioppi
  */
 public class HystrixGatewayFilterFactory extends AbstractGatewayFilterFactory<HystrixGatewayFilterFactory.Config> {
 
@@ -101,8 +103,24 @@ public GatewayFilter apply(Config config) {
 			}).onErrorResume((Function<Throwable, Mono<Void>>) throwable -> {
 				if (throwable instanceof HystrixRuntimeException) {
 					HystrixRuntimeException e = (HystrixRuntimeException) throwable;
-					if (e.getFailureType() == TIMEOUT) {
-						return Mono.error(new ResponseStatusException(HttpStatus.GATEWAY_TIMEOUT));
+					HystrixRuntimeException.FailureType failureType = e.getFailureType();
+
+					switch (failureType) {
+						case TIMEOUT:
+							return Mono.error(new TimeoutException());
+						case COMMAND_EXCEPTION: {
+							Throwable cause = e.getCause();
+
+							/*
+							 * We forsake here the null check for cause as HystrixRuntimeException will
+							 * always have a cause if the failure type is COMMAND_EXCEPTION.
+							 */
+							if (cause instanceof ResponseStatusException || AnnotatedElementUtils
+									.findMergedAnnotation(cause.getClass(), ResponseStatus.class) != null) {
+								return Mono.error(cause);
+							}
+						}
+						default: break;
 					}
 				}
 				return Mono.error(throwable);

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/TimeoutException.java

@@ -17,6 +17,11 @@
 
 package org.springframework.cloud.gateway.support;
 
+import static org.springframework.http.HttpStatus.GATEWAY_TIMEOUT;
+
+import org.springframework.web.bind.annotation.ResponseStatus;
+
+@ResponseStatus(value = GATEWAY_TIMEOUT, reason = "Response took longer than configured timeout")
 public class TimeoutException extends Exception {
 
 	public TimeoutException() {

spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterIntegrationTests.java

@@ -27,6 +27,7 @@
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.cloud.gateway.test.BaseWebClientTests;
 import org.springframework.context.annotation.Import;
+import org.springframework.http.HttpStatus;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.junit4.SpringRunner;
 
@@ -44,11 +45,11 @@ public void responseTimeoutWorks() {
 		testClient.get()
 				.uri("/delay/5")
 				.exchange()
-				.expectStatus().is5xxServerError()
+				.expectStatus().isEqualTo(HttpStatus.GATEWAY_TIMEOUT)
 				.expectBody(Map.class)
 				.consumeWith(result -> {
 					Map body = result.getResponseBody();
-					assertThat(body).containsEntry("message", "Response took longer than timeout: PT3S");
+					assertThat(body).containsEntry("message", "Response took longer than configured timeout");
 				});
 	}
 

spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactoryTests.java

@@ -74,6 +74,20 @@ public void hystrixFilterTimesout() {
 				.isEqualTo(String.valueOf(HttpStatus.GATEWAY_TIMEOUT.value()));
 	}
 
+	/*
+	 * Tests that timeouts bubbling from the underpinning WebClient are treated the same as
+	 * Hystrix timeouts in terms of outside response. (Internally, timeouts from the WebClient
+	 * are seen as command failures and trigger the opening of circuit breakers the same way
+	 * timeouts do; it may be confusing in terms of the Hystrix metrics though)
+	 */
+	@Test
+	public void hystrixTimeoutFromWebClient() {
+		testClient.get().uri("/delay/10")
+				.header("Host", "www.hystrixresponsestall.org")
+				.exchange()
+				.expectStatus().isEqualTo(HttpStatus.GATEWAY_TIMEOUT);
+	}
+
 	@Test
 	public void hystrixFilterFallback() {
 		testClient.get().uri("/delay/3?a=b")
@@ -157,11 +171,18 @@ public RouteLocator hystrixRouteLocator(RouteLocatorBuilder builder) {
 							.filters(f -> f.prefixPath("/httpbin")
 									.hystrix(config -> {}))
 							.uri("lb:badservice"))
+					/*
+					 * This is a route encapsulated in a hystrix command that is ready to wait
+					 * for a response far longer than the underpinning WebClient would.
+					 */
+					.route("hystrix_response_stall", r -> r.host("**.hystrixresponsestall.org")
+							.filters(f -> f.prefixPath("/httpbin")
+									.hystrix(config -> config.setName("stalling-command")))
+							.uri(uri))
 					.build();
 		}
 	}
 
-
 	protected static class TestBadRibbonConfig {
 
 		@LocalServerPort