fix: catch exceptions in RetryProcess (#4492)

* fix: catch exceptions in the StatusResultRetryProcess (#4458) * fix: catch exceptions in CompletableFutureRetryProcess (#4477) * dependencies
eclipse-edc · Sep 25, 2024 · 54ee486 · 54ee486
1 parent ad00f2a
commit 54ee486
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 7 deletions.
diff --git a/...e-lib/src/main/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcess.java b/...e-lib/src/main/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcess.java
@@ -47,7 +47,8 @@ public CompletableFutureRetryProcess(E entity, Supplier<CompletableFuture<C>> pr
     @Override
     boolean process(E entity, String description) {
         monitor.debug(format("%s: ID %s. %s", entity.getClass().getSimpleName(), entity.getId(), description));
-        process.get()
+
+        runProcess()
                 .whenComplete((result, throwable) -> {
                     var reloadedEntity = Optional.ofNullable(entityRetrieve)
                             .map(it -> it.apply(entity.getId()))
@@ -83,6 +84,14 @@ boolean process(E entity, String description) {
         return true;
     }
 
+    private CompletableFuture<C> runProcess() {
+        try {
+            return process.get();
+        } catch (Throwable e) {
+            return CompletableFuture.failedFuture(e);
+        }
+    }
+
     public SELF onSuccess(BiConsumer<E, C> onSuccessHandler) {
         this.onSuccessHandler = onSuccessHandler;
         return (SELF) this;

diff --git a/...achine-lib/src/main/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcess.java b/...achine-lib/src/main/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcess.java
@@ -24,6 +24,7 @@
 import java.util.function.Supplier;
 
 import static java.lang.String.format;
+import static org.eclipse.edc.spi.response.ResponseStatus.ERROR_RETRY;
 
 /**
  * Provides retry capabilities to a synchronous process that returns a {@link StatusResult} object
@@ -45,14 +46,9 @@ public StatusResultRetryProcess(E entity, Supplier<StatusResult<C>> process, Mon
     @Override
     boolean process(E entity, String description) {
         monitor.debug(format("%s: ID %s. %s", entity.getClass().getSimpleName(), entity.getId(), description));
-        var result = process.get();
 
-        handleResult(entity, description, result);
+        var result = runProcess();
 
-        return true;
-    }
-
-    public void handleResult(E entity, String description, StatusResult<C> result) {
         if (result.succeeded()) {
             if (onSuccessHandler != null) {
                 onSuccessHandler.accept(entity, result.getContent());
@@ -92,6 +88,8 @@ public void handleResult(E entity, String description, StatusResult<C> result) {
                 }
             }
         }
+
+        return true;
     }
 
     public StatusResultRetryProcess<E, C> onSuccess(BiConsumer<E, C> onSuccessHandler) {
@@ -113,4 +111,13 @@ public StatusResultRetryProcess<E, C> onRetryExhausted(BiConsumer<E, ResponseFai
         this.onRetryExhausted = onRetryExhausted;
         return this;
     }
+
+    private StatusResult<C> runProcess() {
+        try {
+            return process.get();
+        } catch (Throwable e) {
+            return StatusResult.failure(ERROR_RETRY, "Unexpected exception thrown %s: %s".formatted(e, e.getMessage()));
+        }
+    }
+
 }
diff --git a/...b/src/test/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcessTest.java b/...b/src/test/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcessTest.java
@@ -107,4 +107,17 @@ void shouldExecuteOnRetry_whenFailureAndRetriesHaveNotBeenExhausted() {
 
         verify(onFailure).accept(eq(entity), isA(EdcException.class));
     }
+
+    @Test
+    void shouldFail_whenExceptionIsThrown() {
+        when(process.get()).thenThrow(new EdcException("generic error"));
+        var entity = TestEntity.Builder.newInstance().id(UUID.randomUUID().toString()).clock(clock).build();
+        var retryProcess = new CompletableFutureRetryProcess<>(entity, process, mock(Monitor.class), clock, configuration);
+
+        var result = retryProcess.onSuccess(onSuccess).onFailure(onFailure).execute("any");
+
+        assertThat(result).isTrue();
+        verify(process).get();
+        verify(onFailure).accept(eq(entity), isA(EdcException.class));
+    }
 }
diff --git a/...ne-lib/src/test/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcessTest.java b/...ne-lib/src/test/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcessTest.java
@@ -14,6 +14,7 @@
 
 package org.eclipse.edc.statemachine.retry;
 
+import org.eclipse.edc.spi.EdcException;
 import org.eclipse.edc.spi.monitor.Monitor;
 import org.eclipse.edc.spi.response.ResponseFailure;
 import org.eclipse.edc.spi.response.StatusResult;
@@ -29,8 +30,11 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.eclipse.edc.spi.response.ResponseStatus.ERROR_RETRY;
 import static org.eclipse.edc.spi.response.ResponseStatus.FATAL_ERROR;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.same;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 class StatusResultRetryProcessTest {
@@ -93,4 +97,18 @@ void shouldExecuteOnRetry_whenFailureAndRetriesHaveNotBeenExhausted() {
 
         verify(onFailure).accept(entity, statusResult.getFailure());
     }
+
+    @Test
+    void shouldCallFatalError_whenExceptionIsThrown() {
+        when(process.get()).thenThrow(new EdcException("code throws an exception"));
+        var entity = TestEntity.Builder.newInstance().id(UUID.randomUUID().toString()).clock(clock).build();
+        var retryProcess = new StatusResultRetryProcess<>(entity, process, mock(Monitor.class), clock, configuration);
+
+        var result = retryProcess.onSuccess(onSuccess).onFailure(onFailure).execute("any");
+
+        assertThat(result).isTrue();
+        verify(process).get();
+        verify(onFailure).accept(same(entity), any());
+        verifyNoInteractions(onSuccess);
+    }
 }