Zuul filters missing error status code (#1088)
* Added missing error code for SimpleHostRoutingFilter * Added missing error code for RibbonRoutingFilter
This commit is contained in:
@@ -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();
|
||||
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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<String> 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);
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String> 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<Map<String, Object>> error(HttpServletRequest request) {
|
||||
controllerUsed.set(true);
|
||||
return super.error(request);
|
||||
}
|
||||
|
||||
public boolean wasControllerUsed() {
|
||||
return this.controllerUsed.get();
|
||||
}
|
||||
|
||||
public void clear() {
|
||||
this.controllerUsed.set(false);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user