From c65573ddb570e515c0f6f93d9a866e617d2853b9 Mon Sep 17 00:00:00 2001
From: Lomesh Patel <453290+lomeshpatel@users.noreply.github.com>
Date: Thu, 13 Dec 2018 13:20:39 -0500
Subject: [PATCH] Log Exception from ConsulHealthIndicator (#454)

- Removed `try...catch` block from `ConsulHealthIndicator` to let
`AbstractHealthIndicator` log and handle the `Exception` appropriately
- Added tests for UP and DOWN status

Fixes gh-447
---
 .../cloud/consul/ConsulHealthIndicator.java   | 18 +++-------
 .../consul/ConsulHealthIndicatorDownTest.java | 35 +++++++++++++++++++
 .../consul/ConsulHealthIndicatorUpTest.java   | 35 +++++++++++++++++++
 3 files changed, 75 insertions(+), 13 deletions(-)
 create mode 100644 spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorDownTest.java
 create mode 100644 spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorUpTest.java

diff --git a/spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulHealthIndicator.java b/spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulHealthIndicator.java
index 476bbd07..36fa103c 100644
--- a/spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulHealthIndicator.java
+++ b/spring-cloud-consul-core/src/main/java/org/springframework/cloud/consul/ConsulHealthIndicator.java
@@ -25,8 +25,6 @@ import org.springframework.boot.actuate.health.Health;
 import com.ecwid.consul.v1.ConsulClient;
 import com.ecwid.consul.v1.QueryParams;
 import com.ecwid.consul.v1.Response;
-import com.ecwid.consul.v1.agent.model.Self;
-import com.ecwid.consul.v1.agent.model.Self.Config;
 
 /**
  * @author Spencer Gibb
@@ -41,16 +39,10 @@ public class ConsulHealthIndicator extends AbstractHealthIndicator {
 
 	@Override
 	protected void doHealthCheck(Health.Builder builder) throws Exception {
-		try {  
-			Response<String> leaderStatus = consul.getStatusLeader();
-			Response<Map<String, List<String>>> services = consul
-					.getCatalogServices(QueryParams.DEFAULT);
-			builder.up()
-                                       .withDetail("leader", leaderStatus.getValue())
-					.withDetail("services", services.getValue());
-		}
-		catch (Exception e) {
-			builder.down(e);
-		}
+		final Response<String> leaderStatus = consul.getStatusLeader();
+		final Response<Map<String, List<String>>> services = consul
+				.getCatalogServices(QueryParams.DEFAULT);
+		builder.up().withDetail("leader", leaderStatus.getValue()).withDetail("services",
+				services.getValue());
 	}
 }
diff --git a/spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorDownTest.java b/spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorDownTest.java
new file mode 100644
index 00000000..969251fe
--- /dev/null
+++ b/spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorDownTest.java
@@ -0,0 +1,35 @@
+package org.springframework.cloud.consul;
+
+import static org.junit.Assert.assertEquals;
+
+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.actuate.health.HealthEndpoint;
+import org.springframework.boot.actuate.health.Status;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.junit4.SpringRunner;
+
+/**
+ * @author Lomesh Patel (lomeshpatel)
+ */
+@RunWith(SpringRunner.class)
+@SpringBootTest(properties = "spring.cloud.consul.host=invalidhost")
+public class ConsulHealthIndicatorDownTest {
+
+	@Autowired
+	private HealthEndpoint healthEndpoint;
+
+	@Test
+	public void doHealthCheck() {
+		assertEquals("health status was not DOWN", Status.DOWN,
+				healthEndpoint.health().getStatus());
+	}
+
+	@EnableAutoConfiguration
+	@SpringBootConfiguration
+	protected static class TestConfig {
+	}
+}
\ No newline at end of file
diff --git a/spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorUpTest.java b/spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorUpTest.java
new file mode 100644
index 00000000..503422cf
--- /dev/null
+++ b/spring-cloud-consul-core/src/test/java/org/springframework/cloud/consul/ConsulHealthIndicatorUpTest.java
@@ -0,0 +1,35 @@
+package org.springframework.cloud.consul;
+
+import static org.junit.Assert.assertEquals;
+
+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.actuate.health.HealthEndpoint;
+import org.springframework.boot.actuate.health.Status;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.context.junit4.SpringRunner;
+
+/**
+ * @author Lomesh Patel (lomeshpatel)
+ */
+@RunWith(SpringRunner.class)
+@SpringBootTest
+public class ConsulHealthIndicatorUpTest {
+
+	@Autowired
+	private HealthEndpoint healthEndpoint;
+
+	@Test
+	public void doHealthCheck() {
+		assertEquals("health status was not UP", Status.UP,
+				healthEndpoint.health().getStatus());
+	}
+
+	@EnableAutoConfiguration
+	@SpringBootConfiguration
+	protected static class TestConfig {
+	}
+}
\ No newline at end of file