Skip to content

Commit

Permalink
Fix Fallback.onFailedAttempt
Browse files Browse the repository at this point in the history
Fixes #298
  • Loading branch information
jhalterman committed Sep 18, 2021
1 parent d057983 commit 15d539c
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 12 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
# Next Minor Release
# 2.4.4

### Bug Fixes

- Fixed #298 - `Fallback.onFailedAttempt` not being called correctly

### API Changes

Expand Down
5 changes: 2 additions & 3 deletions src/main/java/net/jodah/failsafe/Fallback.java
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,8 @@ public boolean isAsync() {
}

/**
* Registers the {@code listener} to be called when an execution attempt fails. You can also use {@link
* #onFailure(CheckedConsumer) onFailure} to determine when the execution attempt fails <i>and</i> and the fallback
* result fails.
* Registers the {@code listener} to be called when the last execution attempt prior to the fallback failed. You can
* also use {@link #onFailure(CheckedConsumer) onFailure} to determine when the fallback attempt also fails.
* <p>Note: Any exceptions that are thrown from within the {@code listener} are ignored.</p>
*/
public Fallback<R> onFailedAttempt(CheckedConsumer<? extends ExecutionAttemptedEvent<R>> listener) {
Expand Down
13 changes: 6 additions & 7 deletions src/main/java/net/jodah/failsafe/FallbackExecutor.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ protected Supplier<ExecutionResult> supply(Supplier<ExecutionResult> supplier, S
return result;

if (isFailure(result)) {
if (failedAttemptListener != null)
failedAttemptListener.handle(result, execution);

try {
result = policy == Fallback.VOID ?
result.withNonResult() :
Expand Down Expand Up @@ -75,6 +78,9 @@ protected Supplier<CompletableFuture<ExecutionResult>> supplyAsync(
if (!isFailure(result))
return postExecuteAsync(result, scheduler, future);

if (failedAttemptListener != null)
failedAttemptListener.handle(result, execution);

CompletableFuture<ExecutionResult> promise = new CompletableFuture<>();
Callable<R> callable = () -> {
try {
Expand Down Expand Up @@ -113,11 +119,4 @@ protected Supplier<CompletableFuture<ExecutionResult>> supplyAsync(
return promise.thenCompose(ss -> postExecuteAsync(ss, scheduler, future));
});
}

@Override
protected ExecutionResult onFailure(ExecutionResult result) {
if (failedAttemptListener != null)
failedAttemptListener.handle(result, execution);
return result;
}
}
2 changes: 1 addition & 1 deletion src/test/java/net/jodah/failsafe/issues/Issue284Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void testFallbackSuccess() {
String result = Failsafe.with(fallback).get(() -> null);

assertEquals(result, "hello");
assertEquals(failedAttempt.get(), 0);
assertEquals(failedAttempt.get(), 1);
assertTrue(success.get(), "Fallback should have been successful");
}

Expand Down
45 changes: 45 additions & 0 deletions src/test/java/net/jodah/failsafe/issues/Issue298Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package net.jodah.failsafe.issues;

import net.jodah.failsafe.Failsafe;
import net.jodah.failsafe.Fallback;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import java.util.concurrent.atomic.AtomicBoolean;

import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;

@Test
public class Issue298Test {
AtomicBoolean failedAttemptCalled = new AtomicBoolean();
AtomicBoolean failureCalled = new AtomicBoolean();

Fallback<String> fallback = Fallback.<String>of(e -> "success")
.onFailedAttempt(e -> failedAttemptCalled.set(true))
.onFailure(e -> failureCalled.set(true));

@BeforeMethod
protected void beforeMethod() {
failedAttemptCalled.set(false);
failureCalled.set(false);
}

public void testSync() {
Failsafe.with(fallback).get(() -> {
throw new Exception();
});

assertTrue(failedAttemptCalled.get());
assertFalse(failureCalled.get());
}

public void testAsync() throws Throwable {
Failsafe.with(fallback).getAsync(() -> {
throw new Exception();
}).get();

assertTrue(failedAttemptCalled.get());
assertFalse(failureCalled.get());
}
}

0 comments on commit 15d539c

Please sign in to comment.