From c5f45fc62a4130c98f3de4ca193cd9a34bc59da1 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 1 Feb 2021 16:00:47 +0200 Subject: [PATCH 01/18] allow overriding log level for retryable exception logging --- .../executor/WorkflowStateProcessor.java | 30 +++++++++++++++---- .../definition/ExceptionSeverity.java | 5 ++++ .../workflow/definition/WorkflowState.java | 12 ++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index 87c73f942..f873ebe46 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -87,8 +87,9 @@ class WorkflowStateProcessor implements Runnable { private Thread thread; private ListenerContext listenerContext; - WorkflowStateProcessor(long instanceId, Supplier shutdownRequested, ObjectStringMapper objectMapper, WorkflowDefinitionService workflowDefinitions, - WorkflowInstanceService workflowInstances, WorkflowInstanceDao workflowInstanceDao, MaintenanceDao maintenanceDao, + WorkflowStateProcessor(long instanceId, Supplier shutdownRequested, ObjectStringMapper objectMapper, + WorkflowDefinitionService workflowDefinitions, WorkflowInstanceService workflowInstances, + WorkflowInstanceDao workflowInstanceDao, MaintenanceDao maintenanceDao, WorkflowInstancePreProcessor workflowInstancePreProcessor, Environment env, Map processingInstances, WorkflowExecutorListener... executorListeners) { this.instanceId = instanceId; @@ -125,7 +126,8 @@ public void run() { logger.error("Failed to process workflow instance and shutdown requested", ex); break; } - logger.error("Failed to process workflow instance {}, retrying after {} seconds", instanceId, stateProcessingRetryDelay, ex); + logger.error("Failed to process workflow instance {}, retrying after {} seconds", instanceId, stateProcessingRetryDelay, + ex); sleepIgnoreInterrupted(stateProcessingRetryDelay); } } @@ -170,7 +172,7 @@ private void runImpl() { } execution.setFailed(t); if (state.isRetryAllowed(t)) { - logger.error("Handler threw a retryable exception, trying again later.", t); + logRetryableException(state, t); execution.setRetry(true); execution.setNextState(state); execution.setNextStateReason(getStackTrace(t)); @@ -195,6 +197,21 @@ private void runImpl() { logger.debug("Finished."); } + private void logRetryableException(WorkflowState state, Throwable t) { + switch (state.getExceptionSeverity(t)) { + case INFO: + logger.info("Handler threw a retryable exception, trying again later. Message: ", t.getMessage()); + return; + case WARNING: + logger.warn("Handler threw a retryable exception, trying again later.", t); + return; + case ERROR: + default: + logger.error("Handler threw a retryable exception, trying again later.", t); + return; + } + } + void logIfLagging(WorkflowInstance instance) { Duration executionLag = new Duration(instance.nextActivation, now()); if (executionLag.isLongerThan(standardMinutes(1))) { @@ -275,8 +292,9 @@ private WorkflowInstance saveWorkflowInstanceState(StateExecutionImpl execution, return persistWorkflowInstanceState(execution, instance.stateVariables, actionBuilder, instanceBuilder); } catch (Exception ex) { if (shutdownRequested.get()) { - logger.error("Failed to save workflow instance {} new state, not retrying due to shutdown request. The state will be rerun on recovery.", - instance.id, ex); + logger.error( + "Failed to save workflow instance {} new state, not retrying due to shutdown request. The state will be rerun on recovery.", + instance.id, ex); // return the original instance since persisting failed return instance; } diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java new file mode 100644 index 000000000..2178d9b10 --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java @@ -0,0 +1,5 @@ +package io.nflow.engine.workflow.definition; + +public enum ExceptionSeverity { + ERROR, WARNING, INFO +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java index 9a7c82d53..39539ca4f 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java @@ -39,4 +39,16 @@ default String getDescription() { default boolean isRetryAllowed(Throwable thrown) { return !thrown.getClass().isAnnotationPresent(NonRetryable.class); } + + /** + * Return the severity of the exception thrown by the state execution. Default is ERROR. Stack trace will not be logged for + * INFO. + * + * @param thrown + * The thrown exception. + * @return Exception severity. + */ + default ExceptionSeverity getExceptionSeverity(Throwable thrown) { + return ExceptionSeverity.ERROR; + } } From b054961c689eeaae2f9b4dfac2d23c600f29520a Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 1 Feb 2021 17:23:59 +0200 Subject: [PATCH 02/18] refactor --- .../executor/WorkflowStateProcessor.java | 30 ++++++++++++----- .../definition/ExceptionSeverity.java | 32 +++++++++++++++++-- .../workflow/definition/WorkflowState.java | 5 ++- 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index f873ebe46..382fcb6d8 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -24,12 +24,14 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.function.BiConsumer; import java.util.function.Supplier; import org.joda.time.DateTime; import org.joda.time.Duration; import org.slf4j.Logger; import org.slf4j.MDC; +import org.slf4j.event.Level; import org.springframework.core.env.Environment; import org.springframework.dao.DataAccessException; import org.springframework.dao.EmptyResultDataAccessException; @@ -49,6 +51,7 @@ import io.nflow.engine.service.WorkflowDefinitionService; import io.nflow.engine.service.WorkflowInstanceService; import io.nflow.engine.workflow.definition.AbstractWorkflowDefinition; +import io.nflow.engine.workflow.definition.ExceptionSeverity; import io.nflow.engine.workflow.definition.NextAction; import io.nflow.engine.workflow.definition.WorkflowSettings; import io.nflow.engine.workflow.definition.WorkflowState; @@ -198,17 +201,28 @@ private void runImpl() { } private void logRetryableException(WorkflowState state, Throwable t) { - switch (state.getExceptionSeverity(t)) { + ExceptionSeverity exceptionSeverity = state.getExceptionSeverity(t); + BiConsumer logMethod = getLogMethod(exceptionSeverity.logLevel); + if (exceptionSeverity.logStackTrace) { + logMethod.accept("Handler threw a retryable exception, trying again later.", t); + } else { + logMethod.accept("Handler threw a retryable exception, trying again later. Message: {}", t.getMessage()); + } + } + + private BiConsumer getLogMethod(Level logLevel) { + switch (logLevel) { + case TRACE: + return logger::trace; + case DEBUG: + return logger::debug; case INFO: - logger.info("Handler threw a retryable exception, trying again later. Message: ", t.getMessage()); - return; - case WARNING: - logger.warn("Handler threw a retryable exception, trying again later.", t); - return; + return logger::info; + case WARN: + return logger::warn; case ERROR: default: - logger.error("Handler threw a retryable exception, trying again later.", t); - return; + return logger::error; } } diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java index 2178d9b10..01cc2b849 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java @@ -1,5 +1,33 @@ package io.nflow.engine.workflow.definition; -public enum ExceptionSeverity { - ERROR, WARNING, INFO +import static org.slf4j.event.Level.ERROR; + +import org.slf4j.event.Level; + +public class ExceptionSeverity { + public static final ExceptionSeverity DEFAULT = new ExceptionSeverity(ERROR, true); + public final Level logLevel; + public final boolean logStackTrace; + + ExceptionSeverity(Level logLevel, boolean logStackTrace) { + this.logLevel = logLevel; + this.logStackTrace = logStackTrace; + } + + public static class Builder { + private Level logLevel = ERROR; + private boolean logStackTrace = true; + + public void setLogLevel(Level logLevel) { + this.logLevel = logLevel; + } + + public void setLogStackTrace(boolean logStackTrace) { + this.logStackTrace = logStackTrace; + } + + public ExceptionSeverity build() { + return new ExceptionSeverity(logLevel, logStackTrace); + } + } } diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java index 39539ca4f..8fdc18ce8 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java @@ -41,14 +41,13 @@ default boolean isRetryAllowed(Throwable thrown) { } /** - * Return the severity of the exception thrown by the state execution. Default is ERROR. Stack trace will not be logged for - * INFO. + * Return the severity of the exception thrown by the state execution. Using default means ERROR level logging with stack trace. * * @param thrown * The thrown exception. * @return Exception severity. */ default ExceptionSeverity getExceptionSeverity(Throwable thrown) { - return ExceptionSeverity.ERROR; + return ExceptionSeverity.DEFAULT; } } From d02c93fd35b7f6bf71634d687b98278ea703d969 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 1 Feb 2021 18:27:15 +0200 Subject: [PATCH 03/18] log state name that threw exception --- .../engine/internal/executor/WorkflowStateProcessor.java | 9 +++++---- .../engine/workflow/definition/ExceptionSeverity.java | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index 382fcb6d8..3ae124c59 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -202,15 +202,16 @@ private void runImpl() { private void logRetryableException(WorkflowState state, Throwable t) { ExceptionSeverity exceptionSeverity = state.getExceptionSeverity(t); - BiConsumer logMethod = getLogMethod(exceptionSeverity.logLevel); + BiConsumer logMethod = getLogMethod(exceptionSeverity.logLevel); if (exceptionSeverity.logStackTrace) { - logMethod.accept("Handler threw a retryable exception, trying again later.", t); + logMethod.accept("Handling state '{}' threw a retryable exception, trying again later.", new Object[] { state.name(), t }); } else { - logMethod.accept("Handler threw a retryable exception, trying again later. Message: {}", t.getMessage()); + logMethod.accept("Handling state '{}' threw a retryable exception, trying again later. Message: {}", + new Object[] { state.name(), t.getMessage() }); } } - private BiConsumer getLogMethod(Level logLevel) { + private BiConsumer getLogMethod(Level logLevel) { switch (logLevel) { case TRACE: return logger::trace; diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java index 01cc2b849..101888088 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java @@ -5,7 +5,7 @@ import org.slf4j.event.Level; public class ExceptionSeverity { - public static final ExceptionSeverity DEFAULT = new ExceptionSeverity(ERROR, true); + public static final ExceptionSeverity DEFAULT = new ExceptionSeverity.Builder().build(); public final Level logLevel; public final boolean logStackTrace; From f5c3f4288c0da9d6581fa09657204d786d5231ca Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 1 Feb 2021 19:16:33 +0200 Subject: [PATCH 04/18] v2 --- .../internal/executor/WorkflowStateProcessor.java | 6 +++--- .../workflow/definition/WorkflowSettings.java | 13 +++++++++++++ .../engine/workflow/definition/WorkflowState.java | 11 ----------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index 3ae124c59..0dba1d15b 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -175,7 +175,7 @@ private void runImpl() { } execution.setFailed(t); if (state.isRetryAllowed(t)) { - logRetryableException(state, t); + logRetryableException(settings, state, t); execution.setRetry(true); execution.setNextState(state); execution.setNextStateReason(getStackTrace(t)); @@ -200,8 +200,8 @@ private void runImpl() { logger.debug("Finished."); } - private void logRetryableException(WorkflowState state, Throwable t) { - ExceptionSeverity exceptionSeverity = state.getExceptionSeverity(t); + private void logRetryableException(WorkflowSettings settings, WorkflowState state, Throwable t) { + ExceptionSeverity exceptionSeverity = settings.getExceptionSeverity(state, t); BiConsumer logMethod = getLogMethod(exceptionSeverity.logLevel); if (exceptionSeverity.logStackTrace) { logMethod.accept("Handling state '{}' threw a retryable exception, trying again later.", new Object[] { state.name(), t }); diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java index 0d3861860..36a8c5861 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java @@ -13,6 +13,7 @@ import java.util.Map; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.BiFunction; import java.util.function.BooleanSupplier; import org.joda.time.DateTime; @@ -68,6 +69,7 @@ public class WorkflowSettings extends ModelObject { * Default priority for new workflow instances. */ public final short defaultPriority; + public final BiFunction exceptionSeveritySupplier; WorkflowSettings(Builder builder) { this.minErrorTransitionDelay = builder.minErrorTransitionDelay; @@ -80,6 +82,7 @@ public class WorkflowSettings extends ModelObject { this.historyDeletableAfter = builder.historyDeletableAfter; this.deleteHistoryCondition = builder.deleteHistoryCondition; this.defaultPriority = builder.defaultPriority; + this.exceptionSeveritySupplier = builder.exceptionSeveritySupplier; } /** @@ -98,6 +101,7 @@ public static class Builder { ReadablePeriod historyDeletableAfter; short defaultPriority = 0; BooleanSupplier deleteHistoryCondition = onAverageEveryNthExecution(100); + BiFunction exceptionSeveritySupplier = (s, t) -> ExceptionSeverity.DEFAULT; /** * Returns true randomly every n:th time. @@ -251,6 +255,12 @@ public Builder setDefaultPriority(short defaultPriority) { return this; } + public Builder setExceptionSeveritySupplier( + BiFunction exceptionSeveritySupplier) { + this.exceptionSeveritySupplier = exceptionSeveritySupplier; + return this; + } + /** * Create workflow settings object. * @@ -331,4 +341,7 @@ public Short getDefaultPriority() { return defaultPriority; } + public ExceptionSeverity getExceptionSeverity(WorkflowState state, Throwable t) { + return exceptionSeveritySupplier.apply(state, t); + } } diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java index 8fdc18ce8..9a7c82d53 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java @@ -39,15 +39,4 @@ default String getDescription() { default boolean isRetryAllowed(Throwable thrown) { return !thrown.getClass().isAnnotationPresent(NonRetryable.class); } - - /** - * Return the severity of the exception thrown by the state execution. Using default means ERROR level logging with stack trace. - * - * @param thrown - * The thrown exception. - * @return Exception severity. - */ - default ExceptionSeverity getExceptionSeverity(Throwable thrown) { - return ExceptionSeverity.DEFAULT; - } } From 12d38ab055026be900cd8cea8685b76e48188aa4 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 1 Feb 2021 19:28:23 +0200 Subject: [PATCH 05/18] refactor --- .../executor/WorkflowStateProcessor.java | 9 +++++---- .../workflow/definition/WorkflowSettings.java | 17 ++++++----------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index 0dba1d15b..ffb0fe1ee 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -200,14 +200,15 @@ private void runImpl() { logger.debug("Finished."); } - private void logRetryableException(WorkflowSettings settings, WorkflowState state, Throwable t) { - ExceptionSeverity exceptionSeverity = settings.getExceptionSeverity(state, t); + private void logRetryableException(WorkflowSettings settings, WorkflowState state, Throwable thrown) { + ExceptionSeverity exceptionSeverity = settings.exceptionSeverityResolver.apply(thrown); BiConsumer logMethod = getLogMethod(exceptionSeverity.logLevel); if (exceptionSeverity.logStackTrace) { - logMethod.accept("Handling state '{}' threw a retryable exception, trying again later.", new Object[] { state.name(), t }); + logMethod.accept("Handling state '{}' threw a retryable exception, trying again later.", + new Object[] { state.name(), thrown }); } else { logMethod.accept("Handling state '{}' threw a retryable exception, trying again later. Message: {}", - new Object[] { state.name(), t.getMessage() }); + new Object[] { state.name(), thrown.getMessage() }); } } diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java index 36a8c5861..c5b9a5be5 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java @@ -13,8 +13,8 @@ import java.util.Map; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicLong; -import java.util.function.BiFunction; import java.util.function.BooleanSupplier; +import java.util.function.Function; import org.joda.time.DateTime; import org.joda.time.LocalDateTime; @@ -69,7 +69,7 @@ public class WorkflowSettings extends ModelObject { * Default priority for new workflow instances. */ public final short defaultPriority; - public final BiFunction exceptionSeveritySupplier; + public final Function exceptionSeverityResolver; WorkflowSettings(Builder builder) { this.minErrorTransitionDelay = builder.minErrorTransitionDelay; @@ -82,7 +82,7 @@ public class WorkflowSettings extends ModelObject { this.historyDeletableAfter = builder.historyDeletableAfter; this.deleteHistoryCondition = builder.deleteHistoryCondition; this.defaultPriority = builder.defaultPriority; - this.exceptionSeveritySupplier = builder.exceptionSeveritySupplier; + this.exceptionSeverityResolver = builder.exceptionSeverityResolver; } /** @@ -101,7 +101,7 @@ public static class Builder { ReadablePeriod historyDeletableAfter; short defaultPriority = 0; BooleanSupplier deleteHistoryCondition = onAverageEveryNthExecution(100); - BiFunction exceptionSeveritySupplier = (s, t) -> ExceptionSeverity.DEFAULT; + Function exceptionSeverityResolver = thrown -> ExceptionSeverity.DEFAULT; /** * Returns true randomly every n:th time. @@ -255,9 +255,8 @@ public Builder setDefaultPriority(short defaultPriority) { return this; } - public Builder setExceptionSeveritySupplier( - BiFunction exceptionSeveritySupplier) { - this.exceptionSeveritySupplier = exceptionSeveritySupplier; + public Builder setExceptionSeverityResolver(Function exceptionSeverityResolver) { + this.exceptionSeverityResolver = exceptionSeverityResolver; return this; } @@ -340,8 +339,4 @@ public boolean deleteWorkflowInstanceHistory() { public Short getDefaultPriority() { return defaultPriority; } - - public ExceptionSeverity getExceptionSeverity(WorkflowState state, Throwable t) { - return exceptionSeveritySupplier.apply(state, t); - } } From c99dd4d620e94916f9e76729795effaa046246a4 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 1 Feb 2021 21:07:10 +0200 Subject: [PATCH 06/18] v3 --- .../executor/WorkflowStateProcessor.java | 31 +++++++------- .../definition/ExceptionHandling.java | 42 +++++++++++++++++++ .../definition/ExceptionSeverity.java | 33 --------------- .../workflow/definition/WorkflowSettings.java | 16 ++++--- .../workflow/definition/WorkflowState.java | 3 ++ .../executor/WorkflowStateProcessorTest.java | 9 ++-- .../definition/WorkflowStateTest.java | 17 -------- .../tests/demo/workflow/NoRetryWorkflow.java | 24 +++++------ 8 files changed, 84 insertions(+), 91 deletions(-) create mode 100644 nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java delete mode 100644 nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index ffb0fe1ee..c10b4adaa 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -51,7 +51,7 @@ import io.nflow.engine.service.WorkflowDefinitionService; import io.nflow.engine.service.WorkflowInstanceService; import io.nflow.engine.workflow.definition.AbstractWorkflowDefinition; -import io.nflow.engine.workflow.definition.ExceptionSeverity; +import io.nflow.engine.workflow.definition.ExceptionHandling; import io.nflow.engine.workflow.definition.NextAction; import io.nflow.engine.workflow.definition.WorkflowSettings; import io.nflow.engine.workflow.definition.WorkflowState; @@ -169,19 +169,20 @@ private void runImpl() { } catch (StateVariableValueTooLongException e) { instance = rescheduleStateVariableValueTooLong(e, instance); saveInstanceState = false; - } catch (Throwable t) { - if (t instanceof UndeclaredThrowableException) { - t = t.getCause(); + } catch (Throwable thrown) { + if (thrown instanceof UndeclaredThrowableException) { + thrown = thrown.getCause(); } - execution.setFailed(t); - if (state.isRetryAllowed(t)) { - logRetryableException(settings, state, t); + execution.setFailed(thrown); + ExceptionHandling exceptionHandling = settings.exceptionAnalyzer.apply(state, thrown); + if (exceptionHandling.isRetryable) { + logRetryableException(exceptionHandling, state.name(), thrown); execution.setRetry(true); execution.setNextState(state); - execution.setNextStateReason(getStackTrace(t)); + execution.setNextStateReason(getStackTrace(thrown)); execution.handleRetryAfter(definition.getSettings().getErrorTransitionActivation(execution.getRetries()), definition); } else { - logger.error("Handler threw an exception and retrying is not allowed, going to failure state.", t); + logger.error("Handler threw an exception and retrying is not allowed, going to failure state.", thrown); execution.handleFailure(definition, "Handler threw an exception and retrying is not allowed"); } } finally { @@ -200,15 +201,13 @@ private void runImpl() { logger.debug("Finished."); } - private void logRetryableException(WorkflowSettings settings, WorkflowState state, Throwable thrown) { - ExceptionSeverity exceptionSeverity = settings.exceptionSeverityResolver.apply(thrown); - BiConsumer logMethod = getLogMethod(exceptionSeverity.logLevel); - if (exceptionSeverity.logStackTrace) { - logMethod.accept("Handling state '{}' threw a retryable exception, trying again later.", - new Object[] { state.name(), thrown }); + private void logRetryableException(ExceptionHandling exceptionHandling, String state, Throwable thrown) { + BiConsumer logMethod = getLogMethod(exceptionHandling.logLevel); + if (exceptionHandling.logStackTrace) { + logMethod.accept("Handling state '{}' threw a retryable exception, trying again later.", new Object[] { state, thrown }); } else { logMethod.accept("Handling state '{}' threw a retryable exception, trying again later. Message: {}", - new Object[] { state.name(), thrown.getMessage() }); + new Object[] { state, thrown.getMessage() }); } } diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java new file mode 100644 index 000000000..5dd48470a --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java @@ -0,0 +1,42 @@ +package io.nflow.engine.workflow.definition; + +import static org.slf4j.event.Level.ERROR; + +import org.slf4j.event.Level; + +public class ExceptionHandling { + public final boolean isRetryable; + public final Level logLevel; + public final boolean logStackTrace; + + ExceptionHandling(boolean isRetryable, Level logLevel, boolean logStackTrace) { + this.isRetryable = isRetryable; + this.logLevel = logLevel; + this.logStackTrace = logStackTrace; + } + + public static class Builder { + private boolean isRetryable = false; + private Level logLevel = ERROR; + private boolean logStackTrace = true; + + public Builder setRetryable(boolean isRetryable) { + this.isRetryable = isRetryable; + return this; + } + + public Builder setLogLevel(Level logLevel) { + this.logLevel = logLevel; + return this; + } + + public Builder setLogStackTrace(boolean logStackTrace) { + this.logStackTrace = logStackTrace; + return this; + } + + public ExceptionHandling build() { + return new ExceptionHandling(isRetryable, logLevel, logStackTrace); + } + } +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java deleted file mode 100644 index 101888088..000000000 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionSeverity.java +++ /dev/null @@ -1,33 +0,0 @@ -package io.nflow.engine.workflow.definition; - -import static org.slf4j.event.Level.ERROR; - -import org.slf4j.event.Level; - -public class ExceptionSeverity { - public static final ExceptionSeverity DEFAULT = new ExceptionSeverity.Builder().build(); - public final Level logLevel; - public final boolean logStackTrace; - - ExceptionSeverity(Level logLevel, boolean logStackTrace) { - this.logLevel = logLevel; - this.logStackTrace = logStackTrace; - } - - public static class Builder { - private Level logLevel = ERROR; - private boolean logStackTrace = true; - - public void setLogLevel(Level logLevel) { - this.logLevel = logLevel; - } - - public void setLogStackTrace(boolean logStackTrace) { - this.logStackTrace = logStackTrace; - } - - public ExceptionSeverity build() { - return new ExceptionSeverity(logLevel, logStackTrace); - } - } -} diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java index c5b9a5be5..f9c84e4f2 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java @@ -13,8 +13,8 @@ import java.util.Map; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.BiFunction; import java.util.function.BooleanSupplier; -import java.util.function.Function; import org.joda.time.DateTime; import org.joda.time.LocalDateTime; @@ -69,7 +69,7 @@ public class WorkflowSettings extends ModelObject { * Default priority for new workflow instances. */ public final short defaultPriority; - public final Function exceptionSeverityResolver; + public final BiFunction exceptionAnalyzer; WorkflowSettings(Builder builder) { this.minErrorTransitionDelay = builder.minErrorTransitionDelay; @@ -82,7 +82,7 @@ public class WorkflowSettings extends ModelObject { this.historyDeletableAfter = builder.historyDeletableAfter; this.deleteHistoryCondition = builder.deleteHistoryCondition; this.defaultPriority = builder.defaultPriority; - this.exceptionSeverityResolver = builder.exceptionSeverityResolver; + this.exceptionAnalyzer = builder.exceptionAnalyzer; } /** @@ -101,7 +101,11 @@ public static class Builder { ReadablePeriod historyDeletableAfter; short defaultPriority = 0; BooleanSupplier deleteHistoryCondition = onAverageEveryNthExecution(100); - Function exceptionSeverityResolver = thrown -> ExceptionSeverity.DEFAULT; + // TODO: replace state.isRetryAllowed(thrown) with !thrown.getClass().isAnnotationPresent(NonRetryable.class) in the next + // major release + @SuppressWarnings("deprecation") + BiFunction exceptionAnalyzer = (state, thrown) -> new ExceptionHandling.Builder() + .setRetryable(state.isRetryAllowed(thrown)).build(); /** * Returns true randomly every n:th time. @@ -255,8 +259,8 @@ public Builder setDefaultPriority(short defaultPriority) { return this; } - public Builder setExceptionSeverityResolver(Function exceptionSeverityResolver) { - this.exceptionSeverityResolver = exceptionSeverityResolver; + public Builder setExceptionAnalyzer(BiFunction exceptionAnalyzer) { + this.exceptionAnalyzer = exceptionAnalyzer; return this; } diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java index 9a7c82d53..56d4d1236 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowState.java @@ -35,7 +35,10 @@ default String getDescription() { * @param thrown * The thrown exception. * @return True if the state can be retried. + * @deprecated This will be removed in the next major release. Use new WorkflowSettings.Builder().setExceptionAnalyzer(...) + * instead. */ + @Deprecated default boolean isRetryAllowed(Throwable thrown) { return !thrown.getClass().isAnnotationPresent(NonRetryable.class); } diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java index fee18fab8..ee169844b 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java @@ -96,6 +96,7 @@ import io.nflow.engine.service.WorkflowInstanceInclude; import io.nflow.engine.service.WorkflowInstanceService; import io.nflow.engine.workflow.curated.BulkWorkflow; +import io.nflow.engine.workflow.definition.ExceptionHandling; import io.nflow.engine.workflow.definition.Mutable; import io.nflow.engine.workflow.definition.NextAction; import io.nflow.engine.workflow.definition.StateExecution; @@ -1290,7 +1291,8 @@ public NextAction start(StateExecution execution) { public static class NonRetryableWorkflow extends WorkflowDefinition { protected NonRetryableWorkflow() { - super("non-retryable", State.start, State.end); + super("non-retryable", State.start, State.end, new WorkflowSettings.Builder() + .setExceptionAnalyzer((s, t) -> new ExceptionHandling.Builder().setRetryable(false).build()).build()); } public static enum State implements WorkflowState { @@ -1306,11 +1308,6 @@ private State(WorkflowStateType stateType) { public WorkflowStateType getType() { return stateType; } - - @Override - public boolean isRetryAllowed(Throwable thrown) { - return false; - } } public NextAction start(@SuppressWarnings("unused") StateExecution execution) { diff --git a/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowStateTest.java b/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowStateTest.java index 93c2c0f80..7da649f06 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowStateTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowStateTest.java @@ -2,8 +2,6 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import org.junit.jupiter.api.Test; @@ -16,16 +14,6 @@ public void getDescriptionReturnsNameByDefault() { assertThat(state.getDescription(), is(state.name())); } - @Test - public void isRetryAllowedReturnsFalseWhenThrowableIsAnnotatedWithNonRetryable() { - assertFalse(state.isRetryAllowed(new NonRetryableException())); - } - - @Test - public void isRetryAllowedReturnsTrueWhenThrowableIsNotAnnotatedWithNonRetryable() { - assertTrue(state.isRetryAllowed(new RuntimeException())); - } - static class TestWorkflowState implements WorkflowState { @Override @@ -38,9 +26,4 @@ public WorkflowStateType getType() { return WorkflowStateType.normal; } } - - @NonRetryable - static class NonRetryableException extends RuntimeException { - private static final long serialVersionUID = 1L; - } } diff --git a/nflow-tests/src/main/java/io/nflow/tests/demo/workflow/NoRetryWorkflow.java b/nflow-tests/src/main/java/io/nflow/tests/demo/workflow/NoRetryWorkflow.java index ca011729e..59f890313 100644 --- a/nflow-tests/src/main/java/io/nflow/tests/demo/workflow/NoRetryWorkflow.java +++ b/nflow-tests/src/main/java/io/nflow/tests/demo/workflow/NoRetryWorkflow.java @@ -5,12 +5,16 @@ import static io.nflow.engine.workflow.definition.WorkflowStateType.normal; import static io.nflow.engine.workflow.definition.WorkflowStateType.start; +import java.util.function.BiFunction; + import org.springframework.stereotype.Component; +import io.nflow.engine.workflow.definition.ExceptionHandling; import io.nflow.engine.workflow.definition.NextAction; import io.nflow.engine.workflow.definition.NonRetryable; import io.nflow.engine.workflow.definition.StateExecution; import io.nflow.engine.workflow.definition.WorkflowDefinition; +import io.nflow.engine.workflow.definition.WorkflowSettings; import io.nflow.engine.workflow.definition.WorkflowState; import io.nflow.engine.workflow.definition.WorkflowStateType; import io.nflow.tests.demo.workflow.NoRetryWorkflow.State; @@ -21,23 +25,17 @@ public class NoRetryWorkflow extends WorkflowDefinition { public static final String TYPE = "noRetry"; public static enum State implements WorkflowState { - begin(start, "Retry always disabled for this state", false), // + begin(start, "Retry always disabled for this state"), // process(normal, "Retry disabled for exceptions annotated with @NonRetryable"), // done(end, "End state"), // error(manual, "Error state"); private WorkflowStateType type; private String description; - private boolean isRetryAllowed; private State(WorkflowStateType type, String description) { - this(type, description, true); - } - - private State(WorkflowStateType type, String description, boolean isRetryAllowed) { this.type = type; this.description = description; - this.isRetryAllowed = isRetryAllowed; } @Override @@ -49,20 +47,20 @@ public WorkflowStateType getType() { public String getDescription() { return description; } - - @Override - public boolean isRetryAllowed(Throwable thrown) { - return isRetryAllowed && WorkflowState.super.isRetryAllowed(thrown); - } } public NoRetryWorkflow() { - super(TYPE, State.begin, State.error); + super(TYPE, State.begin, State.error, new WorkflowSettings.Builder().setExceptionAnalyzer(exceptionAnalyzer()).build()); setDescription("Workflow demonstrating how to avoid automatic retry"); permit(State.begin, State.process); permit(State.process, State.done); } + private static BiFunction exceptionAnalyzer() { + return (s, t) -> new ExceptionHandling.Builder() + .setRetryable(s != State.begin && !t.getClass().isAnnotationPresent(NonRetryable.class)).build(); + } + public NextAction begin(@SuppressWarnings("unused") StateExecution execution) { throw new RuntimeException(); } From a77e06cbe92accabb9f0c70fe5caaadd9facca44 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 1 Feb 2021 21:22:24 +0200 Subject: [PATCH 07/18] exceptions are retryable by default, add tests --- CHANGELOG.md | 5 +++ .../definition/ExceptionHandling.java | 2 +- .../workflow/definition/WorkflowSettings.java | 2 +- .../definition/WorkflowSettingsTest.java | 43 ++++++++++++++++--- 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0989226d6..93c814a00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,16 @@ - Support for searching workflow instances by state variable key and value. - Added optional properties for tuning parts of nFlow Explorer - Facelift for workflow instance properties in nFlow Explorer +- Control retryable exception handling via `WorkflowSettings` (replaces deprecated `WorkflowState.isRetryAllowed(...)`) **Details** - `nflow-engine` - `WorkflowInstanceService.updateWorkflowInstance` can now be used to update business key of the workflow instance. - Use `QueryWorkflowInstances.setStateVariable` to limit search query by state variable name and key. Only the latest value of the state variable of the workflow instance is used. + - Control retryable exception handling via `WorkflowSettings.Builder.setExceptionAnalyzer(...)` / `ExceptionHandling`: + - Allow setting the log entry level of the retryable exception thrown by state processing. + - Allow controlling whether the stack trace of the retryable exception is logged or not. + - Allow controlling whether the exception is retryable or not (replaces deprecated `WorkflowState.isRetryAllowed(...)`). - `nflow-rest-api-common`, `nflow-rest-api-jax-rs`, `nflow-rest-api-spring-web` - `UpdateWorkflowInstanceRequest.businessKey` field was added to support updating workflow instance business key via REST API. - Added support for new query parameters `stateVariableKey` and `stateVariableValue` to `GET /v1/workflow-instance` to limit search query by state variable name and key. Only the latest value of the state variable of the workflow instance is used. diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java index 5dd48470a..e6d6743fc 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java @@ -16,7 +16,7 @@ public class ExceptionHandling { } public static class Builder { - private boolean isRetryable = false; + private boolean isRetryable = true; private Level logLevel = ERROR; private boolean logStackTrace = true; diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java index f9c84e4f2..a99af937a 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java @@ -88,7 +88,6 @@ public class WorkflowSettings extends ModelObject { /** * Builder for workflow settings. */ - @SuppressFBWarnings(value = "MDM_RANDOM_SEED", justification = "Random does not need to be secure") public static class Builder { int maxErrorTransitionDelay = (int) DAYS.toMillis(1); @@ -113,6 +112,7 @@ public static class Builder { * @param n The frequency of returning true. * @return Producer of boolean values */ + @SuppressFBWarnings(value = "MDM_RANDOM_SEED", justification = "Random does not need to be secure here") public static BooleanSupplier onAverageEveryNthExecution(int n) { return () -> ThreadLocalRandom.current().nextInt(n) == 0; } diff --git a/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java b/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java index d4995e0d5..8879e3517 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java @@ -16,6 +16,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.slf4j.event.Level; + public class WorkflowSettingsTest { DateTime now = new DateTime(2014, 10, 22, 20, 44, 0); @@ -46,12 +48,13 @@ public void verifyConstantDefaultValues() { public void errorTransitionDelayIsBetweenMinAndMaxDelay() { int maxDelay = 1_000_000; int minDelay = 1000; - WorkflowSettings s = new WorkflowSettings.Builder().setMinErrorTransitionDelay(minDelay).setMaxErrorTransitionDelay(maxDelay).build(); + WorkflowSettings s = new WorkflowSettings.Builder().setMinErrorTransitionDelay(minDelay).setMaxErrorTransitionDelay(maxDelay) + .build(); long prevDelay = 0; - for(int retryCount = 0 ; retryCount < 100 ; retryCount++) { - long delay = s.getErrorTransitionActivation(retryCount).getMillis() - now.getMillis(); - assertThat(delay, greaterThanOrEqualTo((long)minDelay)); - assertThat(delay, lessThanOrEqualTo((long)maxDelay)); + for (int retryCount = 0; retryCount < 100; retryCount++) { + long delay = s.getErrorTransitionActivation(retryCount).getMillis() - now.getMillis(); + assertThat(delay, greaterThanOrEqualTo((long) minDelay)); + assertThat(delay, lessThanOrEqualTo((long) maxDelay)); assertThat(delay, greaterThanOrEqualTo(prevDelay)); prevDelay = delay; } @@ -101,4 +104,34 @@ public void oncePerDaySupplierWorks() { assertThat(supplier.getAsBoolean(), is(false)); } + @Test + public void defaultExceptionAnalyzer() { + WorkflowSettings s = new WorkflowSettings.Builder().build(); + + ExceptionHandling exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, new Throwable()); + assertThat(exceptionHandling.isRetryable, is(true)); + assertThat(exceptionHandling.logLevel, is(Level.ERROR)); + assertThat(exceptionHandling.logStackTrace, is(true)); + + exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, new NonRetryableException()); + assertThat(exceptionHandling.isRetryable, is(false)); + assertThat(exceptionHandling.logLevel, is(Level.ERROR)); + assertThat(exceptionHandling.logStackTrace, is(true)); + } + + @Test + public void customExceptionAnalyzer() { + WorkflowSettings s = new WorkflowSettings.Builder().setExceptionAnalyzer((state, thrown) -> new ExceptionHandling.Builder() + .setLogLevel(Level.INFO).setRetryable(true).setLogStackTrace(false).build()).build(); + + ExceptionHandling exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, new NonRetryableException()); + assertThat(exceptionHandling.isRetryable, is(true)); + assertThat(exceptionHandling.logLevel, is(Level.INFO)); + assertThat(exceptionHandling.logStackTrace, is(false)); + } + + @NonRetryable + static class NonRetryableException extends RuntimeException { + private static final long serialVersionUID = 1L; + } } From bb6b560084773c5935ebbed9bc1cc2480cdd1ee4 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Mon, 1 Feb 2021 23:49:54 +0200 Subject: [PATCH 08/18] add javadocs --- .../definition/ExceptionHandling.java | 41 +++++++++++++++++++ .../workflow/definition/WorkflowSettings.java | 10 +++++ 2 files changed, 51 insertions(+) diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java index e6d6743fc..11eeacd3d 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java @@ -4,9 +4,21 @@ import org.slf4j.event.Level; +/** + * Controls how an exception thrown by a state method should be handled by the workflow state processor. + */ public class ExceptionHandling { + /** + * True when the state method processing should be retried. + */ public final boolean isRetryable; + /** + * The log entry level for logging the exception. + */ public final Level logLevel; + /** + * True when the exception stack trace of the exception should be logged. False to log only exception message. + */ public final boolean logStackTrace; ExceptionHandling(boolean isRetryable, Level logLevel, boolean logStackTrace) { @@ -15,26 +27,55 @@ public class ExceptionHandling { this.logStackTrace = logStackTrace; } + /** + * Builder for exception handling settings. + */ public static class Builder { private boolean isRetryable = true; private Level logLevel = ERROR; private boolean logStackTrace = true; + /** + * Set if state method processing is retryable or not. + * + * @param isRetryable + * True is state method processing should be retried. + * @return This. + */ public Builder setRetryable(boolean isRetryable) { this.isRetryable = isRetryable; return this; } + /** + * Set the log entry level. + * + * @param logLevel + * The log entry level. + * @return This. + */ public Builder setLogLevel(Level logLevel) { this.logLevel = logLevel; return this; } + /** + * Set if exception stack trace should be logged or not. + * + * @param logStackTrace + * True to log the exception stack trace, false to log the exception message only. + * @return This. + */ public Builder setLogStackTrace(boolean logStackTrace) { this.logStackTrace = logStackTrace; return this; } + /** + * Create the exception handling object. + * + * @return Exception handling. + */ public ExceptionHandling build() { return new ExceptionHandling(isRetryable, logLevel, logStackTrace); } diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java index a99af937a..d5e2cb9d5 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java @@ -69,6 +69,9 @@ public class WorkflowSettings extends ModelObject { * Default priority for new workflow instances. */ public final short defaultPriority; + /** + * Exception analyzer controls how an exception thrown by a state method should be handled. + */ public final BiFunction exceptionAnalyzer; WorkflowSettings(Builder builder) { @@ -259,6 +262,13 @@ public Builder setDefaultPriority(short defaultPriority) { return this; } + /** + * Set the exception analyzer function. + * + * @param exceptionAnalyzer + * The exception analyzer function. + * @return this. + */ public Builder setExceptionAnalyzer(BiFunction exceptionAnalyzer) { this.exceptionAnalyzer = exceptionAnalyzer; return this; From 5ff3eedc092e5be0b3cc3df629acea47a2601702 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Tue, 2 Feb 2021 16:42:36 +0200 Subject: [PATCH 09/18] update changelog --- CHANGELOG.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93c814a00..2e2f15db5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,15 +6,16 @@ - Added optional properties for tuning parts of nFlow Explorer - Facelift for workflow instance properties in nFlow Explorer - Control retryable exception handling via `WorkflowSettings` (replaces deprecated `WorkflowState.isRetryAllowed(...)`) +- Control retrying and logging of an exception thrown by a state method via `WorkflowSettings` (replaces deprecated `WorkflowState.isRetryAllowed(...)`) **Details** - `nflow-engine` - `WorkflowInstanceService.updateWorkflowInstance` can now be used to update business key of the workflow instance. - Use `QueryWorkflowInstances.setStateVariable` to limit search query by state variable name and key. Only the latest value of the state variable of the workflow instance is used. - - Control retryable exception handling via `WorkflowSettings.Builder.setExceptionAnalyzer(...)` / `ExceptionHandling`: - - Allow setting the log entry level of the retryable exception thrown by state processing. - - Allow controlling whether the stack trace of the retryable exception is logged or not. - - Allow controlling whether the exception is retryable or not (replaces deprecated `WorkflowState.isRetryAllowed(...)`). + - Control retrying and logging of an exception thrown by a state method via `WorkflowSettings.Builder.setExceptionAnalyzer(...)` / `ExceptionHandling`: + - Control whether the exception is considered retryable or not (replaces deprecated `WorkflowState.isRetryAllowed(...)`). + - Control which log level is used to log the retryable exception. + - Control whether the stack trace of the retryable exception is logged or not. - `nflow-rest-api-common`, `nflow-rest-api-jax-rs`, `nflow-rest-api-spring-web` - `UpdateWorkflowInstanceRequest.businessKey` field was added to support updating workflow instance business key via REST API. - Added support for new query parameters `stateVariableKey` and `stateVariableValue` to `GET /v1/workflow-instance` to limit search query by state variable name and key. Only the latest value of the state variable of the workflow instance is used. From f9245ffec373ca842fb18198da8a79a088dba29f Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Fri, 5 Feb 2021 21:09:31 +0200 Subject: [PATCH 10/18] add support for customizing exception handling in dispatcher --- CHANGELOG.md | 8 ++ .../DefaultDispatcherExceptionAnalyzer.java | 25 ++++ .../executor/DispatcherExceptionAnalyzer.java | 5 + .../executor/DispatcherExceptionHandling.java | 119 ++++++++++++++++++ .../internal/executor/WorkflowDispatcher.java | 32 +++-- .../executor/WorkflowStateProcessor.java | 31 ++--- .../WorkflowStateProcessorFactory.java | 7 +- .../engine/internal/util/NflowLogger.java | 31 +++++ .../definition/ExceptionHandling.java | 2 +- ...efaultDispatcherExceptionAnalyzerTest.java | 56 +++++++++ .../executor/WorkflowDispatcherTest.java | 28 +++-- .../WorkflowStateProcessorFactoryTest.java | 4 +- .../executor/WorkflowStateProcessorTest.java | 13 +- 13 files changed, 310 insertions(+), 51 deletions(-) create mode 100644 nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java create mode 100644 nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java create mode 100644 nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionHandling.java create mode 100644 nflow-engine/src/main/java/io/nflow/engine/internal/util/NflowLogger.java create mode 100644 nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e2f15db5..7c8324442 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ - Facelift for workflow instance properties in nFlow Explorer - Control retryable exception handling via `WorkflowSettings` (replaces deprecated `WorkflowState.isRetryAllowed(...)`) - Control retrying and logging of an exception thrown by a state method via `WorkflowSettings` (replaces deprecated `WorkflowState.isRetryAllowed(...)`) +- Control retrying and logging of an exception thrown by a state method via `WorkflowSettings` (replaces deprecated `WorkflowState.isRetryAllowed(...)`). +- Control logging and sleeping after exceptions in `WorkflowDispatcher`. **Details** - `nflow-engine` @@ -16,6 +18,12 @@ - Control whether the exception is considered retryable or not (replaces deprecated `WorkflowState.isRetryAllowed(...)`). - Control which log level is used to log the retryable exception. - Control whether the stack trace of the retryable exception is logged or not. + - Control logging and sleeping after exceptions in `WorkflowDispatcher` by providing a `DispatcherExceptionHandler` that returns `DispatcherExceptionHandling` objects: + - Control whether the exception is logged or not. + - Control which log level is used to log the exception. + - Control whether the stack trace of the exception is logged or not. + - Control whether dispatcher should sleep after the exception or not. + - Control whether the sleep time should be randomized or not. - `nflow-rest-api-common`, `nflow-rest-api-jax-rs`, `nflow-rest-api-spring-web` - `UpdateWorkflowInstanceRequest.businessKey` field was added to support updating workflow instance business key via REST API. - Added support for new query parameters `stateVariableKey` and `stateVariableValue` to `GET /v1/workflow-instance` to limit search query by state variable name and key. Only the latest value of the state variable of the workflow instance is used. diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java new file mode 100644 index 000000000..7b2dea398 --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java @@ -0,0 +1,25 @@ +package io.nflow.engine.internal.executor; + +import org.slf4j.event.Level; +import org.springframework.stereotype.Component; + +import io.nflow.engine.internal.dao.PollingBatchException; +import io.nflow.engine.internal.dao.PollingRaceConditionException; +import io.nflow.engine.internal.executor.DispatcherExceptionHandling.Builder; + +@Component +public class DefaultDispatcherExceptionAnalyzer implements DispatcherExceptionAnalyzer { + + @Override + public DispatcherExceptionHandling analyze(Exception e) { + Builder builder = new DispatcherExceptionHandling.Builder(); + if (e instanceof PollingRaceConditionException) { + builder.setLogLevel(Level.DEBUG).setLogStackTrace(false).setRandomizeSleep(true); + } else if (e instanceof PollingBatchException) { + builder.setLogLevel(Level.WARN).setLogStackTrace(false).setSleep(false); + } else if (e instanceof InterruptedException) { + builder.setLog(false).setSleep(false); + } + return builder.build(); + } +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java new file mode 100644 index 000000000..a17b95232 --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java @@ -0,0 +1,5 @@ +package io.nflow.engine.internal.executor; + +public interface DispatcherExceptionAnalyzer { + DispatcherExceptionHandling analyze(Exception e); +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionHandling.java new file mode 100644 index 000000000..1a09ed003 --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionHandling.java @@ -0,0 +1,119 @@ +package io.nflow.engine.internal.executor; + +import static org.slf4j.event.Level.ERROR; + +import org.slf4j.event.Level; + +/** + * Controls how an exception should be handled by the dispatcher. + */ +public class DispatcherExceptionHandling { + /** + * True when dispatcher should log the exception. + */ + public final boolean log; + /** + * The log entry level for logging the exception. + */ + public final Level logLevel; + /** + * True when the exception stack trace of the exception should be logged. + */ + public final boolean logStackTrace; + /** + * True when dispatcher should sleep a while after exception. + */ + public final boolean sleep; + /** + * True when the sleep time should be randomized. + */ + public final boolean randomizeSleep; + + DispatcherExceptionHandling(boolean log, Level logLevel, boolean logStackTrace, boolean sleep, boolean randomizeSleep) { + this.log = log; + this.logLevel = logLevel; + this.logStackTrace = logStackTrace; + this.sleep = sleep; + this.randomizeSleep = randomizeSleep; + } + + /** + * Builder for exception handling settings. + */ + public static class Builder { + private boolean log = true; + private Level logLevel = ERROR; + private boolean logStackTrace = true; + private boolean sleep = true; + private boolean randomizeSleep = false; + + /** + * Set if dispatcher should log the exception or not. + * + * @param log + * True if dispatcher should log the exception. + * @return This. + */ + public Builder setLog(boolean log) { + this.log = log; + return this; + } + + /** + * Set the log entry level. + * + * @param logLevel + * The log entry level. + * @return This. + */ + public Builder setLogLevel(Level logLevel) { + this.logLevel = logLevel; + return this; + } + + /** + * Set if exception stack trace should be logged or not. + * + * @param logStackTrace + * True to log the exception stack trace. + * @return This. + */ + public Builder setLogStackTrace(boolean logStackTrace) { + this.logStackTrace = logStackTrace; + return this; + } + + /** + * Set if dispatcher should sleep a while after exception or not. + * + * @param sleep + * True if dispatcher should sleep a while after exception. + * @return This. + */ + public Builder setSleep(boolean sleep) { + this.sleep = sleep; + return this; + } + + /** + * Set if sleep time should be randomized or not. + * + * @param randomizeSleep + * True if sleep time should be randomized. + * @return This. + */ + public Builder setRandomizeSleep(boolean randomizeSleep) { + this.randomizeSleep = randomizeSleep; + return this; + } + + /** + * Create the dispatcher exception handling object. + * + * @return Dispatcher exception handling. + */ + public DispatcherExceptionHandling build() { + return new DispatcherExceptionHandling(log, logLevel, logStackTrace, sleep, randomizeSleep); + } + } +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java index 28de9d6d6..eea2d7d07 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java @@ -15,9 +15,8 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.nflow.engine.internal.dao.ExecutorDao; -import io.nflow.engine.internal.dao.PollingBatchException; -import io.nflow.engine.internal.dao.PollingRaceConditionException; import io.nflow.engine.internal.dao.WorkflowInstanceDao; +import io.nflow.engine.internal.util.NflowLogger; import io.nflow.engine.internal.util.PeriodicLogger; import io.nflow.engine.service.WorkflowDefinitionService; @@ -38,6 +37,8 @@ public class WorkflowDispatcher implements Runnable { private final WorkflowStateProcessorFactory stateProcessorFactory; private final WorkflowDefinitionService workflowDefinitions; private final ExecutorDao executorDao; + private final DispatcherExceptionAnalyzer exceptionAnalyzer; + private final NflowLogger nflowLogger; private final long sleepTimeMillis; private final int stuckThreadThresholdSeconds; private final Random rand = new Random(); @@ -46,12 +47,14 @@ public class WorkflowDispatcher implements Runnable { @SuppressFBWarnings(value = "WEM_WEAK_EXCEPTION_MESSAGING", justification = "Transaction support exception message is fine") public WorkflowDispatcher(WorkflowInstanceExecutor executor, WorkflowInstanceDao workflowInstances, WorkflowStateProcessorFactory stateProcessorFactory, WorkflowDefinitionService workflowDefinitions, ExecutorDao executorDao, - Environment env) { + DispatcherExceptionAnalyzer exceptionAnalyzer, NflowLogger nflowLogger, Environment env) { this.executor = executor; this.workflowInstances = workflowInstances; this.stateProcessorFactory = stateProcessorFactory; this.workflowDefinitions = workflowDefinitions; this.executorDao = executorDao; + this.exceptionAnalyzer = exceptionAnalyzer; + this.nflowLogger = nflowLogger; this.sleepTimeMillis = env.getRequiredProperty("nflow.dispatcher.sleep.ms", Long.class); this.stuckThreadThresholdSeconds = env.getRequiredProperty("nflow.executor.stuckThreadThreshold.seconds", Integer.class); @@ -86,15 +89,22 @@ public void run() { } dispatch(getNextInstanceIds()); } - } catch (PollingRaceConditionException pex) { - logger.debug(pex.getMessage()); - sleep(true); - } catch (PollingBatchException pex) { - logger.warn(pex.getMessage()); - } catch (@SuppressWarnings("unused") InterruptedException dropThrough) { } catch (Exception e) { - logger.error("Exception in executing dispatcher - retrying after sleep period ({})", e.getMessage(), e); - sleep(false); + DispatcherExceptionHandling handling = exceptionAnalyzer.analyze(e); + if (handling.log) { + if (handling.logStackTrace) { + StringBuilder sb = new StringBuilder("Exception in executing dispatcher - retrying"); + if (handling.sleep) { + sb.append(" after sleep period"); + } + nflowLogger.log(logger, handling.logLevel, sb.append(" ({})").toString(), new Object[] { e.getMessage(), e }); + } else { + nflowLogger.log(logger, handling.logLevel, e.getMessage(), new Object[0]); + } + } + if (handling.sleep) { + sleep(handling.randomizeSleep); + } } } } diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index c10b4adaa..2110a2965 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -24,14 +24,12 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.function.BiConsumer; import java.util.function.Supplier; import org.joda.time.DateTime; import org.joda.time.Duration; import org.slf4j.Logger; import org.slf4j.MDC; -import org.slf4j.event.Level; import org.springframework.core.env.Environment; import org.springframework.dao.DataAccessException; import org.springframework.dao.EmptyResultDataAccessException; @@ -40,6 +38,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; +import io.nflow.engine.internal.util.NflowLogger; import io.nflow.engine.internal.util.PeriodicLogger; import io.nflow.engine.internal.workflow.ObjectStringMapper; import io.nflow.engine.internal.workflow.StateExecutionImpl; @@ -86,6 +85,7 @@ class WorkflowStateProcessor implements Runnable { private final int stateSaveRetryDelay; private final int stateVariableValueTooLongRetryDelay; private final Map processingInstances; + private final NflowLogger nflowLogger; private DateTime startTime; private Thread thread; private ListenerContext listenerContext; @@ -94,7 +94,8 @@ class WorkflowStateProcessor implements Runnable { WorkflowDefinitionService workflowDefinitions, WorkflowInstanceService workflowInstances, WorkflowInstanceDao workflowInstanceDao, MaintenanceDao maintenanceDao, WorkflowInstancePreProcessor workflowInstancePreProcessor, Environment env, - Map processingInstances, WorkflowExecutorListener... executorListeners) { + Map processingInstances, NflowLogger nflowLogger, + WorkflowExecutorListener... executorListeners) { this.instanceId = instanceId; this.shutdownRequested = shutdownRequested; this.objectMapper = objectMapper; @@ -103,6 +104,7 @@ class WorkflowStateProcessor implements Runnable { this.workflowInstanceDao = workflowInstanceDao; this.maintenanceDao = maintenanceDao; this.processingInstances = processingInstances; + this.nflowLogger = nflowLogger; this.executorListeners = asList(executorListeners); this.workflowInstancePreProcessor = workflowInstancePreProcessor; illegalStateChangeAction = env.getRequiredProperty("nflow.illegal.state.change.action"); @@ -202,31 +204,16 @@ private void runImpl() { } private void logRetryableException(ExceptionHandling exceptionHandling, String state, Throwable thrown) { - BiConsumer logMethod = getLogMethod(exceptionHandling.logLevel); if (exceptionHandling.logStackTrace) { - logMethod.accept("Handling state '{}' threw a retryable exception, trying again later.", new Object[] { state, thrown }); + nflowLogger.log(logger, exceptionHandling.logLevel, "Handling state '{}' threw a retryable exception, trying again later.", + new Object[] { state, thrown }); } else { - logMethod.accept("Handling state '{}' threw a retryable exception, trying again later. Message: {}", + nflowLogger.log(logger, exceptionHandling.logLevel, + "Handling state '{}' threw a retryable exception, trying again later. Message: {}", new Object[] { state, thrown.getMessage() }); } } - private BiConsumer getLogMethod(Level logLevel) { - switch (logLevel) { - case TRACE: - return logger::trace; - case DEBUG: - return logger::debug; - case INFO: - return logger::info; - case WARN: - return logger::warn; - case ERROR: - default: - return logger::error; - } - } - void logIfLagging(WorkflowInstance instance) { Duration executionLag = new Duration(instance.nextActivation, now()); if (executionLag.isLongerThan(standardMinutes(1))) { diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactory.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactory.java index 5130bd656..1fb4bc1bb 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactory.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactory.java @@ -16,6 +16,7 @@ import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; +import io.nflow.engine.internal.util.NflowLogger; import io.nflow.engine.internal.workflow.ObjectStringMapper; import io.nflow.engine.internal.workflow.WorkflowInstancePreProcessor; import io.nflow.engine.listener.WorkflowExecutorListener; @@ -30,6 +31,7 @@ public class WorkflowStateProcessorFactory { private final WorkflowInstanceDao workflowInstanceDao; private final MaintenanceDao maintenanceDao; private final WorkflowInstancePreProcessor workflowInstancePreProcessor; + private final NflowLogger nflowLogger; private final Environment env; @Autowired(required = false) protected WorkflowExecutorListener[] listeners = new WorkflowExecutorListener[0]; @@ -39,20 +41,21 @@ public class WorkflowStateProcessorFactory { @Inject public WorkflowStateProcessorFactory(WorkflowDefinitionService workflowDefinitions, WorkflowInstanceService workflowInstances, ObjectStringMapper objectMapper, WorkflowInstanceDao workflowInstanceDao, MaintenanceDao maintenanceDao, - WorkflowInstancePreProcessor workflowInstancePreProcessor, Environment env) { + WorkflowInstancePreProcessor workflowInstancePreProcessor, NflowLogger nflowLogger, Environment env) { this.workflowDefinitions = workflowDefinitions; this.workflowInstances = workflowInstances; this.objectMapper = objectMapper; this.workflowInstanceDao = workflowInstanceDao; this.maintenanceDao = maintenanceDao; this.workflowInstancePreProcessor = workflowInstancePreProcessor; + this.nflowLogger = nflowLogger; this.stuckThreadThresholdSeconds = env.getRequiredProperty("nflow.executor.stuckThreadThreshold.seconds", Integer.class); this.env = env; } public WorkflowStateProcessor createProcessor(long instanceId, Supplier shutdownRequested) { return new WorkflowStateProcessor(instanceId, shutdownRequested, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, listeners); + maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listeners); } public int getPotentiallyStuckProcessors() { diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/util/NflowLogger.java b/nflow-engine/src/main/java/io/nflow/engine/internal/util/NflowLogger.java new file mode 100644 index 000000000..f4a0215ca --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/util/NflowLogger.java @@ -0,0 +1,31 @@ +package io.nflow.engine.internal.util; + +import java.util.function.BiConsumer; + +import org.slf4j.Logger; +import org.slf4j.event.Level; +import org.springframework.stereotype.Component; + +@Component +public class NflowLogger { + + public void log(Logger logger, Level level, String message, Object... args) { + getLogMethod(logger, level).accept(message, args); + } + + private BiConsumer getLogMethod(Logger logger, Level level) { + switch (level) { + case TRACE: + return logger::trace; + case DEBUG: + return logger::debug; + case INFO: + return logger::info; + case WARN: + return logger::warn; + case ERROR: + default: + return logger::error; + } + } +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java index 11eeacd3d..143a496ba 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java @@ -39,7 +39,7 @@ public static class Builder { * Set if state method processing is retryable or not. * * @param isRetryable - * True is state method processing should be retried. + * True if state method processing should be retried. * @return This. */ public Builder setRetryable(boolean isRetryable) { diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java new file mode 100644 index 000000000..583155768 --- /dev/null +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java @@ -0,0 +1,56 @@ +package io.nflow.engine.internal.executor; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; +import org.slf4j.event.Level; + +import io.nflow.engine.internal.dao.PollingBatchException; +import io.nflow.engine.internal.dao.PollingRaceConditionException; + +public class DefaultDispatcherExceptionAnalyzerTest { + + DispatcherExceptionAnalyzer analyzer = new DefaultDispatcherExceptionAnalyzer(); + + @Test + void analyzeGenericExceptionReturnsDefaults() { + DispatcherExceptionHandling handling = analyzer.analyze(new Exception()); + + assertTrue(handling.log); + assertEquals(handling.logLevel, Level.ERROR); + assertTrue(handling.logStackTrace); + assertTrue(handling.sleep); + assertFalse(handling.randomizeSleep); + } + + @Test + void analyzePollingRaceConditionException() { + DispatcherExceptionHandling handling = analyzer.analyze(new PollingRaceConditionException("error")); + + assertTrue(handling.log); + assertEquals(handling.logLevel, Level.DEBUG); + assertFalse(handling.logStackTrace); + assertTrue(handling.sleep); + assertTrue(handling.randomizeSleep); + } + + @Test + void analyzePollingBatchException() { + DispatcherExceptionHandling handling = analyzer.analyze(new PollingBatchException("error")); + + assertTrue(handling.log); + assertEquals(handling.logLevel, Level.WARN); + assertFalse(handling.logStackTrace); + assertFalse(handling.sleep); + } + + @Test + void analyzeInterruptedException() { + DispatcherExceptionHandling handling = analyzer.analyze(new InterruptedException()); + + assertFalse(handling.log); + assertFalse(handling.sleep); + } +} diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowDispatcherTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowDispatcherTest.java index d8ff1a66b..18f7828c8 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowDispatcherTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowDispatcherTest.java @@ -15,11 +15,11 @@ import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.quality.Strictness.LENIENT; import static org.slf4j.Logger.ROOT_LOGGER_NAME; import static org.slf4j.LoggerFactory.getLogger; @@ -39,6 +39,7 @@ import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.stubbing.Answer; import org.springframework.beans.factory.BeanCreationException; import org.springframework.mock.env.MockEnvironment; @@ -52,10 +53,12 @@ import edu.umd.cs.mtc.MultithreadedTestCase; import io.nflow.engine.internal.dao.ExecutorDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; +import io.nflow.engine.internal.util.NflowLogger; import io.nflow.engine.listener.WorkflowExecutorListener; import io.nflow.engine.service.WorkflowDefinitionService; @ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = LENIENT) public class WorkflowDispatcherTest { WorkflowDispatcher dispatcher; WorkflowInstanceExecutor executor; @@ -72,6 +75,8 @@ public class WorkflowDispatcherTest { Appender mockAppender; @Captor ArgumentCaptor loggingEventCaptor; + final DispatcherExceptionAnalyzer exceptionAnalyzer = new DefaultDispatcherExceptionAnalyzer(); + final NflowLogger nflowLogger = new NflowLogger(); @BeforeEach public void setup() { @@ -86,10 +91,12 @@ public void setup() { env.setProperty("nflow.executor.stateSaveRetryDelay.seconds", "60"); env.setProperty("nflow.executor.stateVariableValueTooLongRetryDelay.minutes", "60"); env.setProperty("nflow.db.workflowInstanceType.cacheSize", "10000"); - lenient().when(executorDao.isTransactionSupportEnabled()).thenReturn(true); - lenient().when(executorDao.isAutoCommitEnabled()).thenReturn(true); + when(executorDao.isTransactionSupportEnabled()).thenReturn(true); + when(executorDao.isAutoCommitEnabled()).thenReturn(true); + when(executorDao.isAutoCommitEnabled()).thenReturn(true); executor = new WorkflowInstanceExecutor(3, 2, 0, 10, 0, new CustomizableThreadFactory("nflow-executor-")); - dispatcher = new WorkflowDispatcher(executor, workflowInstances, executorFactory, workflowDefinitions, executorDao, env); + dispatcher = new WorkflowDispatcher(executor, workflowInstances, executorFactory, workflowDefinitions, executorDao, + exceptionAnalyzer, nflowLogger, env); Logger logger = (Logger) getLogger(ROOT_LOGGER_NAME); logger.addAppender(mockAppender); } @@ -103,13 +110,15 @@ public void teardown() { @Test public void workflowDispatcherCreationFailsWithoutTransactionSupport() { when(executorDao.isTransactionSupportEnabled()).thenReturn(false); - assertThrows(BeanCreationException.class, () -> new WorkflowDispatcher(executor, workflowInstances, executorFactory, workflowDefinitions, executorDao, env)); + assertThrows(BeanCreationException.class, () -> new WorkflowDispatcher(executor, workflowInstances, executorFactory, + workflowDefinitions, executorDao, exceptionAnalyzer, nflowLogger, env)); } @Test public void workflowDispatcherCreationFailsWithAutoCommitDisabled() { when(executorDao.isAutoCommitEnabled()).thenReturn(false); - assertThrows(BeanCreationException.class, () -> new WorkflowDispatcher(executor, workflowInstances, executorFactory, workflowDefinitions, executorDao, env)); + assertThrows(BeanCreationException.class, () -> new WorkflowDispatcher(executor, workflowInstances, executorFactory, + workflowDefinitions, executorDao, exceptionAnalyzer, nflowLogger, env)); } @Test @@ -265,7 +274,8 @@ class ExceptionOnPoolShutdownIsNotPropagated extends MultithreadedTestCase { @Override public void initialize() { poolSpy = Mockito.spy(executor); - dispatcher = new WorkflowDispatcher(poolSpy, workflowInstances, executorFactory, workflowDefinitions, executorDao, env); + dispatcher = new WorkflowDispatcher(poolSpy, workflowInstances, executorFactory, workflowDefinitions, executorDao, + exceptionAnalyzer, nflowLogger, env); } public void threadDispatcher() { @@ -372,8 +382,8 @@ public void run() { } WorkflowStateProcessor fakeWorkflowExecutor(long instanceId, final Runnable fakeCommand) { - return new WorkflowStateProcessor(instanceId, FALSE::booleanValue, null, null, null, null, null, null, env, new ConcurrentHashMap<>(), - (WorkflowExecutorListener) null) { + return new WorkflowStateProcessor(instanceId, FALSE::booleanValue, null, null, null, null, null, null, env, + new ConcurrentHashMap<>(), null, (WorkflowExecutorListener) null) { @Override public void run() { fakeCommand.run(); diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactoryTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactoryTest.java index 9ee47d8ae..20c40c8ed 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactoryTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactoryTest.java @@ -20,6 +20,7 @@ import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; +import io.nflow.engine.internal.util.NflowLogger; import io.nflow.engine.internal.workflow.ObjectStringMapper; import io.nflow.engine.internal.workflow.WorkflowInstancePreProcessor; import io.nflow.engine.listener.WorkflowExecutorListener; @@ -46,6 +47,7 @@ public class WorkflowStateProcessorFactoryTest extends BaseNflowTest { WorkflowExecutorListener listener2; WorkflowExecutorListener[] listeners = new WorkflowExecutorListener[] { listener1, listener2 }; WorkflowStateProcessorFactory factory; + final NflowLogger nflowLogger = new NflowLogger(); private static final int STUCK_THREAD_THRESHOLD = 5; @BeforeEach @@ -59,7 +61,7 @@ public void setup() { env.setProperty("nflow.executor.stateVariableValueTooLongRetryDelay.minutes", "60"); env.setProperty("nflow.db.workflowInstanceType.cacheSize", "10000"); factory = new WorkflowStateProcessorFactory(workflowDefinitions, workflowInstances, objectMapper, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env); + maintenanceDao, workflowInstancePreProcessor, nflowLogger, env); } @Test diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java index ee169844b..6280ef86d 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java @@ -86,6 +86,7 @@ import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; +import io.nflow.engine.internal.util.NflowLogger; import io.nflow.engine.internal.workflow.ObjectStringMapper; import io.nflow.engine.internal.workflow.StateExecutionImpl; import io.nflow.engine.internal.workflow.WorkflowInstancePreProcessor; @@ -138,6 +139,8 @@ public class WorkflowStateProcessorTest extends BaseNflowTest { @Mock WorkflowInstancePreProcessor workflowInstancePreProcessor; + final NflowLogger nflowLogger = new NflowLogger(); + @Mock StateExecutionImpl executionMock; @@ -200,7 +203,7 @@ public void setup() { env.setProperty("nflow.executor.stateVariableValueTooLongRetryDelay.minutes", "60"); env.setProperty("nflow.db.workflowInstanceType.cacheSize", "10000"); executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, listener1, listener2); + maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener1, listener2); setCurrentMillisFixed(currentTimeMillis()); lenient().doReturn(executeWf).when(workflowDefinitions).getWorkflowDefinition("execute-test"); lenient().doReturn(forceWf).when(workflowDefinitions).getWorkflowDefinition("force-test"); @@ -388,7 +391,7 @@ public void skippingWorkflowWithListenerCausesProcessorToStopProcessingWorkflow( when(workflowInstances.getWorkflowInstance(instance.id, INCLUDES, null)).thenReturn(instance); WorkflowExecutorListener listener = mock(WorkflowExecutorListener.class); executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, listener); + maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener); doAnswer((Answer) invocation -> retryAfter(skipped, "")).when(listener).process(any(ListenerContext.class), any(ListenerChain.class)); @@ -518,7 +521,7 @@ public void finishingChildWakesParentAutomaticallyWhenParentIsInWaitState() { public void goToErrorStateWhenNextStateIsInvalid() { env.setProperty("nflow.illegal.state.change.action", "ignore"); executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, listener1, listener2); + maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener1, listener2); WorkflowInstance instance = executingInstanceBuilder().setType("failing-test").setState("invalidNextState").build(); when(workflowInstances.getWorkflowInstance(instance.id, INCLUDES, null)).thenReturn(instance); @@ -787,7 +790,7 @@ public void illegalStateChangeGoesToErrorState() { public void illegalStateChangeGoesToIllegalStateWhenActionIsLog() { env.setProperty("nflow.illegal.state.change.action", "log"); executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, listener1, listener2); + maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener1, listener2); WorkflowInstance instance = executingInstanceBuilder().setType("simple-test").setState("illegalStateChange").build(); when(workflowInstances.getWorkflowInstance(instance.id, INCLUDES, null)).thenReturn(instance); @@ -806,7 +809,7 @@ public void illegalStateChangeGoesToIllegalStateWhenActionIsLog() { public void illegalStateChangeGoesToIllegalStateWhenActionIsIgnore() { env.setProperty("nflow.illegal.state.change.action", "ignore"); executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, listener1, listener2); + maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener1, listener2); WorkflowInstance instance = executingInstanceBuilder().setType("simple-test").setState("illegalStateChange").build(); when(workflowInstances.getWorkflowInstance(instance.id, INCLUDES, null)).thenReturn(instance); From bc7dd2b264d75d59bc3f55d68e0c144a2719180a Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Fri, 5 Feb 2021 21:15:42 +0200 Subject: [PATCH 11/18] add javadoc --- .../executor/DefaultDispatcherExceptionAnalyzer.java | 6 ++++++ .../executor/DispatcherExceptionAnalyzer.java | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java index 7b2dea398..2e0701427 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java @@ -7,9 +7,15 @@ import io.nflow.engine.internal.dao.PollingRaceConditionException; import io.nflow.engine.internal.executor.DispatcherExceptionHandling.Builder; +/** + * Default dispatcher exception analyzer. + */ @Component public class DefaultDispatcherExceptionAnalyzer implements DispatcherExceptionAnalyzer { + /** + * {@inheritDoc} + */ @Override public DispatcherExceptionHandling analyze(Exception e) { Builder builder = new DispatcherExceptionHandling.Builder(); diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java index a17b95232..4d64f5717 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java @@ -1,5 +1,17 @@ package io.nflow.engine.internal.executor; +/** + * Dispatcher exception analyzer analyzes exceptions throws by the workflow dispatcher and determines how the exception is + * handled. + */ public interface DispatcherExceptionAnalyzer { + + /** + * Analyze the exception. + * + * @param e + * The exception to be analyzed. + * @return How the exception should be handled. + */ DispatcherExceptionHandling analyze(Exception e); } From ddb016df29a1f7a54a7b41762c6aa5871d907e92 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Fri, 5 Feb 2021 22:02:04 +0200 Subject: [PATCH 12/18] extract baseclass for exception analyzers --- .../DispatcherExceptionHandling.java | 72 +++++----------- .../engine/exception/ExceptionHandling.java | 72 ++++++++++++++++ .../StateProcessExceptionHandling.java | 53 ++++++++++++ .../DefaultDispatcherExceptionAnalyzer.java | 3 +- .../executor/DispatcherExceptionAnalyzer.java | 2 + .../internal/executor/WorkflowDispatcher.java | 1 + .../executor/WorkflowStateProcessor.java | 6 +- .../definition/ExceptionHandling.java | 83 ------------------- .../workflow/definition/WorkflowSettings.java | 9 +- ...efaultDispatcherExceptionAnalyzerTest.java | 1 + .../executor/WorkflowStateProcessorTest.java | 4 +- .../definition/WorkflowSettingsTest.java | 9 +- .../tests/demo/workflow/NoRetryWorkflow.java | 6 +- 13 files changed, 171 insertions(+), 150 deletions(-) rename nflow-engine/src/main/java/io/nflow/engine/{internal/executor => exception}/DispatcherExceptionHandling.java (51%) create mode 100644 nflow-engine/src/main/java/io/nflow/engine/exception/ExceptionHandling.java create mode 100644 nflow-engine/src/main/java/io/nflow/engine/exception/StateProcessExceptionHandling.java delete mode 100644 nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionHandling.java similarity index 51% rename from nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionHandling.java rename to nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionHandling.java index 1a09ed003..289b184d5 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionHandling.java +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionHandling.java @@ -1,25 +1,13 @@ -package io.nflow.engine.internal.executor; - -import static org.slf4j.event.Level.ERROR; - -import org.slf4j.event.Level; +package io.nflow.engine.exception; /** * Controls how an exception should be handled by the dispatcher. */ -public class DispatcherExceptionHandling { +public class DispatcherExceptionHandling extends ExceptionHandling { /** * True when dispatcher should log the exception. */ public final boolean log; - /** - * The log entry level for logging the exception. - */ - public final Level logLevel; - /** - * True when the exception stack trace of the exception should be logged. - */ - public final boolean logStackTrace; /** * True when dispatcher should sleep a while after exception. */ @@ -29,57 +17,38 @@ public class DispatcherExceptionHandling { */ public final boolean randomizeSleep; - DispatcherExceptionHandling(boolean log, Level logLevel, boolean logStackTrace, boolean sleep, boolean randomizeSleep) { - this.log = log; - this.logLevel = logLevel; - this.logStackTrace = logStackTrace; - this.sleep = sleep; - this.randomizeSleep = randomizeSleep; + DispatcherExceptionHandling(Builder builder) { + super(builder); + this.log = builder.log; + this.sleep = builder.sleep; + this.randomizeSleep = builder.randomizeSleep; } /** * Builder for exception handling settings. */ - public static class Builder { - private boolean log = true; - private Level logLevel = ERROR; - private boolean logStackTrace = true; - private boolean sleep = true; - private boolean randomizeSleep = false; + public static class Builder extends ExceptionHandling.Builder { + boolean log = true; + boolean sleep = true; + boolean randomizeSleep = false; /** - * Set if dispatcher should log the exception or not. - * - * @param log - * True if dispatcher should log the exception. - * @return This. + * {@inheritDoc} */ - public Builder setLog(boolean log) { - this.log = log; + @Override + public Builder getThis() { return this; } /** - * Set the log entry level. - * - * @param logLevel - * The log entry level. - * @return This. - */ - public Builder setLogLevel(Level logLevel) { - this.logLevel = logLevel; - return this; - } - - /** - * Set if exception stack trace should be logged or not. + * Set if dispatcher should log the exception or not. * - * @param logStackTrace - * True to log the exception stack trace. + * @param log + * True if dispatcher should log the exception. * @return This. */ - public Builder setLogStackTrace(boolean logStackTrace) { - this.logStackTrace = logStackTrace; + public Builder setLog(boolean log) { + this.log = log; return this; } @@ -112,8 +81,9 @@ public Builder setRandomizeSleep(boolean randomizeSleep) { * * @return Dispatcher exception handling. */ + @Override public DispatcherExceptionHandling build() { - return new DispatcherExceptionHandling(log, logLevel, logStackTrace, sleep, randomizeSleep); + return new DispatcherExceptionHandling(this); } } } diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/ExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/exception/ExceptionHandling.java new file mode 100644 index 000000000..3c6208fb8 --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/ExceptionHandling.java @@ -0,0 +1,72 @@ +package io.nflow.engine.exception; + +import static org.slf4j.event.Level.ERROR; + +import org.slf4j.event.Level; + +/** + * Controls how an exception should be handled. + */ +public class ExceptionHandling { + /** + * The log entry level for logging the exception. + */ + public final Level logLevel; + /** + * True when the exception stack trace should be logged. + */ + public final boolean logStackTrace; + + ExceptionHandling(Builder builder) { + this.logLevel = builder.logLevel; + this.logStackTrace = builder.logStackTrace; + } + + /** + * Builder for exception handling settings. + */ + public abstract static class Builder> { + Level logLevel = ERROR; + boolean logStackTrace = true; + + /** + * Return this. + * + * @return This. + */ + public abstract T getThis(); + + /** + * Set the log entry level. + * + * @param logLevel + * The log entry level. + * @return This. + */ + public T setLogLevel(Level logLevel) { + this.logLevel = logLevel; + return getThis(); + } + + /** + * Set if exception stack trace should be logged or not. + * + * @param logStackTrace + * True to log the exception stack trace, false to log the exception message only. + * @return This. + */ + public T setLogStackTrace(boolean logStackTrace) { + this.logStackTrace = logStackTrace; + return getThis(); + } + + /** + * Create the exception handling object. + * + * @return Exception handling. + */ + public ExceptionHandling build() { + return new ExceptionHandling(getThis()); + } + } +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/StateProcessExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/exception/StateProcessExceptionHandling.java new file mode 100644 index 000000000..3190b159e --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/StateProcessExceptionHandling.java @@ -0,0 +1,53 @@ +package io.nflow.engine.exception; + +/** + * Controls how an exception thrown by a state method should be handled by the workflow state processor. + */ +public class StateProcessExceptionHandling extends ExceptionHandling { + /** + * True when the state method processing should be retried. + */ + public final boolean isRetryable; + + StateProcessExceptionHandling(Builder builder) { + super(builder); + this.isRetryable = builder.isRetryable; + } + + /** + * Builder for exception handling settings. + */ + public static class Builder extends ExceptionHandling.Builder { + boolean isRetryable = true; + + /** + * {@inheritDoc} + */ + @Override + public Builder getThis() { + return this; + } + + /** + * Set if state method processing is retryable or not. + * + * @param isRetryable + * True if state method processing should be retried. + * @return This. + */ + public Builder setRetryable(boolean isRetryable) { + this.isRetryable = isRetryable; + return this; + } + + /** + * Create the exception handling object. + * + * @return State process exception handling. + */ + @Override + public StateProcessExceptionHandling build() { + return new StateProcessExceptionHandling(this); + } + } +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java index 2e0701427..97b16d188 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java @@ -3,9 +3,10 @@ import org.slf4j.event.Level; import org.springframework.stereotype.Component; +import io.nflow.engine.exception.DispatcherExceptionHandling; +import io.nflow.engine.exception.DispatcherExceptionHandling.Builder; import io.nflow.engine.internal.dao.PollingBatchException; import io.nflow.engine.internal.dao.PollingRaceConditionException; -import io.nflow.engine.internal.executor.DispatcherExceptionHandling.Builder; /** * Default dispatcher exception analyzer. diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java index 4d64f5717..6234f76cb 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java @@ -1,5 +1,7 @@ package io.nflow.engine.internal.executor; +import io.nflow.engine.exception.DispatcherExceptionHandling; + /** * Dispatcher exception analyzer analyzes exceptions throws by the workflow dispatcher and determines how the exception is * handled. diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java index eea2d7d07..1d8612c2d 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java @@ -14,6 +14,7 @@ import org.springframework.stereotype.Component; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.nflow.engine.exception.DispatcherExceptionHandling; import io.nflow.engine.internal.dao.ExecutorDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; import io.nflow.engine.internal.util.NflowLogger; diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index 2110a2965..801a4a1a7 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -36,6 +36,7 @@ import org.springframework.util.Assert; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.nflow.engine.exception.StateProcessExceptionHandling; import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; import io.nflow.engine.internal.util.NflowLogger; @@ -50,7 +51,6 @@ import io.nflow.engine.service.WorkflowDefinitionService; import io.nflow.engine.service.WorkflowInstanceService; import io.nflow.engine.workflow.definition.AbstractWorkflowDefinition; -import io.nflow.engine.workflow.definition.ExceptionHandling; import io.nflow.engine.workflow.definition.NextAction; import io.nflow.engine.workflow.definition.WorkflowSettings; import io.nflow.engine.workflow.definition.WorkflowState; @@ -176,7 +176,7 @@ private void runImpl() { thrown = thrown.getCause(); } execution.setFailed(thrown); - ExceptionHandling exceptionHandling = settings.exceptionAnalyzer.apply(state, thrown); + StateProcessExceptionHandling exceptionHandling = settings.exceptionAnalyzer.apply(state, thrown); if (exceptionHandling.isRetryable) { logRetryableException(exceptionHandling, state.name(), thrown); execution.setRetry(true); @@ -203,7 +203,7 @@ private void runImpl() { logger.debug("Finished."); } - private void logRetryableException(ExceptionHandling exceptionHandling, String state, Throwable thrown) { + private void logRetryableException(StateProcessExceptionHandling exceptionHandling, String state, Throwable thrown) { if (exceptionHandling.logStackTrace) { nflowLogger.log(logger, exceptionHandling.logLevel, "Handling state '{}' threw a retryable exception, trying again later.", new Object[] { state, thrown }); diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java deleted file mode 100644 index 143a496ba..000000000 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/ExceptionHandling.java +++ /dev/null @@ -1,83 +0,0 @@ -package io.nflow.engine.workflow.definition; - -import static org.slf4j.event.Level.ERROR; - -import org.slf4j.event.Level; - -/** - * Controls how an exception thrown by a state method should be handled by the workflow state processor. - */ -public class ExceptionHandling { - /** - * True when the state method processing should be retried. - */ - public final boolean isRetryable; - /** - * The log entry level for logging the exception. - */ - public final Level logLevel; - /** - * True when the exception stack trace of the exception should be logged. False to log only exception message. - */ - public final boolean logStackTrace; - - ExceptionHandling(boolean isRetryable, Level logLevel, boolean logStackTrace) { - this.isRetryable = isRetryable; - this.logLevel = logLevel; - this.logStackTrace = logStackTrace; - } - - /** - * Builder for exception handling settings. - */ - public static class Builder { - private boolean isRetryable = true; - private Level logLevel = ERROR; - private boolean logStackTrace = true; - - /** - * Set if state method processing is retryable or not. - * - * @param isRetryable - * True if state method processing should be retried. - * @return This. - */ - public Builder setRetryable(boolean isRetryable) { - this.isRetryable = isRetryable; - return this; - } - - /** - * Set the log entry level. - * - * @param logLevel - * The log entry level. - * @return This. - */ - public Builder setLogLevel(Level logLevel) { - this.logLevel = logLevel; - return this; - } - - /** - * Set if exception stack trace should be logged or not. - * - * @param logStackTrace - * True to log the exception stack trace, false to log the exception message only. - * @return This. - */ - public Builder setLogStackTrace(boolean logStackTrace) { - this.logStackTrace = logStackTrace; - return this; - } - - /** - * Create the exception handling object. - * - * @return Exception handling. - */ - public ExceptionHandling build() { - return new ExceptionHandling(isRetryable, logLevel, logStackTrace); - } - } -} diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java index d5e2cb9d5..c635c5b5d 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java @@ -21,6 +21,7 @@ import org.joda.time.ReadablePeriod; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.nflow.engine.exception.StateProcessExceptionHandling; import io.nflow.engine.model.ModelObject; /** @@ -72,7 +73,7 @@ public class WorkflowSettings extends ModelObject { /** * Exception analyzer controls how an exception thrown by a state method should be handled. */ - public final BiFunction exceptionAnalyzer; + public final BiFunction exceptionAnalyzer; WorkflowSettings(Builder builder) { this.minErrorTransitionDelay = builder.minErrorTransitionDelay; @@ -106,8 +107,8 @@ public static class Builder { // TODO: replace state.isRetryAllowed(thrown) with !thrown.getClass().isAnnotationPresent(NonRetryable.class) in the next // major release @SuppressWarnings("deprecation") - BiFunction exceptionAnalyzer = (state, thrown) -> new ExceptionHandling.Builder() - .setRetryable(state.isRetryAllowed(thrown)).build(); + BiFunction exceptionAnalyzer = (state, + thrown) -> new StateProcessExceptionHandling.Builder().setRetryable(state.isRetryAllowed(thrown)).build(); /** * Returns true randomly every n:th time. @@ -269,7 +270,7 @@ public Builder setDefaultPriority(short defaultPriority) { * The exception analyzer function. * @return this. */ - public Builder setExceptionAnalyzer(BiFunction exceptionAnalyzer) { + public Builder setExceptionAnalyzer(BiFunction exceptionAnalyzer) { this.exceptionAnalyzer = exceptionAnalyzer; return this; } diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java index 583155768..a9d4242bc 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java @@ -7,6 +7,7 @@ import org.junit.jupiter.api.Test; import org.slf4j.event.Level; +import io.nflow.engine.exception.DispatcherExceptionHandling; import io.nflow.engine.internal.dao.PollingBatchException; import io.nflow.engine.internal.dao.PollingRaceConditionException; diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java index 6280ef86d..185dea0a1 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java @@ -84,6 +84,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; +import io.nflow.engine.exception.StateProcessExceptionHandling; import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; import io.nflow.engine.internal.util.NflowLogger; @@ -97,7 +98,6 @@ import io.nflow.engine.service.WorkflowInstanceInclude; import io.nflow.engine.service.WorkflowInstanceService; import io.nflow.engine.workflow.curated.BulkWorkflow; -import io.nflow.engine.workflow.definition.ExceptionHandling; import io.nflow.engine.workflow.definition.Mutable; import io.nflow.engine.workflow.definition.NextAction; import io.nflow.engine.workflow.definition.StateExecution; @@ -1295,7 +1295,7 @@ public static class NonRetryableWorkflow extends WorkflowDefinition new ExceptionHandling.Builder().setRetryable(false).build()).build()); + .setExceptionAnalyzer((s, t) -> new StateProcessExceptionHandling.Builder().setRetryable(false).build()).build()); } public static enum State implements WorkflowState { diff --git a/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java b/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java index 8879e3517..ac1e83072 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java @@ -18,6 +18,7 @@ import org.junit.jupiter.api.Test; import org.slf4j.event.Level; +import io.nflow.engine.exception.StateProcessExceptionHandling; public class WorkflowSettingsTest { DateTime now = new DateTime(2014, 10, 22, 20, 44, 0); @@ -108,7 +109,7 @@ public void oncePerDaySupplierWorks() { public void defaultExceptionAnalyzer() { WorkflowSettings s = new WorkflowSettings.Builder().build(); - ExceptionHandling exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, new Throwable()); + StateProcessExceptionHandling exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, new Throwable()); assertThat(exceptionHandling.isRetryable, is(true)); assertThat(exceptionHandling.logLevel, is(Level.ERROR)); assertThat(exceptionHandling.logStackTrace, is(true)); @@ -121,10 +122,12 @@ public void defaultExceptionAnalyzer() { @Test public void customExceptionAnalyzer() { - WorkflowSettings s = new WorkflowSettings.Builder().setExceptionAnalyzer((state, thrown) -> new ExceptionHandling.Builder() + WorkflowSettings s = new WorkflowSettings.Builder() + .setExceptionAnalyzer((state, thrown) -> new StateProcessExceptionHandling.Builder() .setLogLevel(Level.INFO).setRetryable(true).setLogStackTrace(false).build()).build(); - ExceptionHandling exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, new NonRetryableException()); + StateProcessExceptionHandling exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, + new NonRetryableException()); assertThat(exceptionHandling.isRetryable, is(true)); assertThat(exceptionHandling.logLevel, is(Level.INFO)); assertThat(exceptionHandling.logStackTrace, is(false)); diff --git a/nflow-tests/src/main/java/io/nflow/tests/demo/workflow/NoRetryWorkflow.java b/nflow-tests/src/main/java/io/nflow/tests/demo/workflow/NoRetryWorkflow.java index 59f890313..06891f6e7 100644 --- a/nflow-tests/src/main/java/io/nflow/tests/demo/workflow/NoRetryWorkflow.java +++ b/nflow-tests/src/main/java/io/nflow/tests/demo/workflow/NoRetryWorkflow.java @@ -9,7 +9,7 @@ import org.springframework.stereotype.Component; -import io.nflow.engine.workflow.definition.ExceptionHandling; +import io.nflow.engine.exception.StateProcessExceptionHandling; import io.nflow.engine.workflow.definition.NextAction; import io.nflow.engine.workflow.definition.NonRetryable; import io.nflow.engine.workflow.definition.StateExecution; @@ -56,8 +56,8 @@ public NoRetryWorkflow() { permit(State.process, State.done); } - private static BiFunction exceptionAnalyzer() { - return (s, t) -> new ExceptionHandling.Builder() + private static BiFunction exceptionAnalyzer() { + return (s, t) -> new StateProcessExceptionHandling.Builder() .setRetryable(s != State.begin && !t.getClass().isAnnotationPresent(NonRetryable.class)).build(); } From 4acb0aae14c399530dbeeac68ee4f5cbf1f8372f Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Fri, 5 Feb 2021 23:20:27 +0200 Subject: [PATCH 13/18] add exception analyzer and handling for state save --- .../DefaultStateSaveExceptionAnalyzer.java | 38 +++++++++++++ .../DispatcherExceptionAnalyzer.java} | 15 +++-- .../DispatcherExceptionHandling.java | 6 +- .../engine/exception/ExceptionHandling.java | 4 +- .../exception/StateSaveExceptionAnalyzer.java | 20 +++++++ .../exception/StateSaveExceptionHandling.java | 57 +++++++++++++++++++ .../executor/DispatcherExceptionAnalyzer.java | 19 ------- .../internal/executor/WorkflowDispatcher.java | 1 + .../executor/WorkflowStateProcessor.java | 23 +++++--- .../WorkflowStateProcessorFactory.java | 12 ++-- ...DefaultStateSaveExceptionAnalyzerTest.java | 25 ++++++++ .../DispatcherExceptionAnalyzerTest.java} | 7 +-- .../executor/WorkflowDispatcherTest.java | 5 +- .../WorkflowStateProcessorFactoryTest.java | 5 +- .../executor/WorkflowStateProcessorTest.java | 27 ++++++--- 15 files changed, 207 insertions(+), 57 deletions(-) create mode 100644 nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java rename nflow-engine/src/main/java/io/nflow/engine/{internal/executor/DefaultDispatcherExceptionAnalyzer.java => exception/DispatcherExceptionAnalyzer.java} (70%) create mode 100644 nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java create mode 100644 nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionHandling.java delete mode 100644 nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java create mode 100644 nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java rename nflow-engine/src/test/java/io/nflow/engine/{internal/executor/DefaultDispatcherExceptionAnalyzerTest.java => exception/DispatcherExceptionAnalyzerTest.java} (86%) diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java new file mode 100644 index 000000000..02694c894 --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java @@ -0,0 +1,38 @@ +package io.nflow.engine.exception; + +import static org.joda.time.Duration.standardSeconds; + +import javax.inject.Inject; + +import org.joda.time.Duration; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +/** + * {@inheritDoc} + */ +@Component +public class DefaultStateSaveExceptionAnalyzer implements StateSaveExceptionAnalyzer { + + private final StateSaveExceptionHandling handling; + + /** + * Create state save exception analyzer. + * + * @param env + * The Spring environment. + */ + @Inject + public DefaultStateSaveExceptionAnalyzer(Environment env) { + Duration retryDelay = standardSeconds(env.getRequiredProperty("nflow.executor.stateSaveRetryDelay.seconds", Long.class)); + handling = new StateSaveExceptionHandling.Builder().setRetryDelay(retryDelay).build(); + } + + /** + * {@inheritDoc} + */ + @Override + public StateSaveExceptionHandling analyze(Exception e) { + return handling; + } +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionAnalyzer.java similarity index 70% rename from nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java rename to nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionAnalyzer.java index 97b16d188..63a386afc 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzer.java +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionAnalyzer.java @@ -1,23 +1,26 @@ -package io.nflow.engine.internal.executor; +package io.nflow.engine.exception; import org.slf4j.event.Level; import org.springframework.stereotype.Component; -import io.nflow.engine.exception.DispatcherExceptionHandling; import io.nflow.engine.exception.DispatcherExceptionHandling.Builder; import io.nflow.engine.internal.dao.PollingBatchException; import io.nflow.engine.internal.dao.PollingRaceConditionException; /** - * Default dispatcher exception analyzer. + * Dispatcher exception analyzer analyzes exceptions thrown by the workflow dispatcher and determines how the exception is + * handled. */ @Component -public class DefaultDispatcherExceptionAnalyzer implements DispatcherExceptionAnalyzer { +public class DispatcherExceptionAnalyzer { /** - * {@inheritDoc} + * Analyze the exception. + * + * @param e + * The exception to be analyzed. + * @return How the exception should be handled. */ - @Override public DispatcherExceptionHandling analyze(Exception e) { Builder builder = new DispatcherExceptionHandling.Builder(); if (e instanceof PollingRaceConditionException) { diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionHandling.java index 289b184d5..ccf927289 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionHandling.java +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionHandling.java @@ -41,7 +41,7 @@ public Builder getThis() { } /** - * Set if dispatcher should log the exception or not. + * Set if dispatcher should log the exception or not. Default is true. * * @param log * True if dispatcher should log the exception. @@ -53,7 +53,7 @@ public Builder setLog(boolean log) { } /** - * Set if dispatcher should sleep a while after exception or not. + * Set if dispatcher should sleep a while after exception or not. Default is true. * * @param sleep * True if dispatcher should sleep a while after exception. @@ -65,7 +65,7 @@ public Builder setSleep(boolean sleep) { } /** - * Set if sleep time should be randomized or not. + * Set if sleep time should be randomized or not. Default is false. * * @param randomizeSleep * True if sleep time should be randomized. diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/ExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/exception/ExceptionHandling.java index 3c6208fb8..2a9876c75 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/exception/ExceptionHandling.java +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/ExceptionHandling.java @@ -37,7 +37,7 @@ public abstract static class Builder> { public abstract T getThis(); /** - * Set the log entry level. + * Set the log entry level. Default is ERROR. * * @param logLevel * The log entry level. @@ -49,7 +49,7 @@ public T setLogLevel(Level logLevel) { } /** - * Set if exception stack trace should be logged or not. + * Set if exception stack trace should be logged or not. Default is true. * * @param logStackTrace * True to log the exception stack trace, false to log the exception message only. diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java new file mode 100644 index 000000000..b5142b662 --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java @@ -0,0 +1,20 @@ +package io.nflow.engine.exception; + +import org.springframework.stereotype.Component; + +/** + * State save exception analyzer analyzes exceptions thrown while trying to save workflow state and determines how the exception + * is handled. + */ +@Component +public interface StateSaveExceptionAnalyzer { + + /** + * Analyze the exception. + * + * @param e + * The exception to be analyzed. + * @return How the exception should be handled. + */ + StateSaveExceptionHandling analyze(Exception e); +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionHandling.java b/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionHandling.java new file mode 100644 index 000000000..3fc27f8df --- /dev/null +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionHandling.java @@ -0,0 +1,57 @@ +package io.nflow.engine.exception; + +import static org.joda.time.Duration.standardMinutes; + +import org.joda.time.Duration; + +/** + * Controls how an exception thrown while trying to save the workflow instance state should be handled. + */ +public class StateSaveExceptionHandling extends ExceptionHandling { + /** + * Retry delay. + */ + public Duration retryDelay; + + StateSaveExceptionHandling(Builder builder) { + super(builder); + this.retryDelay = builder.retryDelay; + } + + /** + * Builder for exception handling settings. + */ + public static class Builder extends ExceptionHandling.Builder { + Duration retryDelay = standardMinutes(1); + + /** + * {@inheritDoc} + */ + @Override + public Builder getThis() { + return this; + } + + /** + * Set the retry delay. Default is 1 minute. + * + * @param retryDelay + * The retry delay. + * @return This. + */ + public Builder setRetryDelay(Duration retryDelay) { + this.retryDelay = retryDelay; + return this; + } + + /** + * Create the state save exception handling object. + * + * @return State save exception handling. + */ + @Override + public StateSaveExceptionHandling build() { + return new StateSaveExceptionHandling(this); + } + } +} diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java deleted file mode 100644 index 6234f76cb..000000000 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/DispatcherExceptionAnalyzer.java +++ /dev/null @@ -1,19 +0,0 @@ -package io.nflow.engine.internal.executor; - -import io.nflow.engine.exception.DispatcherExceptionHandling; - -/** - * Dispatcher exception analyzer analyzes exceptions throws by the workflow dispatcher and determines how the exception is - * handled. - */ -public interface DispatcherExceptionAnalyzer { - - /** - * Analyze the exception. - * - * @param e - * The exception to be analyzed. - * @return How the exception should be handled. - */ - DispatcherExceptionHandling analyze(Exception e); -} diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java index 1d8612c2d..ea8a6c2c1 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java @@ -14,6 +14,7 @@ import org.springframework.stereotype.Component; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.nflow.engine.exception.DispatcherExceptionAnalyzer; import io.nflow.engine.exception.DispatcherExceptionHandling; import io.nflow.engine.internal.dao.ExecutorDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index 801a4a1a7..b8def1106 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -37,6 +37,8 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.nflow.engine.exception.StateProcessExceptionHandling; +import io.nflow.engine.exception.StateSaveExceptionAnalyzer; +import io.nflow.engine.exception.StateSaveExceptionHandling; import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; import io.nflow.engine.internal.util.NflowLogger; @@ -82,10 +84,10 @@ class WorkflowStateProcessor implements Runnable { private final int unknownWorkflowTypeRetryDelay; private final int unknownWorkflowStateRetryDelay; private final int stateProcessingRetryDelay; - private final int stateSaveRetryDelay; private final int stateVariableValueTooLongRetryDelay; private final Map processingInstances; private final NflowLogger nflowLogger; + private final StateSaveExceptionAnalyzer stateSaveExceptionAnalyzer; private DateTime startTime; private Thread thread; private ListenerContext listenerContext; @@ -95,7 +97,7 @@ class WorkflowStateProcessor implements Runnable { WorkflowInstanceDao workflowInstanceDao, MaintenanceDao maintenanceDao, WorkflowInstancePreProcessor workflowInstancePreProcessor, Environment env, Map processingInstances, NflowLogger nflowLogger, - WorkflowExecutorListener... executorListeners) { + StateSaveExceptionAnalyzer stateSaveExceptionAnalyzer, WorkflowExecutorListener... executorListeners) { this.instanceId = instanceId; this.shutdownRequested = shutdownRequested; this.objectMapper = objectMapper; @@ -105,13 +107,13 @@ class WorkflowStateProcessor implements Runnable { this.maintenanceDao = maintenanceDao; this.processingInstances = processingInstances; this.nflowLogger = nflowLogger; + this.stateSaveExceptionAnalyzer = stateSaveExceptionAnalyzer; this.executorListeners = asList(executorListeners); this.workflowInstancePreProcessor = workflowInstancePreProcessor; illegalStateChangeAction = env.getRequiredProperty("nflow.illegal.state.change.action"); unknownWorkflowTypeRetryDelay = env.getRequiredProperty("nflow.unknown.workflow.type.retry.delay.minutes", Integer.class); unknownWorkflowStateRetryDelay = env.getRequiredProperty("nflow.unknown.workflow.state.retry.delay.minutes", Integer.class); stateProcessingRetryDelay = env.getRequiredProperty("nflow.executor.stateProcessingRetryDelay.seconds", Integer.class); - stateSaveRetryDelay = env.getRequiredProperty("nflow.executor.stateSaveRetryDelay.seconds", Integer.class); stateVariableValueTooLongRetryDelay = env.getRequiredProperty("nflow.executor.stateVariableValueTooLongRetryDelay.minutes", Integer.class); } @@ -300,9 +302,16 @@ private WorkflowInstance saveWorkflowInstanceState(StateExecutionImpl execution, // return the original instance since persisting failed return instance; } - logger.error("Failed to save workflow instance {} new state, retrying after {} seconds", instance.id, stateSaveRetryDelay, - ex); - sleepIgnoreInterrupted(stateSaveRetryDelay); + StateSaveExceptionHandling handling = stateSaveExceptionAnalyzer.analyze(ex); + if (handling.logStackTrace) { + nflowLogger.log(logger, handling.logLevel, "Failed to save workflow instance {} new state, retrying after {} seconds.", + new Object[] { instance.id, handling.retryDelay, ex }); + } else { + nflowLogger.log(logger, handling.logLevel, + "Failed to save workflow instance {} new state, retrying after {} seconds. Error: {}", + new Object[] { instance.id, handling.retryDelay, ex.getMessage() }); + } + sleepIgnoreInterrupted(handling.retryDelay.getStandardSeconds()); } } } @@ -394,7 +403,7 @@ private void optionallyCleanupWorkflowInstanceHistory(WorkflowSettings settings, } } - private void sleepIgnoreInterrupted(int seconds) { + private void sleepIgnoreInterrupted(long seconds) { try { SECONDS.sleep(seconds); } catch (@SuppressWarnings("unused") InterruptedException ok) { diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactory.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactory.java index 1fb4bc1bb..052eccd3a 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactory.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactory.java @@ -14,6 +14,7 @@ import org.springframework.core.env.Environment; import org.springframework.stereotype.Component; +import io.nflow.engine.exception.StateSaveExceptionAnalyzer; import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; import io.nflow.engine.internal.util.NflowLogger; @@ -32,6 +33,7 @@ public class WorkflowStateProcessorFactory { private final MaintenanceDao maintenanceDao; private final WorkflowInstancePreProcessor workflowInstancePreProcessor; private final NflowLogger nflowLogger; + private final StateSaveExceptionAnalyzer stateSaveExceptionAnalyzer; private final Environment env; @Autowired(required = false) protected WorkflowExecutorListener[] listeners = new WorkflowExecutorListener[0]; @@ -41,7 +43,8 @@ public class WorkflowStateProcessorFactory { @Inject public WorkflowStateProcessorFactory(WorkflowDefinitionService workflowDefinitions, WorkflowInstanceService workflowInstances, ObjectStringMapper objectMapper, WorkflowInstanceDao workflowInstanceDao, MaintenanceDao maintenanceDao, - WorkflowInstancePreProcessor workflowInstancePreProcessor, NflowLogger nflowLogger, Environment env) { + WorkflowInstancePreProcessor workflowInstancePreProcessor, NflowLogger nflowLogger, + StateSaveExceptionAnalyzer stateSaveExceptionAnalyzer, Environment env) { this.workflowDefinitions = workflowDefinitions; this.workflowInstances = workflowInstances; this.objectMapper = objectMapper; @@ -49,13 +52,15 @@ public WorkflowStateProcessorFactory(WorkflowDefinitionService workflowDefinitio this.maintenanceDao = maintenanceDao; this.workflowInstancePreProcessor = workflowInstancePreProcessor; this.nflowLogger = nflowLogger; + this.stateSaveExceptionAnalyzer = stateSaveExceptionAnalyzer; this.stuckThreadThresholdSeconds = env.getRequiredProperty("nflow.executor.stuckThreadThreshold.seconds", Integer.class); this.env = env; } public WorkflowStateProcessor createProcessor(long instanceId, Supplier shutdownRequested) { - return new WorkflowStateProcessor(instanceId, shutdownRequested, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listeners); + return new WorkflowStateProcessor(instanceId, shutdownRequested, objectMapper, workflowDefinitions, workflowInstances, + workflowInstanceDao, maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, + stateSaveExceptionAnalyzer, listeners); } public int getPotentiallyStuckProcessors() { @@ -71,5 +76,4 @@ public int getPotentiallyStuckProcessors() { } return potentiallyStuck; } - } diff --git a/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java b/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java new file mode 100644 index 000000000..35e1c1c45 --- /dev/null +++ b/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java @@ -0,0 +1,25 @@ +package io.nflow.engine.exception; + +import static org.joda.time.Duration.standardSeconds; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; +import org.slf4j.event.Level; +import org.springframework.core.env.Environment; +import org.springframework.mock.env.MockEnvironment; + +public class DefaultStateSaveExceptionAnalyzerTest { + + final Environment env = new MockEnvironment().withProperty("nflow.executor.stateSaveRetryDelay.seconds", "10"); + final StateSaveExceptionAnalyzer analyzer = new DefaultStateSaveExceptionAnalyzer(env); + + @Test + void analyzeReturnsConfiguredDelay() { + StateSaveExceptionHandling handling = analyzer.analyze(new Exception()); + + assertEquals(handling.logLevel, Level.ERROR); + assertTrue(handling.logStackTrace); + assertEquals(handling.retryDelay, standardSeconds(10)); + } +} diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java b/nflow-engine/src/test/java/io/nflow/engine/exception/DispatcherExceptionAnalyzerTest.java similarity index 86% rename from nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java rename to nflow-engine/src/test/java/io/nflow/engine/exception/DispatcherExceptionAnalyzerTest.java index a9d4242bc..1e51bb660 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/DefaultDispatcherExceptionAnalyzerTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/exception/DispatcherExceptionAnalyzerTest.java @@ -1,4 +1,4 @@ -package io.nflow.engine.internal.executor; +package io.nflow.engine.exception; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -7,13 +7,12 @@ import org.junit.jupiter.api.Test; import org.slf4j.event.Level; -import io.nflow.engine.exception.DispatcherExceptionHandling; import io.nflow.engine.internal.dao.PollingBatchException; import io.nflow.engine.internal.dao.PollingRaceConditionException; -public class DefaultDispatcherExceptionAnalyzerTest { +public class DispatcherExceptionAnalyzerTest { - DispatcherExceptionAnalyzer analyzer = new DefaultDispatcherExceptionAnalyzer(); + DispatcherExceptionAnalyzer analyzer = new DispatcherExceptionAnalyzer(); @Test void analyzeGenericExceptionReturnsDefaults() { diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowDispatcherTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowDispatcherTest.java index 18f7828c8..cf36520c2 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowDispatcherTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowDispatcherTest.java @@ -51,6 +51,7 @@ import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.Appender; import edu.umd.cs.mtc.MultithreadedTestCase; +import io.nflow.engine.exception.DispatcherExceptionAnalyzer; import io.nflow.engine.internal.dao.ExecutorDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; import io.nflow.engine.internal.util.NflowLogger; @@ -75,7 +76,7 @@ public class WorkflowDispatcherTest { Appender mockAppender; @Captor ArgumentCaptor loggingEventCaptor; - final DispatcherExceptionAnalyzer exceptionAnalyzer = new DefaultDispatcherExceptionAnalyzer(); + final DispatcherExceptionAnalyzer exceptionAnalyzer = new DispatcherExceptionAnalyzer(); final NflowLogger nflowLogger = new NflowLogger(); @BeforeEach @@ -383,7 +384,7 @@ public void run() { WorkflowStateProcessor fakeWorkflowExecutor(long instanceId, final Runnable fakeCommand) { return new WorkflowStateProcessor(instanceId, FALSE::booleanValue, null, null, null, null, null, null, env, - new ConcurrentHashMap<>(), null, (WorkflowExecutorListener) null) { + new ConcurrentHashMap<>(), null, null, (WorkflowExecutorListener) null) { @Override public void run() { fakeCommand.run(); diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactoryTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactoryTest.java index 20c40c8ed..4aa831123 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactoryTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorFactoryTest.java @@ -18,6 +18,7 @@ import org.mockito.Mock; import org.springframework.mock.env.MockEnvironment; +import io.nflow.engine.exception.StateSaveExceptionAnalyzer; import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; import io.nflow.engine.internal.util.NflowLogger; @@ -42,6 +43,8 @@ public class WorkflowStateProcessorFactoryTest extends BaseNflowTest { WorkflowInstancePreProcessor workflowInstancePreProcessor; MockEnvironment env = new MockEnvironment(); @Mock + StateSaveExceptionAnalyzer stateSaveExceptionAnalyzer; + @Mock WorkflowExecutorListener listener1; @Mock WorkflowExecutorListener listener2; @@ -61,7 +64,7 @@ public void setup() { env.setProperty("nflow.executor.stateVariableValueTooLongRetryDelay.minutes", "60"); env.setProperty("nflow.db.workflowInstanceType.cacheSize", "10000"); factory = new WorkflowStateProcessorFactory(workflowDefinitions, workflowInstances, objectMapper, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, nflowLogger, env); + maintenanceDao, workflowInstancePreProcessor, nflowLogger, stateSaveExceptionAnalyzer, env); } @Test diff --git a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java index 185dea0a1..e178ce6d2 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/internal/executor/WorkflowStateProcessorTest.java @@ -85,6 +85,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.nflow.engine.exception.StateProcessExceptionHandling; +import io.nflow.engine.exception.StateSaveExceptionAnalyzer; import io.nflow.engine.internal.dao.MaintenanceDao; import io.nflow.engine.internal.dao.WorkflowInstanceDao; import io.nflow.engine.internal.util.NflowLogger; @@ -141,6 +142,9 @@ public class WorkflowStateProcessorTest extends BaseNflowTest { final NflowLogger nflowLogger = new NflowLogger(); + @Mock + StateSaveExceptionAnalyzer stateSaveExceptionAnalyzer; + @Mock StateExecutionImpl executionMock; @@ -202,8 +206,9 @@ public void setup() { env.setProperty("nflow.executor.stateSaveRetryDelay.seconds", "1"); env.setProperty("nflow.executor.stateVariableValueTooLongRetryDelay.minutes", "60"); env.setProperty("nflow.db.workflowInstanceType.cacheSize", "10000"); - executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener1, listener2); + executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, + workflowInstanceDao, maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, + stateSaveExceptionAnalyzer, listener1, listener2); setCurrentMillisFixed(currentTimeMillis()); lenient().doReturn(executeWf).when(workflowDefinitions).getWorkflowDefinition("execute-test"); lenient().doReturn(forceWf).when(workflowDefinitions).getWorkflowDefinition("force-test"); @@ -390,8 +395,9 @@ public void skippingWorkflowWithListenerCausesProcessorToStopProcessingWorkflow( .setStateText("myStateText").build(); when(workflowInstances.getWorkflowInstance(instance.id, INCLUDES, null)).thenReturn(instance); WorkflowExecutorListener listener = mock(WorkflowExecutorListener.class); - executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener); + executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, + workflowInstanceDao, maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, + stateSaveExceptionAnalyzer, listener); doAnswer((Answer) invocation -> retryAfter(skipped, "")).when(listener).process(any(ListenerContext.class), any(ListenerChain.class)); @@ -521,7 +527,8 @@ public void finishingChildWakesParentAutomaticallyWhenParentIsInWaitState() { public void goToErrorStateWhenNextStateIsInvalid() { env.setProperty("nflow.illegal.state.change.action", "ignore"); executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener1, listener2); + maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, stateSaveExceptionAnalyzer, + listener1, listener2); WorkflowInstance instance = executingInstanceBuilder().setType("failing-test").setState("invalidNextState").build(); when(workflowInstances.getWorkflowInstance(instance.id, INCLUDES, null)).thenReturn(instance); @@ -789,8 +796,9 @@ public void illegalStateChangeGoesToErrorState() { @Test public void illegalStateChangeGoesToIllegalStateWhenActionIsLog() { env.setProperty("nflow.illegal.state.change.action", "log"); - executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener1, listener2); + executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, + workflowInstanceDao, maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, + stateSaveExceptionAnalyzer, listener1, listener2); WorkflowInstance instance = executingInstanceBuilder().setType("simple-test").setState("illegalStateChange").build(); when(workflowInstances.getWorkflowInstance(instance.id, INCLUDES, null)).thenReturn(instance); @@ -808,8 +816,9 @@ public void illegalStateChangeGoesToIllegalStateWhenActionIsLog() { @Test public void illegalStateChangeGoesToIllegalStateWhenActionIsIgnore() { env.setProperty("nflow.illegal.state.change.action", "ignore"); - executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, workflowInstanceDao, - maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, listener1, listener2); + executor = new WorkflowStateProcessor(1, shutdownRequest::get, objectMapper, workflowDefinitions, workflowInstances, + workflowInstanceDao, maintenanceDao, workflowInstancePreProcessor, env, processingInstances, nflowLogger, + stateSaveExceptionAnalyzer, listener1, listener2); WorkflowInstance instance = executingInstanceBuilder().setType("simple-test").setState("illegalStateChange").build(); when(workflowInstances.getWorkflowInstance(instance.id, INCLUDES, null)).thenReturn(instance); From a8162bb7ffe253eb7a191e7eab702aea2f3b14b5 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Fri, 5 Feb 2021 23:28:59 +0200 Subject: [PATCH 14/18] update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c8324442..7bbd5936e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Control retrying and logging of an exception thrown by a state method via `WorkflowSettings` (replaces deprecated `WorkflowState.isRetryAllowed(...)`) - Control retrying and logging of an exception thrown by a state method via `WorkflowSettings` (replaces deprecated `WorkflowState.isRetryAllowed(...)`). - Control logging and sleeping after exceptions in `WorkflowDispatcher`. +- Control logging and sleeping after a failure to save workflow instance state. **Details** - `nflow-engine` @@ -24,6 +25,10 @@ - Control whether the stack trace of the exception is logged or not. - Control whether dispatcher should sleep after the exception or not. - Control whether the sleep time should be randomized or not. + - Control logging after a failure to save workflow instance state by providing a `StateSaveExceptionHandler` that returns `StateSaveExceptionHandling` objects: + - Control which log level is used to log the exception. + - Control whether the stack trace of the exception is logged or not. + - Control how long the `WorkflowStateProcessor` should sleep before retrying. - `nflow-rest-api-common`, `nflow-rest-api-jax-rs`, `nflow-rest-api-spring-web` - `UpdateWorkflowInstanceRequest.businessKey` field was added to support updating workflow instance business key via REST API. - Added support for new query parameters `stateVariableKey` and `stateVariableValue` to `GET /v1/workflow-instance` to limit search query by state variable name and key. Only the latest value of the state variable of the workflow instance is used. From 9b2a7071d3909a71c87668e82243930fcf0baaa2 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Fri, 5 Feb 2021 23:32:30 +0200 Subject: [PATCH 15/18] pass retry count to state save exception analyzer --- .../engine/exception/DefaultStateSaveExceptionAnalyzer.java | 2 +- .../io/nflow/engine/exception/StateSaveExceptionAnalyzer.java | 4 +++- .../engine/internal/executor/WorkflowStateProcessor.java | 3 ++- .../exception/DefaultStateSaveExceptionAnalyzerTest.java | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java index 02694c894..22c0b291d 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java @@ -32,7 +32,7 @@ public DefaultStateSaveExceptionAnalyzer(Environment env) { * {@inheritDoc} */ @Override - public StateSaveExceptionHandling analyze(Exception e) { + public StateSaveExceptionHandling analyze(Exception e, int saveRetryCount) { return handling; } } diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java index b5142b662..ba6192929 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java @@ -14,7 +14,9 @@ public interface StateSaveExceptionAnalyzer { * * @param e * The exception to be analyzed. + * @param saveRetryCount + * How many times the saving has been attempted before this attempt. * @return How the exception should be handled. */ - StateSaveExceptionHandling analyze(Exception e); + StateSaveExceptionHandling analyze(Exception e, int saveRetryCount); } diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index b8def1106..924f48a11 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -291,6 +291,7 @@ private WorkflowInstance saveWorkflowInstanceState(StateExecutionImpl execution, .setStateText(getStateText(instance, execution)) // .setState(execution.getNextState()) // .setRetries(execution.isRetry() ? execution.getRetries() + 1 : 0); + int saveRetryCount = 0; while (true) { try { return persistWorkflowInstanceState(execution, instance.stateVariables, actionBuilder, instanceBuilder); @@ -302,7 +303,7 @@ private WorkflowInstance saveWorkflowInstanceState(StateExecutionImpl execution, // return the original instance since persisting failed return instance; } - StateSaveExceptionHandling handling = stateSaveExceptionAnalyzer.analyze(ex); + StateSaveExceptionHandling handling = stateSaveExceptionAnalyzer.analyze(ex, saveRetryCount++); if (handling.logStackTrace) { nflowLogger.log(logger, handling.logLevel, "Failed to save workflow instance {} new state, retrying after {} seconds.", new Object[] { instance.id, handling.retryDelay, ex }); diff --git a/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java b/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java index 35e1c1c45..89f0ae08f 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java @@ -16,7 +16,7 @@ public class DefaultStateSaveExceptionAnalyzerTest { @Test void analyzeReturnsConfiguredDelay() { - StateSaveExceptionHandling handling = analyzer.analyze(new Exception()); + StateSaveExceptionHandling handling = analyzer.analyze(new Exception(), 0); assertEquals(handling.logLevel, Level.ERROR); assertTrue(handling.logStackTrace); From ccfed6979efe59b5a9cd2e564a0d4494c73a41a1 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Sat, 6 Feb 2021 14:24:55 +0200 Subject: [PATCH 16/18] use default exception handler if custom handler fails --- .../executor/WorkflowStateProcessor.java | 2 +- .../workflow/definition/WorkflowSettings.java | 42 +++++++++++++++---- .../definition/WorkflowSettingsTest.java | 21 ++++++++-- 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index 924f48a11..296b1114f 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -178,7 +178,7 @@ private void runImpl() { thrown = thrown.getCause(); } execution.setFailed(thrown); - StateProcessExceptionHandling exceptionHandling = settings.exceptionAnalyzer.apply(state, thrown); + StateProcessExceptionHandling exceptionHandling = settings.analyzeExeption(state, thrown); if (exceptionHandling.isRetryable) { logRetryableException(exceptionHandling, state.name(), thrown); execution.setRetry(true); diff --git a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java index c635c5b5d..1f7c1318d 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java +++ b/nflow-engine/src/main/java/io/nflow/engine/workflow/definition/WorkflowSettings.java @@ -7,6 +7,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.joda.time.DateTime.now; import static org.joda.time.DateTimeUtils.currentTimeMillis; +import static org.slf4j.LoggerFactory.getLogger; import java.math.BigInteger; import java.util.HashMap; @@ -19,6 +20,7 @@ import org.joda.time.DateTime; import org.joda.time.LocalDateTime; import org.joda.time.ReadablePeriod; +import org.slf4j.Logger; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.nflow.engine.exception.StateProcessExceptionHandling; @@ -70,10 +72,16 @@ public class WorkflowSettings extends ModelObject { * Default priority for new workflow instances. */ public final short defaultPriority; - /** - * Exception analyzer controls how an exception thrown by a state method should be handled. - */ - public final BiFunction exceptionAnalyzer; + + private final BiFunction exceptionAnalyzer; + + // TODO: replace state.isRetryAllowed(thrown) with !thrown.getClass().isAnnotationPresent(NonRetryable.class) in the next + // major release + @SuppressWarnings("deprecation") + private static final BiFunction DEFAULT_EXCEPTION_ANALYZER = (state, + thrown) -> new StateProcessExceptionHandling.Builder().setRetryable(state.isRetryAllowed(thrown)).build(); + + private static final Logger logger = getLogger(WorkflowSettings.class); WorkflowSettings(Builder builder) { this.minErrorTransitionDelay = builder.minErrorTransitionDelay; @@ -104,11 +112,7 @@ public static class Builder { ReadablePeriod historyDeletableAfter; short defaultPriority = 0; BooleanSupplier deleteHistoryCondition = onAverageEveryNthExecution(100); - // TODO: replace state.isRetryAllowed(thrown) with !thrown.getClass().isAnnotationPresent(NonRetryable.class) in the next - // major release - @SuppressWarnings("deprecation") - BiFunction exceptionAnalyzer = (state, - thrown) -> new StateProcessExceptionHandling.Builder().setRetryable(state.isRetryAllowed(thrown)).build(); + BiFunction exceptionAnalyzer; /** * Returns true randomly every n:th time. @@ -354,4 +358,24 @@ public boolean deleteWorkflowInstanceHistory() { public Short getDefaultPriority() { return defaultPriority; } + + /** + * Analyze exception thrown by a state method to determine how it should be handled. + * + * @param state + * The state that failed to be processed. + * @param thrown + * The exception to be analyzed. + * @return How the exception should be handled. + */ + public StateProcessExceptionHandling analyzeExeption(WorkflowState state, Throwable thrown) { + if (exceptionAnalyzer != null) { + try { + return exceptionAnalyzer.apply(state, thrown); + } catch (Exception e) { + logger.error("Custom exception analysis failed, using default analyzer.", e); + } + } + return DEFAULT_EXCEPTION_ANALYZER.apply(state, thrown); + } } diff --git a/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java b/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java index ac1e83072..d0150754d 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/workflow/definition/WorkflowSettingsTest.java @@ -10,6 +10,7 @@ import static org.joda.time.DateTimeUtils.setCurrentMillisFixed; import static org.joda.time.DateTimeUtils.setCurrentMillisSystem; +import java.util.function.BiFunction; import java.util.function.BooleanSupplier; import org.joda.time.DateTime; @@ -109,12 +110,12 @@ public void oncePerDaySupplierWorks() { public void defaultExceptionAnalyzer() { WorkflowSettings s = new WorkflowSettings.Builder().build(); - StateProcessExceptionHandling exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, new Throwable()); + StateProcessExceptionHandling exceptionHandling = s.analyzeExeption(TestWorkflow.State.begin, new Throwable()); assertThat(exceptionHandling.isRetryable, is(true)); assertThat(exceptionHandling.logLevel, is(Level.ERROR)); assertThat(exceptionHandling.logStackTrace, is(true)); - exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, new NonRetryableException()); + exceptionHandling = s.analyzeExeption(TestWorkflow.State.begin, new NonRetryableException()); assertThat(exceptionHandling.isRetryable, is(false)); assertThat(exceptionHandling.logLevel, is(Level.ERROR)); assertThat(exceptionHandling.logStackTrace, is(true)); @@ -126,13 +127,25 @@ public void customExceptionAnalyzer() { .setExceptionAnalyzer((state, thrown) -> new StateProcessExceptionHandling.Builder() .setLogLevel(Level.INFO).setRetryable(true).setLogStackTrace(false).build()).build(); - StateProcessExceptionHandling exceptionHandling = s.exceptionAnalyzer.apply(TestWorkflow.State.begin, - new NonRetryableException()); + StateProcessExceptionHandling exceptionHandling = s.analyzeExeption(TestWorkflow.State.begin, new NonRetryableException()); assertThat(exceptionHandling.isRetryable, is(true)); assertThat(exceptionHandling.logLevel, is(Level.INFO)); assertThat(exceptionHandling.logStackTrace, is(false)); } + @Test + public void defaultExceptionAnalyzerIsUsedWhenCustomAnalyzerFails() { + BiFunction failingExceptionAnalyzer = (state, thrown) -> { + throw new IllegalStateException("fail"); + }; + WorkflowSettings s = new WorkflowSettings.Builder().setExceptionAnalyzer(failingExceptionAnalyzer).build(); + + StateProcessExceptionHandling exceptionHandling = s.analyzeExeption(TestWorkflow.State.begin, new NonRetryableException()); + assertThat(exceptionHandling.isRetryable, is(false)); + assertThat(exceptionHandling.logLevel, is(Level.ERROR)); + assertThat(exceptionHandling.logStackTrace, is(true)); + } + @NonRetryable static class NonRetryableException extends RuntimeException { private static final long serialVersionUID = 1L; From 073d238254c21c7e3fbc4f168bafa7cbc124c9ce Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Sat, 6 Feb 2021 14:55:35 +0200 Subject: [PATCH 17/18] use default exception handler if custom handler fails --- .../DefaultStateSaveExceptionAnalyzer.java | 38 ------------ .../exception/StateSaveExceptionAnalyzer.java | 48 ++++++++++++++- .../executor/WorkflowStateProcessor.java | 2 +- ...DefaultStateSaveExceptionAnalyzerTest.java | 25 -------- .../StateSaveExceptionAnalyzerTest.java | 58 +++++++++++++++++++ 5 files changed, 105 insertions(+), 66 deletions(-) delete mode 100644 nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java delete mode 100644 nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java create mode 100644 nflow-engine/src/test/java/io/nflow/engine/exception/StateSaveExceptionAnalyzerTest.java diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java deleted file mode 100644 index 22c0b291d..000000000 --- a/nflow-engine/src/main/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzer.java +++ /dev/null @@ -1,38 +0,0 @@ -package io.nflow.engine.exception; - -import static org.joda.time.Duration.standardSeconds; - -import javax.inject.Inject; - -import org.joda.time.Duration; -import org.springframework.core.env.Environment; -import org.springframework.stereotype.Component; - -/** - * {@inheritDoc} - */ -@Component -public class DefaultStateSaveExceptionAnalyzer implements StateSaveExceptionAnalyzer { - - private final StateSaveExceptionHandling handling; - - /** - * Create state save exception analyzer. - * - * @param env - * The Spring environment. - */ - @Inject - public DefaultStateSaveExceptionAnalyzer(Environment env) { - Duration retryDelay = standardSeconds(env.getRequiredProperty("nflow.executor.stateSaveRetryDelay.seconds", Long.class)); - handling = new StateSaveExceptionHandling.Builder().setRetryDelay(retryDelay).build(); - } - - /** - * {@inheritDoc} - */ - @Override - public StateSaveExceptionHandling analyze(Exception e, int saveRetryCount) { - return handling; - } -} diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java index ba6192929..5d0942c2f 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/StateSaveExceptionAnalyzer.java @@ -1,5 +1,13 @@ package io.nflow.engine.exception; +import static org.joda.time.Duration.standardSeconds; +import static org.slf4j.LoggerFactory.getLogger; + +import javax.inject.Inject; + +import org.joda.time.Duration; +import org.slf4j.Logger; +import org.springframework.core.env.Environment; import org.springframework.stereotype.Component; /** @@ -7,7 +15,23 @@ * is handled. */ @Component -public interface StateSaveExceptionAnalyzer { +public class StateSaveExceptionAnalyzer { + + private static final Logger logger = getLogger(StateSaveExceptionAnalyzer.class); + + private final StateSaveExceptionHandling handling; + + /** + * Create state save exception analyzer. + * + * @param env + * The Spring environment. + */ + @Inject + public StateSaveExceptionAnalyzer(Environment env) { + Duration retryDelay = standardSeconds(env.getRequiredProperty("nflow.executor.stateSaveRetryDelay.seconds", Long.class)); + handling = new StateSaveExceptionHandling.Builder().setRetryDelay(retryDelay).build(); + } /** * Analyze the exception. @@ -18,5 +42,25 @@ public interface StateSaveExceptionAnalyzer { * How many times the saving has been attempted before this attempt. * @return How the exception should be handled. */ - StateSaveExceptionHandling analyze(Exception e, int saveRetryCount); + public final StateSaveExceptionHandling analyzeSafely(Exception e, int saveRetryCount) { + try { + return analyze(e, saveRetryCount); + } catch (Exception analyzerException) { + logger.error("Failed to analyze exception, using default handling.", analyzerException); + } + return handling; + } + + /** + * Override this to provide custom handling. + * + * @param e + * The exception to be analyzed. + * @param saveRetryCount + * How many times the saving has been attempted before this attempt. + * @return How the exception should be handled. + */ + protected StateSaveExceptionHandling analyze(Exception e, int saveRetryCount) { + return handling; + } } diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java index 296b1114f..a066d1a04 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowStateProcessor.java @@ -303,7 +303,7 @@ private WorkflowInstance saveWorkflowInstanceState(StateExecutionImpl execution, // return the original instance since persisting failed return instance; } - StateSaveExceptionHandling handling = stateSaveExceptionAnalyzer.analyze(ex, saveRetryCount++); + StateSaveExceptionHandling handling = stateSaveExceptionAnalyzer.analyzeSafely(ex, saveRetryCount++); if (handling.logStackTrace) { nflowLogger.log(logger, handling.logLevel, "Failed to save workflow instance {} new state, retrying after {} seconds.", new Object[] { instance.id, handling.retryDelay, ex }); diff --git a/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java b/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java deleted file mode 100644 index 89f0ae08f..000000000 --- a/nflow-engine/src/test/java/io/nflow/engine/exception/DefaultStateSaveExceptionAnalyzerTest.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.nflow.engine.exception; - -import static org.joda.time.Duration.standardSeconds; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import org.junit.jupiter.api.Test; -import org.slf4j.event.Level; -import org.springframework.core.env.Environment; -import org.springframework.mock.env.MockEnvironment; - -public class DefaultStateSaveExceptionAnalyzerTest { - - final Environment env = new MockEnvironment().withProperty("nflow.executor.stateSaveRetryDelay.seconds", "10"); - final StateSaveExceptionAnalyzer analyzer = new DefaultStateSaveExceptionAnalyzer(env); - - @Test - void analyzeReturnsConfiguredDelay() { - StateSaveExceptionHandling handling = analyzer.analyze(new Exception(), 0); - - assertEquals(handling.logLevel, Level.ERROR); - assertTrue(handling.logStackTrace); - assertEquals(handling.retryDelay, standardSeconds(10)); - } -} diff --git a/nflow-engine/src/test/java/io/nflow/engine/exception/StateSaveExceptionAnalyzerTest.java b/nflow-engine/src/test/java/io/nflow/engine/exception/StateSaveExceptionAnalyzerTest.java new file mode 100644 index 000000000..f9d1c508b --- /dev/null +++ b/nflow-engine/src/test/java/io/nflow/engine/exception/StateSaveExceptionAnalyzerTest.java @@ -0,0 +1,58 @@ +package io.nflow.engine.exception; + +import static org.joda.time.Duration.standardSeconds; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; +import org.slf4j.event.Level; +import org.springframework.core.env.Environment; +import org.springframework.mock.env.MockEnvironment; + +public class StateSaveExceptionAnalyzerTest { + + final Environment env = new MockEnvironment().withProperty("nflow.executor.stateSaveRetryDelay.seconds", "10"); + + @Test + void defaultAnalyzerReturnsConfiguredDelay() { + StateSaveExceptionAnalyzer analyzer = new StateSaveExceptionAnalyzer(env); + + StateSaveExceptionHandling handling = analyzer.analyzeSafely(new Exception(), 0); + + assertEquals(handling.logLevel, Level.ERROR); + assertTrue(handling.logStackTrace); + assertEquals(handling.retryDelay, standardSeconds(10)); + } + + @Test + void customerAnalyzerCanBeUsed() { + StateSaveExceptionAnalyzer analyzer = new StateSaveExceptionAnalyzer(env) { + @Override + protected StateSaveExceptionHandling analyze(Exception e, int saveRetryCount) { + return new StateSaveExceptionHandling.Builder().setRetryDelay(standardSeconds(1)).build(); + } + }; + + StateSaveExceptionHandling handling = analyzer.analyzeSafely(new Exception(), 0); + + assertEquals(handling.logLevel, Level.ERROR); + assertTrue(handling.logStackTrace); + assertEquals(handling.retryDelay, standardSeconds(1)); + } + + @Test + void defaultAnalyzerIsUsedWhenCustomerAnalyzerFails() { + StateSaveExceptionAnalyzer analyzer = new StateSaveExceptionAnalyzer(env) { + @Override + protected StateSaveExceptionHandling analyze(Exception e, int saveRetryCount) { + throw new IllegalStateException("fail"); + } + }; + + StateSaveExceptionHandling handling = analyzer.analyzeSafely(new Exception(), 0); + + assertEquals(handling.logLevel, Level.ERROR); + assertTrue(handling.logStackTrace); + assertEquals(handling.retryDelay, standardSeconds(10)); + } +} From 395aa628262440d26f64abef3409b833004bfa36 Mon Sep 17 00:00:00 2001 From: Edvard Fonsell Date: Sat, 6 Feb 2021 15:04:29 +0200 Subject: [PATCH 18/18] use default exception handler if custom handler fails --- .../DispatcherExceptionAnalyzer.java | 27 ++++++++++++- .../internal/executor/WorkflowDispatcher.java | 2 +- .../DispatcherExceptionAnalyzerTest.java | 40 +++++++++++++++++-- .../StateSaveExceptionAnalyzerTest.java | 2 +- 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionAnalyzer.java b/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionAnalyzer.java index 63a386afc..a6442a75f 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionAnalyzer.java +++ b/nflow-engine/src/main/java/io/nflow/engine/exception/DispatcherExceptionAnalyzer.java @@ -1,5 +1,8 @@ package io.nflow.engine.exception; +import static org.slf4j.LoggerFactory.getLogger; + +import org.slf4j.Logger; import org.slf4j.event.Level; import org.springframework.stereotype.Component; @@ -14,6 +17,8 @@ @Component public class DispatcherExceptionAnalyzer { + private static final Logger logger = getLogger(DispatcherExceptionAnalyzer.class); + /** * Analyze the exception. * @@ -21,7 +26,27 @@ public class DispatcherExceptionAnalyzer { * The exception to be analyzed. * @return How the exception should be handled. */ - public DispatcherExceptionHandling analyze(Exception e) { + public final DispatcherExceptionHandling analyzeSafely(Exception e) { + try { + return analyze(e); + } catch (Exception analyzerException) { + logger.error("Failed to analyze exception, using default handling.", analyzerException); + } + return getDefultHandling(e); + } + + /** + * Override this to provide custom handling. + * + * @param e + * The exception to be analyzed. + * @return How the exception should be handled. + */ + protected DispatcherExceptionHandling analyze(Exception e) { + return getDefultHandling(e); + } + + private DispatcherExceptionHandling getDefultHandling(Exception e) { Builder builder = new DispatcherExceptionHandling.Builder(); if (e instanceof PollingRaceConditionException) { builder.setLogLevel(Level.DEBUG).setLogStackTrace(false).setRandomizeSleep(true); diff --git a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java index ea8a6c2c1..8a81ef6ed 100644 --- a/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java +++ b/nflow-engine/src/main/java/io/nflow/engine/internal/executor/WorkflowDispatcher.java @@ -92,7 +92,7 @@ public void run() { dispatch(getNextInstanceIds()); } } catch (Exception e) { - DispatcherExceptionHandling handling = exceptionAnalyzer.analyze(e); + DispatcherExceptionHandling handling = exceptionAnalyzer.analyzeSafely(e); if (handling.log) { if (handling.logStackTrace) { StringBuilder sb = new StringBuilder("Exception in executing dispatcher - retrying"); diff --git a/nflow-engine/src/test/java/io/nflow/engine/exception/DispatcherExceptionAnalyzerTest.java b/nflow-engine/src/test/java/io/nflow/engine/exception/DispatcherExceptionAnalyzerTest.java index 1e51bb660..820df40ef 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/exception/DispatcherExceptionAnalyzerTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/exception/DispatcherExceptionAnalyzerTest.java @@ -16,7 +16,7 @@ public class DispatcherExceptionAnalyzerTest { @Test void analyzeGenericExceptionReturnsDefaults() { - DispatcherExceptionHandling handling = analyzer.analyze(new Exception()); + DispatcherExceptionHandling handling = analyzer.analyzeSafely(new Exception()); assertTrue(handling.log); assertEquals(handling.logLevel, Level.ERROR); @@ -27,7 +27,7 @@ void analyzeGenericExceptionReturnsDefaults() { @Test void analyzePollingRaceConditionException() { - DispatcherExceptionHandling handling = analyzer.analyze(new PollingRaceConditionException("error")); + DispatcherExceptionHandling handling = analyzer.analyzeSafely(new PollingRaceConditionException("error")); assertTrue(handling.log); assertEquals(handling.logLevel, Level.DEBUG); @@ -38,7 +38,7 @@ void analyzePollingRaceConditionException() { @Test void analyzePollingBatchException() { - DispatcherExceptionHandling handling = analyzer.analyze(new PollingBatchException("error")); + DispatcherExceptionHandling handling = analyzer.analyzeSafely(new PollingBatchException("error")); assertTrue(handling.log); assertEquals(handling.logLevel, Level.WARN); @@ -48,9 +48,41 @@ void analyzePollingBatchException() { @Test void analyzeInterruptedException() { - DispatcherExceptionHandling handling = analyzer.analyze(new InterruptedException()); + DispatcherExceptionHandling handling = analyzer.analyzeSafely(new InterruptedException()); assertFalse(handling.log); assertFalse(handling.sleep); } + + @Test + void customAnalyzerCanBeUsed() { + DispatcherExceptionAnalyzer customAnalyzer = new DispatcherExceptionAnalyzer() { + @Override + protected DispatcherExceptionHandling analyze(Exception e) { + return new DispatcherExceptionHandling.Builder().setLogStackTrace(false).build(); + } + }; + + DispatcherExceptionHandling handling = customAnalyzer.analyzeSafely(new Exception()); + + assertFalse(handling.logStackTrace); + } + + @Test + void defaultAnalyzerIsUsedWhenCustomerAnalyzerFails() { + DispatcherExceptionAnalyzer customAnalyzer = new DispatcherExceptionAnalyzer() { + @Override + protected DispatcherExceptionHandling analyze(Exception e) { + throw new IllegalStateException("fail"); + } + }; + + DispatcherExceptionHandling handling = customAnalyzer.analyzeSafely(new Exception()); + + assertTrue(handling.log); + assertEquals(handling.logLevel, Level.ERROR); + assertTrue(handling.logStackTrace); + assertTrue(handling.sleep); + assertFalse(handling.randomizeSleep); + } } diff --git a/nflow-engine/src/test/java/io/nflow/engine/exception/StateSaveExceptionAnalyzerTest.java b/nflow-engine/src/test/java/io/nflow/engine/exception/StateSaveExceptionAnalyzerTest.java index f9d1c508b..e43dd3e4f 100644 --- a/nflow-engine/src/test/java/io/nflow/engine/exception/StateSaveExceptionAnalyzerTest.java +++ b/nflow-engine/src/test/java/io/nflow/engine/exception/StateSaveExceptionAnalyzerTest.java @@ -25,7 +25,7 @@ void defaultAnalyzerReturnsConfiguredDelay() { } @Test - void customerAnalyzerCanBeUsed() { + void customAnalyzerCanBeUsed() { StateSaveExceptionAnalyzer analyzer = new StateSaveExceptionAnalyzer(env) { @Override protected StateSaveExceptionHandling analyze(Exception e, int saveRetryCount) {