From 8a28b712e2265d929d22aea4527c88cdf5ada268 Mon Sep 17 00:00:00 2001 From: Oliver Marienfeld Date: Tue, 12 Oct 2021 20:17:22 +0200 Subject: [PATCH] Make UniRetry & MultiRetry propagate the original Exception (via ExponentialBackoff) #724 --- .../src/test/java/guides/RetryTest.java | 5 +++- .../mutiny/helpers/ExponentialBackoff.java | 14 ++++----- .../mutiny/groups/UniOnFailureRetryTest.java | 15 +++++----- .../MultiOnFailureRetryWhenTest.java | 29 ++++++++++++++++--- 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/documentation/src/test/java/guides/RetryTest.java b/documentation/src/test/java/guides/RetryTest.java index 73974a980..9a268cb92 100644 --- a/documentation/src/test/java/guides/RetryTest.java +++ b/documentation/src/test/java/guides/RetryTest.java @@ -38,7 +38,10 @@ public void testRetryWithBackoff() { .withBackOff(Duration.ofMillis(100), Duration.ofSeconds(1)) .atMost(3); // end::retry-backoff[] - assertThatThrownBy(() -> u.await().indefinitely()).hasMessageContaining("3/3"); + assertThatThrownBy(() -> u.await().indefinitely()) + .getCause() // Expected exception is wrapped in a java.util.concurrent.CompletionException + .hasMessageContaining("boom") + .hasSuppressedException(new IllegalStateException("Retries exhausted: 3/3")); } @Test diff --git a/implementation/src/main/java/io/smallrye/mutiny/helpers/ExponentialBackoff.java b/implementation/src/main/java/io/smallrye/mutiny/helpers/ExponentialBackoff.java index bcef2941f..440af4359 100644 --- a/implementation/src/main/java/io/smallrye/mutiny/helpers/ExponentialBackoff.java +++ b/implementation/src/main/java/io/smallrye/mutiny/helpers/ExponentialBackoff.java @@ -41,14 +41,14 @@ public static Function, Publisher> randomExponentialBacko .onItem().transformToUni(failure -> { int iteration = index.getAndIncrement(); if (iteration >= numRetries) { - return Uni.createFrom().failure( - new IllegalStateException("Retries exhausted: " + iteration + "/" + numRetries, - failure)); + failure.addSuppressed( + new IllegalStateException("Retries exhausted: " + iteration + "/" + numRetries, failure)); + return Uni.createFrom().failure(failure); + } else { + Duration delay = getNextDelay(firstBackoff, maxBackoff, jitterFactor, iteration); + return Uni.createFrom().item((long) iteration).onItem().delayIt() + .onExecutor(executor).by(delay); } - - Duration delay = getNextDelay(firstBackoff, maxBackoff, jitterFactor, iteration); - return Uni.createFrom().item((long) iteration).onItem().delayIt() - .onExecutor(executor).by(delay); }).concatenate(); } diff --git a/implementation/src/test/java/io/smallrye/mutiny/groups/UniOnFailureRetryTest.java b/implementation/src/test/java/io/smallrye/mutiny/groups/UniOnFailureRetryTest.java index b718b3244..285d5969f 100644 --- a/implementation/src/test/java/io/smallrye/mutiny/groups/UniOnFailureRetryTest.java +++ b/implementation/src/test/java/io/smallrye/mutiny/groups/UniOnFailureRetryTest.java @@ -1,6 +1,7 @@ package io.smallrye.mutiny.groups; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -172,24 +173,22 @@ public void testExpireAtRetryWithBackOff() { @Test public void testRetryWithBackOffReachingMaxAttempt() { - assertThrows(IllegalStateException.class, () -> { + assertThatThrownBy(() -> { AtomicInteger count = new AtomicInteger(); - Uni.createFrom(). emitter(e -> { - e.fail(new Exception("boom " + count.getAndIncrement())); - }) + Uni.createFrom(). emitter(e -> e.fail(new Exception("boom " + count.getAndIncrement()))) .onFailure().retry().withBackOff(Duration.ofMillis(10), Duration.ofSeconds(20)).withJitter(1.0) .atMost(4) .await().atMost(Duration.ofSeconds(5)); - }); + }).getCause() // Expected exception is wrapped in a java.util.concurrent.CompletionException + .hasMessageContaining("boom") + .hasSuppressedException(new IllegalStateException("Retries exhausted: 4/4")); } @Test public void testRetryWithBackOffReachingExpiresIn() { assertThrows(IllegalStateException.class, () -> { AtomicInteger count = new AtomicInteger(); - Uni.createFrom(). emitter(e -> { - e.fail(new Exception("boom " + count.getAndIncrement())); - }) + Uni.createFrom(). emitter(e -> e.fail(new Exception("boom " + count.getAndIncrement()))) .onFailure().retry().withBackOff(Duration.ofMillis(10), Duration.ofSeconds(20)).withJitter(1.0) .expireIn(90L) .await().atMost(Duration.ofSeconds(5)); diff --git a/implementation/src/test/java/io/smallrye/mutiny/operators/MultiOnFailureRetryWhenTest.java b/implementation/src/test/java/io/smallrye/mutiny/operators/MultiOnFailureRetryWhenTest.java index 0f8b72d64..df06f4f0c 100644 --- a/implementation/src/test/java/io/smallrye/mutiny/operators/MultiOnFailureRetryWhenTest.java +++ b/implementation/src/test/java/io/smallrye/mutiny/operators/MultiOnFailureRetryWhenTest.java @@ -299,7 +299,7 @@ public void testManualExponentialRetry() { .subscribe().withSubscriber(AssertSubscriber.create(100)); subscriber - .await(Duration.ofSeconds(10)) + .awaitCompletion(Duration.ofSeconds(10)) .assertItems("hey") .assertCompleted(); } @@ -319,7 +319,14 @@ public void testRetryWithRandomBackoff() { await().until(() -> subscriber.getItems().size() >= 10); subscriber .assertItems(0, 1, 0, 1, 0, 1, 0, 1, 0, 1) // Initial subscription + 4 retries - .assertFailedWith(IllegalStateException.class, "Retries exhausted"); + .assertFailedWith(IOException.class, "boom retry") + .awaitFailure(t -> { + // expecting an IllegalStateException with an info about the retries made + Throwable[] suppressed = exception.getSuppressed(); + assertThat(suppressed.length).isEqualTo(1); + assertThat(suppressed).anyMatch(s -> s instanceof IllegalStateException && + s.getMessage().equals("Retries exhausted: 4/4")); + }); } @@ -336,7 +343,14 @@ public void testRetryWithRandomBackoffAndDefaultJitter() { await().until(() -> subscriber.getItems().size() >= 10); subscriber .assertItems(0, 1, 0, 1, 0, 1, 0, 1, 0, 1) // Initial subscription + 4 retries - .assertFailedWith(IllegalStateException.class, "Retries exhausted: 4/4"); + .assertFailedWith(IOException.class, "boom retry") + .awaitFailure(t -> { + // expecting an IllegalStateException with an info about the retries made + Throwable[] suppressed = exception.getSuppressed(); + assertThat(suppressed.length).isEqualTo(1); + assertThat(suppressed).anyMatch(s -> s instanceof IllegalStateException && + s.getMessage().equals("Retries exhausted: 4/4")); + }); } @Test @@ -353,7 +367,14 @@ public void testRetryWithDefaultMax() { .until(() -> subscriber.getItems().size() >= 10); subscriber .assertItems(0, 1, 0, 1, 0, 1, 0, 1, 0, 1) // Initial subscription + 4 retries - .assertFailedWith(IllegalStateException.class, "Retries exhausted: 4/4"); + .assertFailedWith(IOException.class, "boom retry") + .awaitFailure(t -> { + // expecting an IllegalStateException with an info about the retries made + Throwable[] suppressed = exception.getSuppressed(); + assertThat(suppressed.length).isEqualTo(1); + assertThat(suppressed).anyMatch(s -> s instanceof IllegalStateException && + s.getMessage().equals("Retries exhausted: 4/4")); + }); } }