Skip to content

Commit

Permalink
Rework the RetryExhausted exception message to better reflect transient
Browse files Browse the repository at this point in the history
  • Loading branch information
@simonbasle
simonbasle committed Mar 10, 2020
1 parent ccda4b3 commit 4caf692
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 32 deletions.
8 changes: 4 additions & 4 deletions reactor-core/src/main/java/reactor/util/retry/RetryBackoffSpec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@
public final class RetryBackoffSpec implements Retry, Supplier<Retry> {

static final BiFunction<RetryBackoffSpec, RetrySignal, Throwable> BACKOFF_EXCEPTION_GENERATOR = (builder, rs) ->
Exceptions.retryExhausted("Retries exhausted: " + rs.totalRetries() + "/" + builder.maxAttempts +
(rs.totalRetriesInARow() != rs.totalRetries()
? " (" + rs.totalRetriesInARow() + " in a row)"
: ""
Exceptions.retryExhausted("Retries exhausted: " + (
builder.isTransientErrors
? rs.totalRetriesInARow() + "/" + builder.maxAttempts + " in a row (" + rs.totalRetries() + " total)"
: rs.totalRetries() + "/" + builder.maxAttempts
), rs.failure());

/**
Expand Down
10 changes: 5 additions & 5 deletions reactor-core/src/main/java/reactor/util/retry/RetrySpec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ public final class RetrySpec implements Retry, Supplier<Retry> {

static final BiFunction<RetrySpec, RetrySignal, Throwable>
RETRY_EXCEPTION_GENERATOR = (builder, rs) ->
Exceptions.retryExhausted("Retries exhausted: " + rs.totalRetries() + "/" + builder.maxAttempts +
(rs.totalRetriesInARow() != rs.totalRetries()
? " (" + rs.totalRetriesInARow() + " in a row)"
: ""
), rs.failure());
Exceptions.retryExhausted("Retries exhausted: " + (
builder.isTransientErrors
? rs.totalRetriesInARow() + "/" + builder.maxAttempts + " in a row (" + rs.totalRetries() + " total)"
: rs.totalRetries() + "/" + builder.maxAttempts
), rs.failure());

/**
* The configured maximum for retry attempts.
Expand Down
18 changes: 9 additions & 9 deletions reactor-core/src/test/java/reactor/core/publisher/FluxRetryWhenTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ public void fluxRetryRandomBackoff() {
.expectNext(0, 1) //normal output
.expectNext(0, 1, 0, 1, 0, 1, 0, 1) //4 retry attempts
.expectErrorSatisfies(e -> assertThat(e).isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 4/4 (0 in a row)")
.hasMessage("Retries exhausted: 4/4")
.hasCause(exception))
.verify(Duration.ofSeconds(1)); //vts test shouldn't even take that long

Expand Down Expand Up @@ -493,7 +493,7 @@ public void fluxRetryRandomBackoffDefaultJitter() {
.expectNext(0, 1) //normal output
.expectNext(0, 1, 0, 1, 0, 1, 0, 1) //4 retry attempts
.expectErrorSatisfies(e -> assertThat(e).isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 4/4 (0 in a row)")
.hasMessage("Retries exhausted: 4/4")
.hasCause(exception))
.verify(Duration.ofSeconds(1)); //vts test shouldn't even take that long

Expand Down Expand Up @@ -526,7 +526,7 @@ public void fluxRetryRandomBackoffDefaultMaxDuration() {
.expectNext(0, 1) //normal output
.expectNext(0, 1, 0, 1, 0, 1, 0, 1) //4 retry attempts
.expectErrorSatisfies(e -> assertThat(e).isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 4/4 (0 in a row)")
.hasMessage("Retries exhausted: 4/4")
.hasCause(exception))
.verify(Duration.ofSeconds(1)); //vts test shouldn't even take that long

Expand Down Expand Up @@ -563,7 +563,7 @@ public void fluxRetryRandomBackoff_maxBackoffShaves() {
.expectNext(0, 1) //normal output
.expectNext(0, 1, 0, 1, 0, 1, 0, 1) //4 retry attempts
.expectErrorSatisfies(e -> assertThat(e).isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 4/4 (0 in a row)")
.hasMessage("Retries exhausted: 4/4")
.hasCause(exception))
.verify(Duration.ofSeconds(1)); //vts test shouldn't even take that long

Expand Down Expand Up @@ -611,7 +611,7 @@ public void fluxRetryRandomBackoff_minBackoffFloor() {
.expectNext(0, 1) //normal output
.expectNext(0, 1) //1 retry attempts
.expectErrorSatisfies(e -> assertThat(e).isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 1/1 (0 in a row)")
.hasMessage("Retries exhausted: 1/1")
.hasCause(exception))
.verify(Duration.ofSeconds(1)); //vts test shouldn't even take that long

Expand Down Expand Up @@ -646,7 +646,7 @@ public void fluxRetryRandomBackoff_noRandomness() {
.expectNext(0, 1) //normal output
.expectNext(0, 1, 0, 1, 0, 1, 0, 1) //4 retry attempts
.expectErrorSatisfies(e -> assertThat(e).isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 4/4 (0 in a row)")
.hasMessage("Retries exhausted: 4/4")
.hasCause(exception))
.verify(Duration.ofSeconds(1)); //vts test shouldn't even take that long

Expand Down Expand Up @@ -697,7 +697,7 @@ public void fluxRetryBackoffWithSpecificScheduler() {
.then(() -> backoffScheduler.advanceTimeBy(Duration.ofHours(4)))
.expectNext(0, 1, 0, 1, 0, 1, 0, 1) //4 retry attempts
.expectErrorSatisfies(e -> assertThat(e).isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 4/4 (0 in a row)")
.hasMessage("Retries exhausted: 4/4")
.hasCause(exception))
.verify(Duration.ofMillis(100)); //test should only take roughly the expectNoEvent time
}
Expand All @@ -722,7 +722,7 @@ public void fluxRetryBackoffRetriesOnGivenScheduler() {
)
.expectNext(0, 1, 0, 1, 0, 1)
.expectErrorSatisfies(e -> assertThat(e).isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 2/2 (0 in a row)")
.hasMessage("Retries exhausted: 2/2")
.hasCause(exception))
.verify(Duration.ofMillis(200));

Expand All @@ -748,7 +748,7 @@ public void backoffFunctionNotTransient() {
new FluxRetryWhen<>(source, retryFunction)
.as(StepVerifier::create)
.expectNext(3, 4)
.expectErrorMessage("Retries exhausted: 2/2 (0 in a row)")
.expectErrorMessage("Retries exhausted: 2/2")
.verify(Duration.ofSeconds(2));
}

Expand Down
2 changes: 1 addition & 1 deletion reactor-core/src/test/java/reactor/guide/GuideTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ else if (i % 3 == 0) { // <3>
transientFlux.retryWhen(Retry.max(2))
.as(StepVerifier::create)
.expectNext(0, 3)
.verifyErrorMessage("Retries exhausted: 2/2 (0 in a row)");
.verifyErrorMessage("Retries exhausted: 2/2");

}

Expand Down
16 changes: 9 additions & 7 deletions reactor-core/src/test/java/reactor/util/retry/RetryBackoffSpecTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public void builderCanBeUsedAsTemplate() {
.expectNext(1, 3)
.verifyErrorSatisfies(t -> assertThat(t)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 2/2 (0 in a row)")
.hasMessage("Retries exhausted: 2/2")
.hasCause(new IllegalStateException("boom 4")));

StepVerifier.create(modifiedTemplate2, StepVerifierOptions.create().scenarioName("modified template 2"))
Expand Down Expand Up @@ -201,13 +201,13 @@ public void retryThenIsCumulative() {

@Test
public void retryExceptionDefaultsToRetryExhausted() {
RetryBackoffSpec retryBuilder = Retry.backoff(50, Duration.ZERO);
RetryBackoffSpec retryBuilder = Retry.backoff(50, Duration.ZERO).transientErrors(true);

final ImmutableRetrySignal trigger = new ImmutableRetrySignal(100, 21, new IllegalStateException("boom"));
final ImmutableRetrySignal trigger = new ImmutableRetrySignal(100, 50, new IllegalStateException("boom"));

StepVerifier.create(retryBuilder.generateCompanion(Flux.just(trigger)))
.expectErrorSatisfies(e -> assertThat(e).matches(Exceptions::isRetryExhausted, "isRetryExhausted")
.hasMessage("Retries exhausted: 100/50 (21 in a row)")
.hasMessage("Retries exhausted: 50/50 in a row (100 total)")
.hasCause(new IllegalStateException("boom")))
.verify();
}
Expand All @@ -228,14 +228,16 @@ public void retryExceptionCanBeCustomized() {

@Test
public void defaultRetryExhaustedMessageWithNoTransientErrors() {
assertThat(RetryBackoffSpec.BACKOFF_EXCEPTION_GENERATOR.apply(Retry.backoff(123, Duration.ZERO), new ImmutableRetrySignal(123, 123, null)))
assertThat(RetryBackoffSpec.BACKOFF_EXCEPTION_GENERATOR.apply(Retry.backoff(123, Duration.ZERO),
new ImmutableRetrySignal(123, 123, null)))
.hasMessage("Retries exhausted: 123/123");
}

@Test
public void defaultRetryExhaustedMessageWithTransientErrors() {
assertThat(RetryBackoffSpec.BACKOFF_EXCEPTION_GENERATOR.apply(Retry.backoff(123, Duration.ZERO), new ImmutableRetrySignal(123, 12, null)))
.hasMessage("Retries exhausted: 123/123 (12 in a row)");
assertThat(RetryBackoffSpec.BACKOFF_EXCEPTION_GENERATOR.apply(Retry.backoff(12, Duration.ZERO).transientErrors(true),
new ImmutableRetrySignal(123, 12, null)))
.hasMessage("Retries exhausted: 12/12 in a row (123 total)");
}

@Test
Expand Down
13 changes: 7 additions & 6 deletions reactor-core/src/test/java/reactor/util/retry/RetrySpecTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public void builderCanBeUsedAsTemplate() {
.expectNext(1, 3)
.verifyErrorSatisfies(t -> assertThat(t)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Retries exhausted: 2/2 (0 in a row)")
.hasMessage("Retries exhausted: 2/2")
.hasCause(new IllegalStateException("boom 4")));

StepVerifier.create(modifiedTemplate2, StepVerifierOptions.create().scenarioName("modified template 2"))
Expand Down Expand Up @@ -192,13 +192,13 @@ public void retryThenIsCumulative() {

@Test
public void retryExceptionDefaultsToRetryExhausted() {
RetrySpec retrySpec = Retry.max(50);
RetrySpec retrySpec = Retry.max(50).transientErrors(true);

final ImmutableRetrySignal trigger = new ImmutableRetrySignal(100, 21, new IllegalStateException("boom"));
final ImmutableRetrySignal trigger = new ImmutableRetrySignal(100, 50, new IllegalStateException("boom"));

StepVerifier.create(retrySpec.generateCompanion(Flux.just(trigger)))
.expectErrorSatisfies(e -> assertThat(e).matches(Exceptions::isRetryExhausted, "isRetryExhausted")
.hasMessage("Retries exhausted: 100/50 (21 in a row)")
.hasMessage("Retries exhausted: 50/50 in a row (100 total)")
.hasCause(new IllegalStateException("boom")))
.verify();
}
Expand Down Expand Up @@ -226,8 +226,9 @@ public void defaultRetryExhaustedMessageWithNoTransientErrors() {

@Test
public void defaultRetryExhaustedMessageWithTransientErrors() {
assertThat(RetrySpec.RETRY_EXCEPTION_GENERATOR.apply(Retry.max(123), new ImmutableRetrySignal(123, 12, null)))
.hasMessage("Retries exhausted: 123/123 (12 in a row)");
assertThat(RetrySpec.RETRY_EXCEPTION_GENERATOR.apply(Retry.max(12).transientErrors(true),
new ImmutableRetrySignal(123, 12, null)))
.hasMessage("Retries exhausted: 12/12 in a row (123 total)");
}

@Test
Expand Down

0 comments on commit 4caf692

Please sign in to comment.