From 9dab4bf789870d0774068083e2ed531293b11b65 Mon Sep 17 00:00:00 2001 From: ndr_brt Date: Mon, 16 Sep 2024 10:03:46 +0200 Subject: [PATCH] fix: catch exceptions in CompletableFutureRetryProcess --- .../retry/CompletableFutureRetryProcess.java | 11 +++++++- .../retry/StatusResultRetryProcess.java | 26 +++++++++---------- .../CompletableFutureRetryProcessTest.java | 13 ++++++++++ .../retry/StatusResultRetryProcessTest.java | 5 ++-- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/core/common/lib/state-machine-lib/src/main/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcess.java b/core/common/lib/state-machine-lib/src/main/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcess.java index d491e0431e8..77d94696ddb 100644 --- a/core/common/lib/state-machine-lib/src/main/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcess.java +++ b/core/common/lib/state-machine-lib/src/main/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcess.java @@ -47,7 +47,8 @@ public CompletableFutureRetryProcess(E entity, Supplier> 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 runProcess() { + try { + return process.get(); + } catch (Throwable e) { + return CompletableFuture.failedFuture(e); + } + } + public SELF onSuccess(BiConsumer onSuccessHandler) { this.onSuccessHandler = onSuccessHandler; return (SELF) this; diff --git a/core/common/lib/state-machine-lib/src/main/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcess.java b/core/common/lib/state-machine-lib/src/main/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcess.java index 41661203b44..1f0bdc19027 100644 --- a/core/common/lib/state-machine-lib/src/main/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcess.java +++ b/core/common/lib/state-machine-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> process, Mon boolean process(E entity, String description) { monitor.debug(format("%s: ID %s. %s", entity.getClass().getSimpleName(), entity.getId(), description)); - StatusResult 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 result) { if (result.succeeded()) { if (onSuccessHandler != null) { onSuccessHandler.accept(entity, result.getContent()); @@ -99,6 +88,8 @@ public void handleResult(E entity, String description, StatusResult result) { } } } + + return true; } public StatusResultRetryProcess onSuccess(BiConsumer onSuccessHandler) { @@ -120,4 +111,13 @@ public StatusResultRetryProcess onRetryExhausted(BiConsumer 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/core/common/lib/state-machine-lib/src/test/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcessTest.java b/core/common/lib/state-machine-lib/src/test/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcessTest.java index 79427ffbe13..3b6c91a9169 100644 --- a/core/common/lib/state-machine-lib/src/test/java/org/eclipse/edc/statemachine/retry/CompletableFutureRetryProcessTest.java +++ b/core/common/lib/state-machine-lib/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/core/common/lib/state-machine-lib/src/test/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcessTest.java b/core/common/lib/state-machine-lib/src/test/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcessTest.java index ae9758ecba8..ecf84127e9a 100644 --- a/core/common/lib/state-machine-lib/src/test/java/org/eclipse/edc/statemachine/retry/StatusResultRetryProcessTest.java +++ b/core/common/lib/state-machine-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); } }