Treat WebClient timeouts the same as Hystrix timeouts

WebClient timeouts from Hystrix-wrapped routes resulted in status code 500
sent back to the client of the gateway, while Hystrix-issued timeouts
result in 504 (which is the expected behavior).

This change harmonizes the behavior of Hystrix- and WebClient-issued timeouts
on Hystrix-wrapped routes to both return 504.

Fixes gh-554

Author: michele-mancioppi

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

@@ -25,6 +25,7 @@
 
 import org.springframework.cloud.gateway.filter.GatewayFilter;
 import org.springframework.cloud.gateway.filter.GatewayFilterChain;
+import org.springframework.cloud.gateway.support.TimeoutException;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.util.Assert;
@@ -39,6 +40,7 @@
 import com.netflix.hystrix.HystrixObservableCommand.Setter;
 import com.netflix.hystrix.exception.HystrixRuntimeException;
 
+import static com.netflix.hystrix.exception.HystrixRuntimeException.FailureType.COMMAND_EXCEPTION;
 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;
@@ -52,6 +54,7 @@
 /**
  * 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,7 +104,15 @@ public GatewayFilter apply(Config config) {
 			}).onErrorResume((Function<Throwable, Mono<Void>>) throwable -> {
 				if (throwable instanceof HystrixRuntimeException) {
 					HystrixRuntimeException e = (HystrixRuntimeException) throwable;
-					if (e.getFailureType() == TIMEOUT) { //TODO: optionally set status
+					HystrixRuntimeException.FailureType failureType = e.getFailureType();
+
+					if (failureType == TIMEOUT ||
+							/*
+							 * In this case, the timeout originates from WebClient due to the settings
+							 * of `spring.cloud.gateway.httpclient.responseTimeout`
+							 */
+							(failureType == COMMAND_EXCEPTION && e.getCause() instanceof TimeoutException)) {
+
 						setResponseStatus(exchange, HttpStatus.GATEWAY_TIMEOUT);
 						return exchange.getResponse().setComplete();
 					}

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

@@ -47,7 +47,11 @@
 import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
 
 @RunWith(SpringRunner.class)
-@SpringBootTest(webEnvironment = RANDOM_PORT)
+@SpringBootTest(webEnvironment = RANDOM_PORT,
+	properties = {
+		"spring.cloud.gateway.httpclient.responseTimeout=3s",
+		"hystrix.command.stalling-command.execution.isolation.thread.timeoutInMilliseconds=10000"
+	})
 @DirtiesContext
 public class HystrixGatewayFilterFactoryTests extends BaseWebClientTests {
 
@@ -68,6 +72,20 @@ public void hystrixFilterTimesout() {
 				.expectStatus().isEqualTo(HttpStatus.GATEWAY_TIMEOUT);
 	}
 
+	/*
+	 * 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")
@@ -134,6 +152,14 @@ 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();
 		}
 	}