This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
The following commit(s) were added to refs/heads/master by this push: new 7513c0586c better warning for errors in workflow step config (between steps) 7513c0586c is described below commit 7513c0586ca6e9f0bb075a241d00f2bac635f443 Author: Alex Heneveld <a...@cloudsoft.io> AuthorDate: Fri Feb 2 17:03:32 2024 +0000 better warning for errors in workflow step config (between steps) --- .../core/workflow/WorkflowExecutionContext.java | 7 ++++- .../workflow/WorkflowPersistReplayErrorsTest.java | 34 +++++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java index deef3d0353..28e19b5ec3 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java @@ -1210,7 +1210,12 @@ public class WorkflowExecutionContext { } else if (onError != null && (!(onError instanceof Collection) || !((Collection)onError).isEmpty())) { try { - log.debug("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + ", running error handler"); + if (currentStepInstance.getError()==null) { + log.warn("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + ", running error handler but likely this should be corrected in code -- " + Exceptions.collapseText(e)); + log.debug("Trace of error:", e); + } else { + log.debug("Error in workflow '" + getName() + "' around step " + workflowStepReference(currentStepIndex) + ", running error handler"); + } Task<WorkflowErrorHandling.WorkflowErrorHandlingResult> workflowErrorHandlerTask = WorkflowErrorHandling.createWorkflowErrorHandlerTask(WorkflowExecutionContext.this, task, e); errorHandlerTaskId = workflowErrorHandlerTask.getId(); WorkflowErrorHandling.WorkflowErrorHandlingResult result = DynamicTasks.queue(workflowErrorHandlerTask).getUnchecked(); diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java index e0635e174a..73e242de5f 100644 --- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java @@ -47,6 +47,7 @@ import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; +import org.apache.brooklyn.util.core.task.DeferredSupplier; import org.apache.brooklyn.util.core.task.DynamicTasks; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.guava.Maybe; @@ -649,7 +650,7 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl } @Test - public void testSimpleErrorHandlerOnWorkflowFailing() throws IOException { + public void testSimpleErrorHandlerOnWorkflowFailingWorksAndDoesntLogWarning() throws IOException { try (ClassLogWatcher logWatcher = new ClassLogWatcher(getClass().getPackage().getName()) { @Override protected void append(ILoggingEvent iLoggingEvent) { @@ -678,6 +679,37 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl } } + @Test + public void testSimpleErrorHandlerOnWorkflowTriggeredByConditionAssertWorksAndDoesLog() throws IOException { + try (ClassLogWatcher logWatcher = new ClassLogWatcher(getClass().getPackage().getName()) { + @Override + protected void append(ILoggingEvent iLoggingEvent) { + if (iLoggingEvent.getLevel().isGreaterOrEqual(Level.INFO)) super.append(iLoggingEvent); + } + }) { + lastInvocation = runSteps(MutableList.of(MutableMap.of("condition", + MutableMap.of("assert", MutableMap.of("config", "missing_will_cause_to_throw")), + "step", "return true")), + null, + ConfigBag.newInstance().configure( + WorkflowEffector.ON_ERROR, MutableList.of("set-sensor had_error = yes", "fail rethrow message rethrown"))); + Asserts.assertFailsWith(() -> lastInvocation.getUnchecked(), e -> { + Asserts.expectedFailureContains(e, "rethrown"); + Asserts.assertThat(Exceptions.getCausalChain(e), ee -> ee.stream().filter(e2 -> e2.toString().contains("missing_will_cause_to_throw")).findAny().isPresent()); + return true; + }); + + List<String> msgs = logWatcher.getMessages(); + log.info("Error handler output:\n"+msgs.stream().collect(Collectors.joining("\n"))); + Asserts.assertSize(msgs, 1); + Asserts.assertNotNull(msgs.stream().filter(s -> s.toLowerCase().matches(".*assertion.* failed.* config.*missing_will_cause_to_throw.*")).findAny().orElse(null)); + + Asserts.assertEquals(app.sensors().get(Sensors.newSensor(Object.class, "had_error")), "yes"); + findSingleLastWorkflow(); + Asserts.assertEquals(lastWorkflowContext.status, WorkflowExecutionContext.WorkflowStatus.ERROR); + } + } + @Test public void testErrorHandlerListWithGotoExit() throws IOException { try (ClassLogWatcher logWatcher = new ClassLogWatcher(getClass().getPackage().getName())) {