fix HealthIndicatorProcessor.doHealthCheck() bug when exception occur (…

…#1046)
sofastack · Dec 28, 2022 · 97d4364 · 97d4364
1 parent d7e6a1d
commit 97d4364
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 15 deletions.
diff --git a/...thcheck-sofa-boot/src/main/java/com/alipay/sofa/healthcheck/HealthIndicatorProcessor.java b/...thcheck-sofa-boot/src/main/java/com/alipay/sofa/healthcheck/HealthIndicatorProcessor.java
@@ -158,7 +158,7 @@ public boolean readinessHealthCheck(Map<String, Health> healthMap) {
         logger.info("Begin SOFABoot HealthIndicator readiness check.");
         String checkComponentNames = healthIndicators.keySet().stream()
                 .collect(Collectors.joining(","));
-        logger.info("SOFABoot HealthChecker readiness check {} item: {}.",
+        logger.info("SOFABoot HealthIndicator readiness check {} item: {}.",
                 healthIndicators.size(), checkComponentNames);
         boolean result;
         if (healthCheckProperties.isHealthCheckParallelEnable()) {
@@ -225,19 +225,21 @@ public boolean doHealthCheck(String beanId, HealthIndicator healthIndicator,
                         ErrorCode.convert("01-21001",
                         beanId, status, objectMapper.writeValueAsString(health.getDetails())));
             }
-            healthMap.put(getKey(beanId), health);
         } catch (TimeoutException e) {
             result = false;
             logger.error(
                     "HealthIndicator[{}] readiness check fail; the status is: {}; the detail is: timeout, the timeout value is: {}ms.",
                     beanId, Status.UNKNOWN, timeout);
+            health = new Health.Builder().withException(e).status(Status.UNKNOWN).build();
         } catch (Exception e) {
             result = false;
             logger.error(
                     ErrorCode.convert("01-21002",
                             healthIndicator.getClass()),
                     e);
+            health = new Health.Builder().withException(e).status(Status.DOWN).build();
         }
+        healthMap.put(getKey(beanId), health);
 
         return result;
     }

diff --git a/...oot/src/test/java/com/alipay/sofa/healthcheck/test/HealthIndicatorCheckProcessorTest.java b/...oot/src/test/java/com/alipay/sofa/healthcheck/test/HealthIndicatorCheckProcessorTest.java
@@ -16,12 +16,14 @@
  */
 package com.alipay.sofa.healthcheck.test;
 
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.Map;
-
+import com.alipay.sofa.healthcheck.AfterReadinessCheckCallbackProcessor;
 import com.alipay.sofa.healthcheck.HealthCheckProperties;
+import com.alipay.sofa.healthcheck.HealthCheckerProcessor;
+import com.alipay.sofa.healthcheck.HealthIndicatorProcessor;
+import com.alipay.sofa.healthcheck.ReadinessCheckListener;
 import com.alipay.sofa.healthcheck.core.HealthCheckExecutor;
+import com.alipay.sofa.healthcheck.test.bean.DiskHealthIndicator;
+import com.alipay.sofa.healthcheck.test.bean.TimeoutHealthIndicator;
 import com.alipay.sofa.runtime.configure.SofaRuntimeConfigurationProperties;
 import org.junit.Assert;
 import org.junit.Test;
@@ -34,14 +36,12 @@
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-
-import com.alipay.sofa.healthcheck.AfterReadinessCheckCallbackProcessor;
-import com.alipay.sofa.healthcheck.HealthCheckerProcessor;
-import com.alipay.sofa.healthcheck.HealthIndicatorProcessor;
-import com.alipay.sofa.healthcheck.ReadinessCheckListener;
-import com.alipay.sofa.healthcheck.test.bean.DiskHealthIndicator;
 import org.springframework.core.env.Environment;
 
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
 /**
  * @author liangen
  * @author qilong.zql
@@ -60,6 +60,11 @@ public DiskHealthIndicator diskHealthIndicator(@Value("${disk-health-indicator.h
             return new DiskHealthIndicator(health);
         }
 
+        @Bean
+        public TimeoutHealthIndicator timeoutHealthIndicator(@Value("${timeout-health-indicator.health}") boolean health) {
+            return new TimeoutHealthIndicator(health);
+        }
+
         @Bean
         public AfterReadinessCheckCallbackProcessor afterReadinessCheckCallbackProcessor() {
             return new AfterReadinessCheckCallbackProcessor();
@@ -104,7 +109,7 @@ public void testCheckIndicatorPassed() {
         boolean result = healthIndicatorProcessor.readinessHealthCheck(hashMap);
         Health diskHealth = hashMap.get("disk");
         Assert.assertTrue(result);
-        Assert.assertEquals(1, hashMap.size());
+        Assert.assertEquals(2, hashMap.size());
         Assert.assertNotNull(diskHealth);
         Assert.assertEquals(diskHealth.getStatus(), Status.UP);
         Assert.assertEquals("hard disk is ok", diskHealth.getDetails().get("disk"));
@@ -119,15 +124,27 @@ public void testCheckIndicatorFailed() {
         boolean result = healthIndicatorProcessor.readinessHealthCheck(hashMap);
         Health diskHealth = hashMap.get("disk");
         Assert.assertFalse(result);
-        Assert.assertEquals(1, hashMap.size());
+        Assert.assertEquals(2, hashMap.size());
         Assert.assertNotNull(diskHealth);
-        Assert.assertEquals(diskHealth.getStatus(), Status.DOWN);
+        Assert.assertEquals(Status.DOWN, diskHealth.getStatus());
         Assert.assertEquals("hard disk is bad", diskHealth.getDetails().get("disk"));
+
+        Health timeoutHealth = hashMap.get("timeout");
+        Assert.assertNotNull(timeoutHealth);
+        Assert.assertEquals(Status.UNKNOWN, timeoutHealth.getStatus());
+        Assert.assertEquals("java.util.concurrent.TimeoutException: null", timeoutHealth
+            .getDetails().get("error"));
     }
 
     private void initApplicationContext(boolean health) {
         Map<String, Object> properties = new LinkedHashMap<>();
         properties.put("disk-health-indicator.health", health);
+        properties.put("timeout-health-indicator.health", true);
+        if (!health) {
+            properties.put("com.alipay.sofa.healthcheck.indicator.timeout.timeoutHealthIndicator",
+                "1");
+        }
+        properties.put("com.alipay.sofa.healthcheck.skip.indicator", "true");
         properties.put("spring.application.name", "HealthIndicatorCheckProcessorTest");
         SpringApplication springApplication = new SpringApplication(
             HealthIndicatorConfiguration.class);

diff --git a/...sofa-boot/src/test/java/com/alipay/sofa/healthcheck/test/bean/TimeoutHealthIndicator.java b/...sofa-boot/src/test/java/com/alipay/sofa/healthcheck/test/bean/TimeoutHealthIndicator.java
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 com.alipay.sofa.healthcheck.test.bean;
+
+import org.springframework.boot.actuate.health.Health;
+import org.springframework.boot.actuate.health.HealthIndicator;
+
+/**
+ * TimeOutHealthIndicator
+ *
+ * @author xunfang
+ * @version TimeOutHealthIndicator.java, v 0.1 2022/12/27
+ */
+public class TimeoutHealthIndicator implements HealthIndicator {
+    private boolean health;
+
+    public TimeoutHealthIndicator(boolean health) {
+        this.health = health;
+    }
+
+    @Override
+    public Health health() {
+        try {
+            Thread.sleep(100);
+        } catch (InterruptedException e) {
+            e.printStackTrace();
+        }
+        if (health) {
+            return Health.up().withDetail("timeout", "timeoutHealthIndicator is ok").build();
+        } else {
+            return Health.down().withDetail("timeout", "timeoutHealthIndicator is bad").build();
+        }
+    }
+}