Merge branch 'master' of github.com:spring-cloud/spring-cloud-netflix

spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/post/SendErrorFilter.java

@@ -18,19 +18,19 @@ package org.springframework.cloud.netflix.zuul.filters.post;
 
 import javax.servlet.RequestDispatcher;
 import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
 
+import com.netflix.zuul.ZuulFilter;
+import com.netflix.zuul.context.RequestContext;
+import com.netflix.zuul.exception.ZuulException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.cloud.netflix.zuul.util.ZuulRuntimeException;
+import org.springframework.http.HttpStatus;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.util.StringUtils;
 
-import com.netflix.zuul.ZuulFilter;
-import com.netflix.zuul.context.RequestContext;
-import com.netflix.zuul.exception.ZuulException;
-
 import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.ERROR_TYPE;
 import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.SEND_ERROR_FILTER_ORDER;
 
@@ -70,16 +70,16 @@ public class SendErrorFilter extends ZuulFilter {
 	public Object run() {
 		try {
 			RequestContext ctx = RequestContext.getCurrentContext();
-			ZuulException exception = findZuulException(ctx.getThrowable());
+			ExceptionHolder exception = findZuulException(ctx.getThrowable());
 			HttpServletRequest request = ctx.getRequest();
 
-			request.setAttribute("javax.servlet.error.status_code", exception.nStatusCode);
+			request.setAttribute("javax.servlet.error.status_code", exception.getStatusCode());
 
-			log.warn("Error during filtering", exception);
-			request.setAttribute("javax.servlet.error.exception", exception);
+			log.warn("Error during filtering", exception.getThrowable());
+			request.setAttribute("javax.servlet.error.exception", exception.getThrowable());
 
-			if (StringUtils.hasText(exception.errorCause)) {
-				request.setAttribute("javax.servlet.error.message", exception.errorCause);
+			if (StringUtils.hasText(exception.getErrorCause())) {
+				request.setAttribute("javax.servlet.error.message", exception.getErrorCause());
 			}
 
 			RequestDispatcher dispatcher = request.getRequestDispatcher(
@@ -87,7 +87,7 @@ public class SendErrorFilter extends ZuulFilter {
 			if (dispatcher != null) {
 				ctx.set(SEND_ERROR_FILTER_RAN, true);
 				if (!ctx.getResponse().isCommitted()) {
-					ctx.setResponseStatusCode(exception.nStatusCode);
+					ctx.setResponseStatusCode(exception.getStatusCode());
 					dispatcher.forward(request, ctx.getResponse());
 				}
 			}
@@ -98,24 +98,72 @@ public class SendErrorFilter extends ZuulFilter {
 		return null;
 	}
 
-	ZuulException findZuulException(Throwable throwable) {
+	protected ExceptionHolder findZuulException(Throwable throwable) {
 		if (throwable.getCause() instanceof ZuulRuntimeException) {
 			// this was a failure initiated by one of the local filters
-			return (ZuulException) throwable.getCause().getCause();
+			return new ZuulExceptionHolder((ZuulException) throwable.getCause().getCause());
 		}
 
 		if (throwable.getCause() instanceof ZuulException) {
 			// wrapped zuul exception
-			return (ZuulException) throwable.getCause();
+			return  new ZuulExceptionHolder((ZuulException) throwable.getCause());
 		}
 
 		if (throwable instanceof ZuulException) {
 			// exception thrown by zuul lifecycle
-			return (ZuulException) throwable;
+			return new ZuulExceptionHolder((ZuulException) throwable);
+		}
+
+		// fallback
+		return new DefaultExceptionHolder(throwable);
 	}
 
-		// fallback, should never get here
-		return new ZuulException(throwable, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null);
+	protected interface ExceptionHolder {
+		Throwable getThrowable();
+
+	    default int getStatusCode() {
+	    	return HttpStatus.INTERNAL_SERVER_ERROR.value();
+		}
+
+	    default String getErrorCause() {
+	    	return null;
+		}
+	}
+
+	protected static class DefaultExceptionHolder implements ExceptionHolder {
+		private final Throwable throwable;
+
+		public DefaultExceptionHolder(Throwable throwable) {
+			this.throwable = throwable;
+		}
+
+		@Override
+		public Throwable getThrowable() {
+			return this.throwable;
+		}
+	}
+
+	protected static class ZuulExceptionHolder implements ExceptionHolder {
+		private final ZuulException exception;
+
+		public ZuulExceptionHolder(ZuulException exception) {
+			this.exception = exception;
+		}
+
+		@Override
+		public Throwable getThrowable() {
+			return this.exception;
+		}
+
+		@Override
+		public int getStatusCode() {
+			return this.exception.nStatusCode;
+		}
+
+		@Override
+		public String getErrorCause() {
+			return this.exception.errorCause;
+		}
 	}
 
 	public void setErrorPath(String errorPath) {

spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/util/ZuulRuntimeException.java

@@ -18,7 +18,6 @@
 package org.springframework.cloud.netflix.zuul.util;
 
 import com.netflix.zuul.exception.ZuulException;
-import org.springframework.http.HttpStatus;
 
 /**
  * @author Spencer Gibb
@@ -30,6 +29,6 @@ public class ZuulRuntimeException extends RuntimeException {
 	}
 
 	public ZuulRuntimeException(Exception ex) {
-		this(new ZuulException(ex, HttpStatus.INTERNAL_SERVER_ERROR.value(), null));
+		super(ex);
 	}
 }

spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/post/SendErrorFilterIntegrationTests.java

@@ -23,17 +23,21 @@ import com.netflix.loadbalancer.Server;
 import com.netflix.loadbalancer.ServerList;
 import com.netflix.zuul.ZuulFilter;
 import com.netflix.zuul.context.RequestContext;
-
+import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.MockClock;
+import io.micrometer.core.instrument.simple.SimpleConfig;
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.SpringBootConfiguration;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
-import org.springframework.boot.web.server.LocalServerPort;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.web.client.TestRestTemplate;
+import org.springframework.boot.web.server.LocalServerPort;
 import org.springframework.cloud.netflix.ribbon.RibbonClient;
 import org.springframework.cloud.netflix.ribbon.StaticServerList;
 import org.springframework.cloud.netflix.zuul.EnableZuulProxy;
@@ -58,6 +62,8 @@ import static org.springframework.cloud.netflix.zuul.filters.support.FilterConst
 @SpringBootTest(classes = SendErrorFilterIntegrationTests.Config.class, properties = "zuul.routes.filtertest:/filtertest/**", webEnvironment = RANDOM_PORT)
 @DirtiesContext
 public class SendErrorFilterIntegrationTests {
+	@Autowired
+	private MeterRegistry meterRegistry;
 
 	@LocalServerPort
 	private int port;
@@ -79,6 +85,15 @@ public class SendErrorFilterIntegrationTests {
 		ResponseEntity<String> response = new TestRestTemplate().getForEntity(url,
 				String.class);
 		assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
+
+		assertMetrics("pre");
+	}
+
+	private void assertMetrics(String filterType) {
+		Double count = meterRegistry.counter("ZUUL::EXCEPTION:"+ filterType +"::500").count();
+		assertThat(count.longValue()).isEqualTo(1L);
+		count = meterRegistry.counter("ZUUL::EXCEPTION:null:500").count();
+		assertThat(count.longValue()).isEqualTo(0L);
 	}
 
 	@Test
@@ -87,6 +102,8 @@ public class SendErrorFilterIntegrationTests {
 		ResponseEntity<String> response = new TestRestTemplate().getForEntity(url,
 				String.class);
 		assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
+
+		assertMetrics("route");
 	}
 
 	@Test
@@ -95,6 +112,8 @@ public class SendErrorFilterIntegrationTests {
 		ResponseEntity<String> response = new TestRestTemplate().getForEntity(url,
 				String.class);
 		assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
+
+		assertMetrics("post");
 	}
 
 	@SpringBootConfiguration
@@ -138,6 +157,11 @@ public class SendErrorFilterIntegrationTests {
 				}
 			};
 		}
+
+		@Bean
+		public MeterRegistry meterRegistry() {
+			return new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock());
+		}
 	}
 
 	public static class RibbonConfig {

spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java

@@ -17,14 +17,6 @@
 
 package org.springframework.cloud.netflix.zuul.filters.route.restclient;
 
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -34,9 +26,13 @@ import java.util.UUID;
 
 import javax.servlet.http.HttpServletRequest;
 
-import org.junit.Ignore;
+import com.netflix.client.ClientException;
+import com.netflix.loadbalancer.Server;
+import com.netflix.loadbalancer.ServerList;
+import com.netflix.niws.client.http.RestClient;
 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;
@@ -44,21 +40,23 @@ import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
 import org.springframework.boot.test.web.client.TestRestTemplate;
 import org.springframework.boot.web.servlet.error.ErrorAttributes;
+import org.springframework.boot.web.servlet.filter.ApplicationContextHeaderFilter;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 import org.springframework.cloud.netflix.ribbon.RibbonClient;
 import org.springframework.cloud.netflix.ribbon.RibbonClients;
 import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
 import org.springframework.cloud.netflix.ribbon.StaticServerList;
+import org.springframework.cloud.netflix.ribbon.support.RibbonCommandContext;
 import org.springframework.cloud.netflix.zuul.EnableZuulProxy;
 import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
 import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientRouteLocator;
 import org.springframework.cloud.netflix.zuul.filters.route.FallbackProvider;
 import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommand;
 import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommandFactory;
-import org.springframework.cloud.netflix.ribbon.support.RibbonCommandContext;
 import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory;
 import org.springframework.cloud.netflix.zuul.filters.route.support.NoEncodingFormHttpMessageConverter;
 import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase;
+import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.http.HttpEntity;
@@ -83,10 +81,13 @@ import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
 
-import com.netflix.client.ClientException;
-import com.netflix.loadbalancer.Server;
-import com.netflix.loadbalancer.ServerList;
-import com.netflix.niws.client.http.RestClient;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 @RunWith(SpringJUnit4ClassRunner.class)
 @SpringBootTest(classes = RestClientRibbonCommandIntegrationTests.TestConfig.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = {
@@ -148,7 +149,6 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase {
 	}
 
 	@Test
-	@Ignore //FIXME: does spring 5.0 no longer send the X-Application-Context header?
 	public void simpleHostRouteDefaultIgnoredHeader() {
 		this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/");
 		this.endpoint.reset();
@@ -158,7 +158,7 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase {
 		assertEquals(HttpStatus.OK, result.getStatusCode());
 		List<String> headers = result.getHeaders().get("X-Application-Context");
 		assertNotNull("header was null", headers);
-		assertEquals("[testclient:0]", headers.toString());
+		assertEquals("[application-1]", headers.toString());
 	}
 
 	@Test
@@ -353,6 +353,12 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase {
 			return new MyErrorController(errorAttributes);
 		}
 
+		@Bean
+		public ApplicationContextHeaderFilter applicationContextIdFilter(
+				ApplicationContext context) {
+			return new ApplicationContextHeaderFilter(context);
+		}
+
 		public static void main(String[] args) {
 			SpringApplication.run(TestConfig.class, args);
 		}

spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/metrics/ZuulMetricsApplicationTests.java

@@ -23,6 +23,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.cloud.netflix.zuul.EnableZuulServer;
+import org.springframework.cloud.netflix.zuul.util.ZuulRuntimeException;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.test.annotation.DirtiesContext;
@@ -68,10 +69,15 @@ public class ZuulMetricsApplicationTests {
 	@Test
 	@SuppressWarnings("all")
 	public void shouldIncrementCounters() throws Exception {
+		new ZuulRuntimeException(new Exception());
+
+		Double count = meterRegistry.counter("ZUUL::EXCEPTION:null:500").count();
+		assertEquals(count.longValue(), 0L);
+
 		new ZuulException("any", 500, "cause");
 		new ZuulException("any", 500, "cause");
 
-		Double count = meterRegistry.counter("ZUUL::EXCEPTION:cause:500").count();
+		count = meterRegistry.counter("ZUUL::EXCEPTION:cause:500").count();
 		assertEquals(count.longValue(), 2L);
 
 		new ZuulException("any", 404, "cause2");