diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java index e945ab5a..3903822b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java @@ -39,6 +39,7 @@ import lombok.extern.apachecommons.CommonsLog; @CommonsLog public class RibbonRoutingFilter extends ZuulFilter { + private static final String ERROR_STATUS_CODE = "error.status_code"; protected ProxyRequestHelper helper; protected RibbonCommandFactory ribbonCommandFactory; @@ -77,10 +78,11 @@ public class RibbonRoutingFilter extends ZuulFilter { RibbonCommandContext commandContext = buildCommandContext(context); ClientHttpResponse response = forward(commandContext); setResponse(response); + setErrorCodeFor4xx(context, response); return response; } catch (ZuulException ex) { - context.set("error.status_code", ex.nStatusCode); + context.set(ERROR_STATUS_CODE, ex.nStatusCode); context.set("error.message", ex.errorCause); context.set("error.exception", ex); } @@ -92,6 +94,14 @@ public class RibbonRoutingFilter extends ZuulFilter { return null; } + private void setErrorCodeFor4xx(RequestContext context, ClientHttpResponse response) + throws IOException { + HttpStatus httpStatus = response.getStatusCode(); + if (httpStatus.is4xxClientError()) { + context.set(ERROR_STATUS_CODE, httpStatus.value()); + } + } + protected RibbonCommandContext buildCommandContext(RequestContext context) { HttpServletRequest request = context.getRequest(); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java index fe0b8fa1..ae1d3f43 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java @@ -66,6 +66,7 @@ import org.apache.http.protocol.HttpContext; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.Host; +import org.springframework.http.HttpStatus; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; @@ -88,6 +89,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter { private static final DynamicIntProperty CONNECTION_TIMEOUT = DynamicPropertyFactory .getInstance() .getIntProperty(ZuulConstants.ZUUL_HOST_CONNECT_TIMEOUT_MILLIS, 2000); + private static final String ERROR_STATUS_CODE = "error.status_code"; private final Timer connectionManagerTimer = new Timer( "SimpleHostRoutingFilter.connectionManagerTimer", true); @@ -173,15 +175,23 @@ public class SimpleHostRoutingFilter extends ZuulFilter { HttpResponse response = forward(this.httpClient, verb, uri, request, headers, params, requestEntity); setResponse(response); + setErrorCodeFor4xx(context, response); } catch (Exception ex) { - context.set("error.status_code", + context.set(ERROR_STATUS_CODE, HttpServletResponse.SC_INTERNAL_SERVER_ERROR); context.set("error.exception", ex); } return null; } + private void setErrorCodeFor4xx(RequestContext context, HttpResponse response) { + HttpStatus httpStatus = HttpStatus.valueOf(response.getStatusLine().getStatusCode()); + if (httpStatus.is4xxClientError()) { + context.set(ERROR_STATUS_CODE, httpStatus.value()); + } + } + protected PoolingHttpClientConnectionManager newConnectionManager() { try { final SSLContext sslContext = SSLContext.getInstance("SSL"); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyApplicationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyApplicationTests.java index acab11cd..35eb1281 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyApplicationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyApplicationTests.java @@ -18,15 +18,21 @@ package org.springframework.cloud.netflix.zuul; import java.io.InputStream; import java.net.URISyntaxException; +import java.util.Map; import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.http.HttpServletRequest; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.autoconfigure.web.BasicErrorController; +import org.springframework.boot.autoconfigure.web.ErrorAttributes; +import org.springframework.boot.autoconfigure.web.ErrorProperties; import org.springframework.boot.test.IntegrationTest; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.TestRestTemplate; @@ -67,14 +73,15 @@ import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerList; import com.netflix.niws.client.http.RestClient; +import lombok.SneakyThrows; + import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -import lombok.SneakyThrows; - @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(classes = SampleZuulProxyApplication.class) @WebAppConfiguration @@ -99,6 +106,18 @@ public class SampleZuulProxyApplicationTests extends ZuulProxyTestBase { new HttpEntity<>((Void) null), String.class); assertEquals(HttpStatus.OK, result.getStatusCode()); assertEquals("/trailing-slash", result.getBody()); + assertFalse(this.myErrorController.wasControllerUsed()); + } + + @Test + public void simpleHostRouteWithNonExistentUrl() { + this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/"); + this.endpoint.reset(); + ResponseEntity result = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/self/nonExistentUrl", HttpMethod.GET, + new HttpEntity<>((Void) null), String.class); + assertEquals(HttpStatus.NOT_FOUND, result.getStatusCode()); + assertTrue(this.myErrorController.wasControllerUsed()); } @Test @@ -274,6 +293,11 @@ class SampleZuulProxyApplication extends ZuulProxyTestBase.AbstractZuulProxyAppl return new MyRouteLocator("/", discoveryClient, zuulProperties); } + @Bean + public MyErrorController myErrorController(ErrorAttributes errorAttributes) { + return new MyErrorController(errorAttributes); + } + public static void main(String[] args) { SpringApplication.run(SampleZuulProxyApplication.class, args); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyWithHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyWithHttpClientTests.java index e144acb2..2bee1dc2 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyWithHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyWithHttpClientTests.java @@ -21,6 +21,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.autoconfigure.web.ErrorAttributes; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.TestRestTemplate; import org.springframework.boot.test.WebIntegrationTest; @@ -131,4 +132,8 @@ class SampleHttpClientZuulProxyApplication extends ZuulProxyTestBase.AbstractZuu return new HttpClientRibbonCommandFactory(clientFactory); } + @Bean + public MyErrorController myErrorController(ErrorAttributes errorAttributes) { + return new MyErrorController(errorAttributes); + } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyTestBase.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyTestBase.java index 4299e157..33fc36bc 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyTestBase.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyTestBase.java @@ -4,11 +4,17 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; + +import javax.servlet.http.HttpServletRequest; import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.web.BasicErrorController; +import org.springframework.boot.autoconfigure.web.ErrorAttributes; +import org.springframework.boot.autoconfigure.web.ErrorProperties; import org.springframework.boot.test.TestRestTemplate; import org.springframework.cloud.netflix.ribbon.StaticServerList; import org.springframework.cloud.netflix.zuul.filters.Route; @@ -36,6 +42,8 @@ import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.context.RequestContext; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; /** * @author Spencer Gibb @@ -54,6 +62,14 @@ public abstract class ZuulProxyTestBase { @Autowired protected RibbonCommandFactory ribbonCommandFactory; + @Autowired + protected MyErrorController myErrorController; + + @Before + public void cleanup() { + this.myErrorController.clear(); + } + @Before public void setTestRequestcontext() { RequestContext.testSetCurrentContext(null); @@ -144,6 +160,16 @@ public abstract class ZuulProxyTestBase { new HttpEntity<>((Void) null), String.class); assertEquals(HttpStatus.OK, result.getStatusCode()); assertEquals("Hello space", result.getBody()); + assertFalse(myErrorController.wasControllerUsed()); + } + + @Test + public void ribbonRouteWithNonExistentUri() { + ResponseEntity result = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/simple/nonExistent", HttpMethod.GET, + new HttpEntity<>((Void) null), String.class); + assertEquals(HttpStatus.NOT_FOUND, result.getStatusCode()); + assertTrue(myErrorController.wasControllerUsed()); } @Test @@ -296,4 +322,27 @@ class AnotherRibbonClientConfiguration { return new StaticServerList<>(new Server("localhost", this.port)); } +} + +class MyErrorController extends BasicErrorController { + + AtomicBoolean controllerUsed = new AtomicBoolean(); + + public MyErrorController(ErrorAttributes errorAttributes) { + super(errorAttributes, new ErrorProperties()); + } + + @Override + public ResponseEntity> error(HttpServletRequest request) { + controllerUsed.set(true); + return super.error(request); + } + + public boolean wasControllerUsed() { + return this.controllerUsed.get(); + } + + public void clear() { + this.controllerUsed.set(false); + } } \ No newline at end of file