fix: catch exceptions in CompletableFutureRetryProcess

eclipse-edc · Sep 16, 2024 · 9dab4bf · 9dab4bf
1 parent 7e0e0f5
commit 9dab4bf
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 17 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,7 +24,7 @@
 import java.util.function.Supplier;
 
 import static java.lang.String.format;
-import static org.eclipse.edc.spi.response.ResponseStatus.FATAL_ERROR;
+import static org.eclipse.edc.spi.response.ResponseStatus.ERROR_RETRY;
 
 /**
  * Provides retry capabilities to a synchronous process that returns a {@link StatusResult} object
@@ -47,19 +47,8 @@ public StatusResultRetryProcess(E entity, Supplier<StatusResult<C>> process, Mon
     boolean process(E entity, String description) {
         monitor.debug(format("%s: ID %s. %s", entity.getClass().getSimpleName(), entity.getId(), description));
 
-        StatusResult<C> result;
-        try {
-            result = process.get();
-        } catch (Exception e) {
-            result = StatusResult.failure(FATAL_ERROR, "Unexpected exception thrown %s: %s".formatted(e, e.getMessage()));
-        }
+        var result = runProcess();
 
-        handleResult(entity, description, result);
-
-        return true;
-    }
-
-    public void handleResult(E entity, String description, StatusResult<C> result) {
         if (result.succeeded()) {
             if (onSuccessHandler != null) {
                 onSuccessHandler.accept(entity, result.getContent());
@@ -99,6 +88,8 @@ public void handleResult(E entity, String description, StatusResult<C> result) {
                 }
             }
         }
+
+        return true;
     }
 
     public StatusResultRetryProcess<E, C> onSuccess(BiConsumer<E, C> onSuccessHandler) {
@@ -120,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
@@ -98,18 +98,17 @@ 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).onFatalError(onFatalError).execute("any");
+        var result = retryProcess.onSuccess(onSuccess).onFailure(onFailure).execute("any");
 
         assertThat(result).isTrue();
         verify(process).get();
-        verify(onFatalError).accept(same(entity), any());
+        verify(onFailure).accept(same(entity), any());
         verifyNoInteractions(onSuccess);
     }
 }