Don't send 4xx errors to /error

fixes gh-1110
parent 050c6625
...@@ -78,7 +78,6 @@ public class RibbonRoutingFilter extends ZuulFilter { ...@@ -78,7 +78,6 @@ public class RibbonRoutingFilter extends ZuulFilter {
RibbonCommandContext commandContext = buildCommandContext(context); RibbonCommandContext commandContext = buildCommandContext(context);
ClientHttpResponse response = forward(commandContext); ClientHttpResponse response = forward(commandContext);
setResponse(response); setResponse(response);
setErrorCodeFor4xx(context, response);
return response; return response;
} }
catch (ZuulException ex) { catch (ZuulException ex) {
...@@ -94,14 +93,6 @@ public class RibbonRoutingFilter extends ZuulFilter { ...@@ -94,14 +93,6 @@ public class RibbonRoutingFilter extends ZuulFilter {
return null; 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) { protected RibbonCommandContext buildCommandContext(RequestContext context) {
HttpServletRequest request = context.getRequest(); HttpServletRequest request = context.getRequest();
......
...@@ -175,7 +175,6 @@ public class SimpleHostRoutingFilter extends ZuulFilter { ...@@ -175,7 +175,6 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
HttpResponse response = forward(this.httpClient, verb, uri, request, headers, HttpResponse response = forward(this.httpClient, verb, uri, request, headers,
params, requestEntity); params, requestEntity);
setResponse(response); setResponse(response);
setErrorCodeFor4xx(context, response);
} }
catch (Exception ex) { catch (Exception ex) {
context.set(ERROR_STATUS_CODE, context.set(ERROR_STATUS_CODE,
...@@ -185,13 +184,6 @@ public class SimpleHostRoutingFilter extends ZuulFilter { ...@@ -185,13 +184,6 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
return null; 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() { protected PoolingHttpClientConnectionManager newConnectionManager() {
try { try {
final SSLContext sslContext = SSLContext.getInstance("SSL"); final SSLContext sslContext = SSLContext.getInstance("SSL");
......
...@@ -113,11 +113,13 @@ public class SampleZuulProxyApplicationTests extends ZuulProxyTestBase { ...@@ -113,11 +113,13 @@ public class SampleZuulProxyApplicationTests extends ZuulProxyTestBase {
public void simpleHostRouteWithNonExistentUrl() { public void simpleHostRouteWithNonExistentUrl() {
this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/"); this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/");
this.endpoint.reset(); this.endpoint.reset();
String uri = "/self/nonExistentUrl";
this.myErrorController.setUriToMatch(uri);
ResponseEntity<String> result = new TestRestTemplate().exchange( 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); new HttpEntity<>((Void) null), String.class);
assertEquals(HttpStatus.NOT_FOUND, result.getStatusCode()); assertEquals(HttpStatus.NOT_FOUND, result.getStatusCode());
assertTrue(this.myErrorController.wasControllerUsed()); assertFalse(this.myErrorController.wasControllerUsed());
} }
@Test @Test
......
...@@ -43,7 +43,6 @@ import com.netflix.zuul.context.RequestContext; ...@@ -43,7 +43,6 @@ import com.netflix.zuul.context.RequestContext;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
/** /**
* @author Spencer Gibb * @author Spencer Gibb
...@@ -155,8 +154,10 @@ public abstract class ZuulProxyTestBase { ...@@ -155,8 +154,10 @@ public abstract class ZuulProxyTestBase {
@Test @Test
public void ribbonRouteWithSpace() { public void ribbonRouteWithSpace() {
String uri = "/simple/spa ce";
this.myErrorController.setUriToMatch(uri);
ResponseEntity<String> result = new TestRestTemplate().exchange( 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); new HttpEntity<>((Void) null), String.class);
assertEquals(HttpStatus.OK, result.getStatusCode()); assertEquals(HttpStatus.OK, result.getStatusCode());
assertEquals("Hello space", result.getBody()); assertEquals("Hello space", result.getBody());
...@@ -165,11 +166,13 @@ public abstract class ZuulProxyTestBase { ...@@ -165,11 +166,13 @@ public abstract class ZuulProxyTestBase {
@Test @Test
public void ribbonRouteWithNonExistentUri() { public void ribbonRouteWithNonExistentUri() {
String uri = "/simple/nonExistent";
this.myErrorController.setUriToMatch(uri);
ResponseEntity<String> result = new TestRestTemplate().exchange( 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); new HttpEntity<>((Void) null), String.class);
assertEquals(HttpStatus.NOT_FOUND, result.getStatusCode()); assertEquals(HttpStatus.NOT_FOUND, result.getStatusCode());
assertTrue(myErrorController.wasControllerUsed()); assertFalse(myErrorController.wasControllerUsed());
} }
@Test @Test
...@@ -325,6 +328,7 @@ class AnotherRibbonClientConfiguration { ...@@ -325,6 +328,7 @@ class AnotherRibbonClientConfiguration {
} }
class MyErrorController extends BasicErrorController { class MyErrorController extends BasicErrorController {
ThreadLocal<String> uriToMatch = new ThreadLocal<>();
AtomicBoolean controllerUsed = new AtomicBoolean(); AtomicBoolean controllerUsed = new AtomicBoolean();
...@@ -334,10 +338,19 @@ class MyErrorController extends BasicErrorController { ...@@ -334,10 +338,19 @@ class MyErrorController extends BasicErrorController {
@Override @Override
public ResponseEntity<Map<String, Object>> error(HttpServletRequest request) { 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); return super.error(request);
} }
public void setUriToMatch(String uri) {
this.uriToMatch.set(uri);
}
public boolean wasControllerUsed() { public boolean wasControllerUsed() {
return this.controllerUsed.get(); return this.controllerUsed.get();
} }
...@@ -345,4 +358,4 @@ class MyErrorController extends BasicErrorController { ...@@ -345,4 +358,4 @@ class MyErrorController extends BasicErrorController {
public void clear() { public void clear() {
this.controllerUsed.set(false); this.controllerUsed.set(false);
} }
} }
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment