Move SendErrorFilter to type error.

This will clear up empty 200 responses. fixes gh-1123
parent 2e46bdf9
...@@ -18,18 +18,23 @@ package org.springframework.cloud.netflix.zuul.filters.post; ...@@ -18,18 +18,23 @@ package org.springframework.cloud.netflix.zuul.filters.post;
import javax.servlet.RequestDispatcher; import javax.servlet.RequestDispatcher;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import com.netflix.zuul.exception.ZuulException;
import lombok.extern.apachecommons.CommonsLog; import lombok.extern.apachecommons.CommonsLog;
import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.annotation.Value;
import org.springframework.cloud.netflix.zuul.util.ZuulRuntimeException;
import org.springframework.util.ReflectionUtils; import org.springframework.util.ReflectionUtils;
import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.ZuulFilter;
import com.netflix.zuul.context.RequestContext; import com.netflix.zuul.context.RequestContext;
import org.springframework.util.StringUtils;
/** /**
* @author Spencer Gibb * @author Spencer Gibb
*/ */
//TODO: move to error package in Edgware
@CommonsLog @CommonsLog
public class SendErrorFilter extends ZuulFilter { public class SendErrorFilter extends ZuulFilter {
...@@ -40,7 +45,7 @@ public class SendErrorFilter extends ZuulFilter { ...@@ -40,7 +45,7 @@ public class SendErrorFilter extends ZuulFilter {
@Override @Override
public String filterType() { public String filterType() {
return "post"; return "error";
} }
@Override @Override
...@@ -52,7 +57,7 @@ public class SendErrorFilter extends ZuulFilter { ...@@ -52,7 +57,7 @@ public class SendErrorFilter extends ZuulFilter {
public boolean shouldFilter() { public boolean shouldFilter() {
RequestContext ctx = RequestContext.getCurrentContext(); RequestContext ctx = RequestContext.getCurrentContext();
// only forward to errorPath if it hasn't been forwarded to already // only forward to errorPath if it hasn't been forwarded to already
return ctx.containsKey("error.status_code") return ctx.getThrowable() != null
&& !ctx.getBoolean(SEND_ERROR_FILTER_RAN, false); && !ctx.getBoolean(SEND_ERROR_FILTER_RAN, false);
} }
...@@ -60,20 +65,16 @@ public class SendErrorFilter extends ZuulFilter { ...@@ -60,20 +65,16 @@ public class SendErrorFilter extends ZuulFilter {
public Object run() { public Object run() {
try { try {
RequestContext ctx = RequestContext.getCurrentContext(); RequestContext ctx = RequestContext.getCurrentContext();
ZuulException exception = findZuulException(ctx.getThrowable());
HttpServletRequest request = ctx.getRequest(); HttpServletRequest request = ctx.getRequest();
int statusCode = (Integer) ctx.get("error.status_code"); request.setAttribute("javax.servlet.error.status_code", exception.nStatusCode);
request.setAttribute("javax.servlet.error.status_code", statusCode);
if (ctx.containsKey("error.exception")) { log.warn("Error during filtering", exception);
Object e = ctx.get("error.exception"); request.setAttribute("javax.servlet.error.exception", exception);
log.warn("Error during filtering", Throwable.class.cast(e));
request.setAttribute("javax.servlet.error.exception", e);
}
if (ctx.containsKey("error.message")) { if (StringUtils.hasText(exception.errorCause)) {
String message = (String) ctx.get("error.message"); request.setAttribute("javax.servlet.error.message", exception.errorCause);
request.setAttribute("javax.servlet.error.message", message);
} }
RequestDispatcher dispatcher = request.getRequestDispatcher( RequestDispatcher dispatcher = request.getRequestDispatcher(
...@@ -91,6 +92,26 @@ public class SendErrorFilter extends ZuulFilter { ...@@ -91,6 +92,26 @@ public class SendErrorFilter extends ZuulFilter {
return null; return null;
} }
ZuulException findZuulException(Throwable throwable) {
if (throwable.getCause() instanceof ZuulRuntimeException) {
// this was a failure initiated by one of the local filters
return (ZuulException) throwable.getCause().getCause();
}
if (throwable.getCause() instanceof ZuulException) {
// wrapped zuul exception
return (ZuulException) throwable.getCause();
}
if (throwable instanceof ZuulException) {
// exception thrown by zuul lifecycle
return (ZuulException) throwable;
}
// fallback, should never get here
return new ZuulException(throwable, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null);
}
public void setErrorPath(String errorPath) { public void setErrorPath(String errorPath) {
this.errorPath = errorPath; this.errorPath = errorPath;
} }
......
...@@ -77,9 +77,11 @@ public class SendResponseFilter extends ZuulFilter { ...@@ -77,9 +77,11 @@ public class SendResponseFilter extends ZuulFilter {
@Override @Override
public boolean shouldFilter() { public boolean shouldFilter() {
return !RequestContext.getCurrentContext().getZuulResponseHeaders().isEmpty() RequestContext context = RequestContext.getCurrentContext();
|| RequestContext.getCurrentContext().getResponseDataStream() != null return context.getThrowable() == null
|| RequestContext.getCurrentContext().getResponseBody() != null; && (!context.getZuulResponseHeaders().isEmpty()
|| context.getResponseDataStream() != null
|| context.getResponseBody() != null);
} }
@Override @Override
......
...@@ -16,29 +16,32 @@ ...@@ -16,29 +16,32 @@
package org.springframework.cloud.netflix.zuul.filters.route; package org.springframework.cloud.netflix.zuul.filters.route;
import lombok.extern.apachecommons.CommonsLog;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer;
import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper;
import org.springframework.cloud.netflix.zuul.util.ZuulRuntimeException;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
import org.springframework.http.client.ClientHttpResponse; import org.springframework.http.client.ClientHttpResponse;
import org.springframework.util.MultiValueMap; import org.springframework.util.MultiValueMap;
import com.netflix.client.ClientException; import com.netflix.client.ClientException;
import com.netflix.hystrix.exception.HystrixRuntimeException; import com.netflix.hystrix.exception.HystrixRuntimeException;
import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.ZuulFilter;
import com.netflix.zuul.context.RequestContext; import com.netflix.zuul.context.RequestContext;
import com.netflix.zuul.exception.ZuulException; import com.netflix.zuul.exception.ZuulException;
import lombok.extern.apachecommons.CommonsLog;
@CommonsLog @CommonsLog
public class RibbonRoutingFilter extends ZuulFilter { public class RibbonRoutingFilter extends ZuulFilter {
private static final String ERROR_STATUS_CODE = "error.status_code";
protected ProxyRequestHelper helper; protected ProxyRequestHelper helper;
protected RibbonCommandFactory<?> ribbonCommandFactory; protected RibbonCommandFactory<?> ribbonCommandFactory;
protected List<RibbonRequestCustomizer> requestCustomizers; protected List<RibbonRequestCustomizer> requestCustomizers;
...@@ -90,16 +93,11 @@ public class RibbonRoutingFilter extends ZuulFilter { ...@@ -90,16 +93,11 @@ public class RibbonRoutingFilter extends ZuulFilter {
return response; return response;
} }
catch (ZuulException ex) { catch (ZuulException ex) {
context.set(ERROR_STATUS_CODE, ex.nStatusCode); throw new ZuulRuntimeException(ex);
context.set("error.message", ex.errorCause);
context.set("error.exception", ex);
} }
catch (Exception ex) { catch (Exception ex) {
context.set("error.status_code", throw new ZuulRuntimeException(ex);
HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
context.set("error.exception", ex);
} }
return null;
} }
protected RibbonCommandContext buildCommandContext(RequestContext context) { protected RibbonCommandContext buildCommandContext(RequestContext context) {
......
...@@ -34,7 +34,6 @@ import javax.net.ssl.SSLContext; ...@@ -34,7 +34,6 @@ import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager; import javax.net.ssl.X509TrustManager;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.http.Header; import org.apache.http.Header;
import org.apache.http.HttpHost; import org.apache.http.HttpHost;
...@@ -69,6 +68,7 @@ import org.apache.http.protocol.HttpContext; ...@@ -69,6 +68,7 @@ import org.apache.http.protocol.HttpContext;
import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper;
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.Host; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.Host;
import org.springframework.cloud.netflix.zuul.util.ZuulRuntimeException;
import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap; import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
...@@ -91,7 +91,6 @@ public class SimpleHostRoutingFilter extends ZuulFilter { ...@@ -91,7 +91,6 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
private static final DynamicIntProperty CONNECTION_TIMEOUT = DynamicPropertyFactory private static final DynamicIntProperty CONNECTION_TIMEOUT = DynamicPropertyFactory
.getInstance() .getInstance()
.getIntProperty(ZuulConstants.ZUUL_HOST_CONNECT_TIMEOUT_MILLIS, 2000); .getIntProperty(ZuulConstants.ZUUL_HOST_CONNECT_TIMEOUT_MILLIS, 2000);
private static final String ERROR_STATUS_CODE = "error.status_code";
private final Timer connectionManagerTimer = new Timer( private final Timer connectionManagerTimer = new Timer(
"SimpleHostRoutingFilter.connectionManagerTimer", true); "SimpleHostRoutingFilter.connectionManagerTimer", true);
...@@ -182,8 +181,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter { ...@@ -182,8 +181,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter {
setResponse(response); setResponse(response);
} }
catch (Exception ex) { catch (Exception ex) {
context.set(ERROR_STATUS_CODE, HttpServletResponse.SC_INTERNAL_SERVER_ERROR); throw new ZuulRuntimeException(ex);
context.set("error.exception", ex);
} }
return null; return null;
} }
......
...@@ -32,7 +32,7 @@ public abstract class AbstractRibbonCommandFactory implements RibbonCommandFacto ...@@ -32,7 +32,7 @@ public abstract class AbstractRibbonCommandFactory implements RibbonCommandFacto
private Map<String, ZuulFallbackProvider> fallbackProviderCache; private Map<String, ZuulFallbackProvider> fallbackProviderCache;
public AbstractRibbonCommandFactory(Set<ZuulFallbackProvider> fallbackProviders){ public AbstractRibbonCommandFactory(Set<ZuulFallbackProvider> fallbackProviders){
this.fallbackProviderCache = new HashMap<String, ZuulFallbackProvider>(); this.fallbackProviderCache = new HashMap<>();
for(ZuulFallbackProvider provider : fallbackProviders) { for(ZuulFallbackProvider provider : fallbackProviders) {
fallbackProviderCache.put(provider.getRoute(), provider); fallbackProviderCache.put(provider.getRoute(), provider);
} }
......
/*
* Copyright 2013-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
package org.springframework.cloud.netflix.zuul.util;
import com.netflix.zuul.exception.ZuulException;
import org.springframework.http.HttpStatus;
/**
* @author Spencer Gibb
*/
public class ZuulRuntimeException extends RuntimeException {
public ZuulRuntimeException(ZuulException cause) {
super(cause);
}
public ZuulRuntimeException(Exception ex) {
this(new ZuulException(ex, HttpStatus.INTERNAL_SERVER_ERROR.value(), null));
}
}
/*
* Copyright 2013-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
package org.springframework.cloud.netflix.zuul.filters.post;
import javax.servlet.http.HttpServletRequest;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.boot.SpringBootConfiguration;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.context.embedded.LocalServerPort;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.client.TestRestTemplate;
import org.springframework.cloud.netflix.ribbon.RibbonClient;
import org.springframework.cloud.netflix.ribbon.StaticServerList;
import org.springframework.cloud.netflix.zuul.EnableZuulProxy;
import org.springframework.context.annotation.Bean;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit4.SpringRunner;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import com.netflix.loadbalancer.Server;
import com.netflix.loadbalancer.ServerList;
import com.netflix.zuul.ZuulFilter;
import com.netflix.zuul.context.RequestContext;
import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
/**
* @author Spencer Gibb
*/
@RunWith(SpringRunner.class)
@SpringBootTest(classes = SendErrorFilterIntegrationTests.Config.class,
properties = "zuul.routes.filtertest:/filtertest/**",
webEnvironment = RANDOM_PORT)
@DirtiesContext
public class SendErrorFilterIntegrationTests {
@LocalServerPort
private int port;
@Before
public void setTestRequestcontext() {
RequestContext context = new RequestContext();
RequestContext.testSetCurrentContext(context);
}
@Test
public void testPreFails() {
String url = "http://localhost:" + port + "/filtertest/get?failpre=true";
ResponseEntity<String> response = new TestRestTemplate().getForEntity(url, String.class);
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
}
@Test
public void testRouteFails() {
String url = "http://localhost:" + port + "/filtertest/get?failroute=true";
ResponseEntity<String> response = new TestRestTemplate().getForEntity(url, String.class);
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
}
@Test
public void testPostFails() {
String url = "http://localhost:" + port + "/filtertest/get?failpost=true";
ResponseEntity<String> response = new TestRestTemplate().getForEntity(url, String.class);
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
}
@SpringBootConfiguration
@EnableAutoConfiguration
@EnableZuulProxy
@RestController
@RibbonClient(name = "filtertest", configuration = RibbonConfig.class)
protected static class Config {
@RequestMapping("/get")
public String get() {
return "Hello";
}
@Bean
public ZuulFilter testPreFilter() {
return new FailureFilter() {
@Override
public String filterType() {
return "pre";
}
};
}
@Bean
public ZuulFilter testRouteFilter() {
return new FailureFilter() {
@Override
public String filterType() {
return "route";
}
};
}
@Bean
public ZuulFilter testPostFilter() {
return new FailureFilter() {
@Override
public String filterType() {
return "post";
}
};
}
}
public static class RibbonConfig {
@LocalServerPort
private int port;
@Bean
public ServerList<Server> ribbonServerList() {
return new StaticServerList<>(new Server("localhost", this.port));
}
}
private abstract static class FailureFilter extends ZuulFilter {
@Override
public int filterOrder() {
return Integer.MIN_VALUE;
}
@Override
public boolean shouldFilter() {
HttpServletRequest request = RequestContext.getCurrentContext().getRequest();
return request.getParameter("fail"+filterType()) != null;
}
@Override
public Object run() {
HttpServletRequest request = RequestContext.getCurrentContext().getRequest();
if (request.getParameter("fail"+filterType()) != null) {
throw new RuntimeException("failing on purpose in " + filterType());
}
return null;
}
}
}
...@@ -18,6 +18,8 @@ package org.springframework.cloud.netflix.zuul.filters.post; ...@@ -18,6 +18,8 @@ package org.springframework.cloud.netflix.zuul.filters.post;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import com.netflix.zuul.exception.ZuulException;
import com.netflix.zuul.monitoring.MonitoringHelper;
import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
...@@ -38,6 +40,7 @@ public class SendErrorFilterTests { ...@@ -38,6 +40,7 @@ public class SendErrorFilterTests {
@Before @Before
public void setTestRequestcontext() { public void setTestRequestcontext() {
MonitoringHelper.initMocks();
RequestContext context = new RequestContext(); RequestContext context = new RequestContext();
RequestContext.testSetCurrentContext(context); RequestContext.testSetCurrentContext(context);
} }
...@@ -58,7 +61,7 @@ public class SendErrorFilterTests { ...@@ -58,7 +61,7 @@ public class SendErrorFilterTests {
RequestContext context = new RequestContext(); RequestContext context = new RequestContext();
context.setRequest(request); context.setRequest(request);
context.setResponse(new MockHttpServletResponse()); context.setResponse(new MockHttpServletResponse());
context.set("error.status_code", HttpStatus.NOT_FOUND.value()); context.setThrowable(new ZuulException(new RuntimeException(), HttpStatus.NOT_FOUND.value(), null));
RequestContext.testSetCurrentContext(context); RequestContext.testSetCurrentContext(context);
SendErrorFilter filter = new SendErrorFilter(); SendErrorFilter filter = new SendErrorFilter();
filter.setErrorPath("/error"); filter.setErrorPath("/error");
......
...@@ -23,15 +23,18 @@ import org.junit.runner.RunWith; ...@@ -23,15 +23,18 @@ import org.junit.runner.RunWith;
import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.cloud.netflix.zuul.filters.route.support.RibbonCommandFallbackTests; import org.springframework.cloud.netflix.zuul.filters.route.support.RibbonCommandFallbackTests;
import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.context.junit4.SpringRunner;
import com.netflix.zuul.context.RequestContext; import com.netflix.zuul.context.RequestContext;
import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
/** /**
* @author Ryan Baxter * @author Ryan Baxter
*/ */
@RunWith(SpringJUnit4ClassRunner.class) @RunWith(SpringRunner.class)
@SpringBootTest(classes = HttpClientRibbonCommandIntegrationTests.TestConfig.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, value = { @SpringBootTest(classes = HttpClientRibbonCommandIntegrationTests.TestConfig.class, webEnvironment = RANDOM_PORT,
properties = {
"zuul.routes.simple: /simple/**", "zuul.routes.another: /another/twolevel/**", "zuul.routes.simple: /simple/**", "zuul.routes.another: /another/twolevel/**",
"ribbon.ReadTimeout: 1"}) "ribbon.ReadTimeout: 1"})
@DirtiesContext @DirtiesContext
......
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
package org.springframework.cloud.netflix.zuul.filters.route.support; package org.springframework.cloud.netflix.zuul.filters.route.support;
import org.junit.Test; import org.junit.Test;
import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.context.embedded.LocalServerPort;
import org.springframework.boot.test.web.client.TestRestTemplate; import org.springframework.boot.test.web.client.TestRestTemplate;
import org.springframework.http.HttpEntity; import org.springframework.http.HttpEntity;
import org.springframework.http.HttpMethod; import org.springframework.http.HttpMethod;
...@@ -33,7 +33,7 @@ import static org.junit.Assert.assertEquals; ...@@ -33,7 +33,7 @@ import static org.junit.Assert.assertEquals;
*/ */
public abstract class RibbonCommandFallbackTests { public abstract class RibbonCommandFallbackTests {
@Value("${local.server.port}") @LocalServerPort
protected int port; protected int port;
@Test @Test
...@@ -52,7 +52,6 @@ public abstract class RibbonCommandFallbackTests { ...@@ -52,7 +52,6 @@ public abstract class RibbonCommandFallbackTests {
ResponseEntity<String> result = new TestRestTemplate().exchange( ResponseEntity<String> result = new TestRestTemplate().exchange(
"http://localhost:" + this.port + uri, HttpMethod.GET, "http://localhost:" + this.port + uri, HttpMethod.GET,
new HttpEntity<>((Void) null), String.class); new HttpEntity<>((Void) null), String.class);
System.out.println("no fallback body: " + result.getBody());
assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, result.getStatusCode()); assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, result.getStatusCode());
} }
} }
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