From 99c2fa699e0d2a11670c756ad744df5fc5220390 Mon Sep 17 00:00:00 2001 From: Nikolay Rybak Date: Mon, 30 Oct 2017 16:24:14 +0200 Subject: [PATCH 1/2] Handle AbstractReactiveHealthIndicator.doHealthCheck exception Exceptions inside AbstractReactiveHealthIndicator.doHealthCheck() method, outside of Mono pipeline, could fail whole endpoint response instead of returning `DOWN` status from indicator. See gh-10822 --- .../health/AbstractReactiveHealthIndicator.java | 13 +++++++++++-- .../redis/RedisReactiveHealthIndicatorTests.java | 15 ++++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java index 5370f8964f..0e40224fa0 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java @@ -29,8 +29,17 @@ public abstract class AbstractReactiveHealthIndicator implements ReactiveHealthI @Override public final Mono health() { - return doHealthCheck(new Health.Builder()) - .onErrorResume((ex) -> Mono.just(new Health.Builder().down(ex).build())); + try { + return doHealthCheck(new Health.Builder()) + .onErrorResume(this::handleFailure); + } + catch (Throwable ex) { + return handleFailure(ex); + } + } + + private Mono handleFailure(Throwable ex) { + return Mono.just(new Health.Builder().down(ex).build()); } /** diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java index 6d90134599..66beab1a2f 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java @@ -18,6 +18,7 @@ package org.springframework.boot.actuate.redis; import java.util.Properties; +import io.lettuce.core.RedisConnectionException; import org.junit.Test; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -61,7 +62,7 @@ public class RedisReactiveHealthIndicatorTests { } @Test - public void redisIsDown() throws Exception { + public void redisCommandIsDown() throws Exception { ReactiveServerCommands commands = mock(ReactiveServerCommands.class); given(commands.info()).willReturn( Mono.error(new RedisConnectionFailureException("Connection failed"))); @@ -75,6 +76,18 @@ public class RedisReactiveHealthIndicatorTests { verify(redisConnection).close(); } + @Test + public void redisConnectionIsDown() throws Exception { + ReactiveRedisConnectionFactory redisConnectionFactory = mock(ReactiveRedisConnectionFactory.class); + given(redisConnectionFactory.getReactiveConnection()).willThrow( + new RedisConnectionException("Unable to connect to localhost:6379")); + RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory); + Mono health = healthIndicator.health(); + StepVerifier.create(health) + .consumeNextWith((h) -> assertThat(h.getStatus()).isEqualTo(Status.DOWN)) + .verifyComplete(); + } + private RedisReactiveHealthIndicator createHealthIndicator( ReactiveRedisConnection redisConnection, ReactiveServerCommands serverCommands) { From 350377c3f2740fcae72268f447322bb0180cbfe7 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 30 Oct 2017 17:03:49 +0100 Subject: [PATCH 2/2] Polish "Handle AbstractReactiveHealthIndicator.doHealthCheck exception" Closes gh-10822 --- .../health/AbstractReactiveHealthIndicator.java | 3 ++- .../redis/RedisReactiveHealthIndicatorTests.java | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java index 0e40224fa0..14ba42c9f4 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/AbstractReactiveHealthIndicator.java @@ -23,6 +23,7 @@ import reactor.core.publisher.Mono; * {@link Health} instance and error handling. * * @author Stephane Nicoll + * @author Nikolay Rybak * @since 2.0.0 */ public abstract class AbstractReactiveHealthIndicator implements ReactiveHealthIndicator { @@ -33,7 +34,7 @@ public abstract class AbstractReactiveHealthIndicator implements ReactiveHealthI return doHealthCheck(new Health.Builder()) .onErrorResume(this::handleFailure); } - catch (Throwable ex) { + catch (Exception ex) { return handleFailure(ex); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java index 66beab1a2f..e93bcb5c2c 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/redis/RedisReactiveHealthIndicatorTests.java @@ -40,11 +40,12 @@ import static org.mockito.Mockito.verify; * * @author Stephane Nicoll * @author Mark Paluch + * @author Nikolay Rybak */ public class RedisReactiveHealthIndicatorTests { @Test - public void redisIsUp() throws Exception { + public void redisIsUp() { Properties info = new Properties(); info.put("redis_version", "2.8.9"); ReactiveRedisConnection redisConnection = mock(ReactiveRedisConnection.class); @@ -62,7 +63,7 @@ public class RedisReactiveHealthIndicatorTests { } @Test - public void redisCommandIsDown() throws Exception { + public void redisCommandIsDown() { ReactiveServerCommands commands = mock(ReactiveServerCommands.class); given(commands.info()).willReturn( Mono.error(new RedisConnectionFailureException("Connection failed"))); @@ -77,11 +78,13 @@ public class RedisReactiveHealthIndicatorTests { } @Test - public void redisConnectionIsDown() throws Exception { - ReactiveRedisConnectionFactory redisConnectionFactory = mock(ReactiveRedisConnectionFactory.class); + public void redisConnectionIsDown() { + ReactiveRedisConnectionFactory redisConnectionFactory = mock( + ReactiveRedisConnectionFactory.class); given(redisConnectionFactory.getReactiveConnection()).willThrow( new RedisConnectionException("Unable to connect to localhost:6379")); - RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator(redisConnectionFactory); + RedisReactiveHealthIndicator healthIndicator = new RedisReactiveHealthIndicator( + redisConnectionFactory); Mono health = healthIndicator.health(); StepVerifier.create(health) .consumeNextWith((h) -> assertThat(h.getStatus()).isEqualTo(Status.DOWN))