diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8b5f8a09..b4f37cd7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,8 @@
-# Next Minor Release
+# 2.4.4
+
+### Bug Fixes
+
+- Fixed #298 - `Fallback.onFailedAttempt` not being called correctly
### API Changes
diff --git a/src/main/java/net/jodah/failsafe/Fallback.java b/src/main/java/net/jodah/failsafe/Fallback.java
index 5d0fd9c1..2d85caf7 100644
--- a/src/main/java/net/jodah/failsafe/Fallback.java
+++ b/src/main/java/net/jodah/failsafe/Fallback.java
@@ -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 and 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.
*
Note: Any exceptions that are thrown from within the {@code listener} are ignored.
*/
public Fallback onFailedAttempt(CheckedConsumer extends ExecutionAttemptedEvent> listener) {
diff --git a/src/main/java/net/jodah/failsafe/FallbackExecutor.java b/src/main/java/net/jodah/failsafe/FallbackExecutor.java
index 972a0f2c..09366d00 100644
--- a/src/main/java/net/jodah/failsafe/FallbackExecutor.java
+++ b/src/main/java/net/jodah/failsafe/FallbackExecutor.java
@@ -47,6 +47,9 @@ protected Supplier supply(Supplier supplier, S
return result;
if (isFailure(result)) {
+ if (failedAttemptListener != null)
+ failedAttemptListener.handle(result, execution);
+
try {
result = policy == Fallback.VOID ?
result.withNonResult() :
@@ -75,6 +78,9 @@ protected Supplier> supplyAsync(
if (!isFailure(result))
return postExecuteAsync(result, scheduler, future);
+ if (failedAttemptListener != null)
+ failedAttemptListener.handle(result, execution);
+
CompletableFuture promise = new CompletableFuture<>();
Callable callable = () -> {
try {
@@ -113,11 +119,4 @@ protected Supplier> supplyAsync(
return promise.thenCompose(ss -> postExecuteAsync(ss, scheduler, future));
});
}
-
- @Override
- protected ExecutionResult onFailure(ExecutionResult result) {
- if (failedAttemptListener != null)
- failedAttemptListener.handle(result, execution);
- return result;
- }
}
diff --git a/src/test/java/net/jodah/failsafe/issues/Issue284Test.java b/src/test/java/net/jodah/failsafe/issues/Issue284Test.java
index 79ca34bf..e91674ef 100644
--- a/src/test/java/net/jodah/failsafe/issues/Issue284Test.java
+++ b/src/test/java/net/jodah/failsafe/issues/Issue284Test.java
@@ -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");
}
diff --git a/src/test/java/net/jodah/failsafe/issues/Issue298Test.java b/src/test/java/net/jodah/failsafe/issues/Issue298Test.java
new file mode 100644
index 00000000..6f8dcb87
--- /dev/null
+++ b/src/test/java/net/jodah/failsafe/issues/Issue298Test.java
@@ -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 fallback = Fallback.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());
+ }
+}