Don't send 4xx errors to /error
fixes gh-1110
This commit is contained in:
@@ -78,7 +78,6 @@ public class RibbonRoutingFilter extends ZuulFilter {
|
||||
RibbonCommandContext commandContext = buildCommandContext(context);
|
||||
ClientHttpResponse response = forward(commandContext);
|
||||
setResponse(response);
|
||||
setErrorCodeFor4xx(context, response);
|
||||
return response;
|
||||
}
|
||||
catch (ZuulException ex) {
|
||||
@@ -94,14 +93,6 @@ 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();
|
||||
|
||||
|
||||
@@ -175,7 +175,6 @@ 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,
|
||||
@@ -185,13 +184,6 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
|
||||
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");
|
||||
|
||||
@@ -113,11 +113,13 @@ public class SampleZuulProxyApplicationTests extends ZuulProxyTestBase {
|
||||
public void simpleHostRouteWithNonExistentUrl() {
|
||||
this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/");
|
||||
this.endpoint.reset();
|
||||
String uri = "/self/nonExistentUrl";
|
||||
this.myErrorController.setUriToMatch(uri);
|
||||
ResponseEntity<String> result = new TestRestTemplate().exchange(
|
||||
"http://localhost:" + this.port + "/self/nonExistentUrl", HttpMethod.GET,
|
||||
"http://localhost:" + this.port + uri, HttpMethod.GET,
|
||||
new HttpEntity<>((Void) null), String.class);
|
||||
assertEquals(HttpStatus.NOT_FOUND, result.getStatusCode());
|
||||
assertTrue(this.myErrorController.wasControllerUsed());
|
||||
assertFalse(this.myErrorController.wasControllerUsed());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -43,7 +43,6 @@ 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
|
||||
@@ -155,8 +154,10 @@ public abstract class ZuulProxyTestBase {
|
||||
|
||||
@Test
|
||||
public void ribbonRouteWithSpace() {
|
||||
String uri = "/simple/spa ce";
|
||||
this.myErrorController.setUriToMatch(uri);
|
||||
ResponseEntity<String> result = new TestRestTemplate().exchange(
|
||||
"http://localhost:" + this.port + "/simple/spa ce", HttpMethod.GET,
|
||||
"http://localhost:" + this.port + uri, HttpMethod.GET,
|
||||
new HttpEntity<>((Void) null), String.class);
|
||||
assertEquals(HttpStatus.OK, result.getStatusCode());
|
||||
assertEquals("Hello space", result.getBody());
|
||||
@@ -165,11 +166,13 @@ public abstract class ZuulProxyTestBase {
|
||||
|
||||
@Test
|
||||
public void ribbonRouteWithNonExistentUri() {
|
||||
String uri = "/simple/nonExistent";
|
||||
this.myErrorController.setUriToMatch(uri);
|
||||
ResponseEntity<String> result = new TestRestTemplate().exchange(
|
||||
"http://localhost:" + this.port + "/simple/nonExistent", HttpMethod.GET,
|
||||
"http://localhost:" + this.port + uri, HttpMethod.GET,
|
||||
new HttpEntity<>((Void) null), String.class);
|
||||
assertEquals(HttpStatus.NOT_FOUND, result.getStatusCode());
|
||||
assertTrue(myErrorController.wasControllerUsed());
|
||||
assertFalse(myErrorController.wasControllerUsed());
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -325,6 +328,7 @@ class AnotherRibbonClientConfiguration {
|
||||
}
|
||||
|
||||
class MyErrorController extends BasicErrorController {
|
||||
ThreadLocal<String> uriToMatch = new ThreadLocal<>();
|
||||
|
||||
AtomicBoolean controllerUsed = new AtomicBoolean();
|
||||
|
||||
@@ -334,10 +338,19 @@ class MyErrorController extends BasicErrorController {
|
||||
|
||||
@Override
|
||||
public ResponseEntity<Map<String, Object>> error(HttpServletRequest request) {
|
||||
controllerUsed.set(true);
|
||||
String errorUri = (String) request.getAttribute("javax.servlet.error.request_uri");
|
||||
|
||||
if (errorUri != null && errorUri.equals(this.uriToMatch.get())) {
|
||||
controllerUsed.set(true);
|
||||
}
|
||||
this.uriToMatch.remove();
|
||||
return super.error(request);
|
||||
}
|
||||
|
||||
public void setUriToMatch(String uri) {
|
||||
this.uriToMatch.set(uri);
|
||||
}
|
||||
|
||||
public boolean wasControllerUsed() {
|
||||
return this.controllerUsed.get();
|
||||
}
|
||||
@@ -345,4 +358,4 @@ class MyErrorController extends BasicErrorController {
|
||||
public void clear() {
|
||||
this.controllerUsed.set(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user