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
commit fc9009d0724ff1ac25f504f6b33560bc6893a704 Author: Alex Heneveld <[email protected]> AuthorDate: Fri Feb 2 13:14:17 2024 +0000 log handled workflow errors and explicit fail steps as debug not warn --- .../core/workflow/WorkflowErrorHandling.java | 10 +++++- .../workflow/WorkflowPersistReplayErrorsTest.java | 42 +++++++++++++++------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java index ea39a0d1ff..e93c30dd4d 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java @@ -23,6 +23,7 @@ import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.mgmt.BrooklynTaskTags; +import org.apache.brooklyn.core.workflow.steps.flow.FailWorkflowStep.WorkflowFailException; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.core.predicates.DslPredicates; import org.apache.brooklyn.util.core.task.DynamicTasks; @@ -242,6 +243,9 @@ public class WorkflowErrorHandling implements Callable<WorkflowErrorHandling.Wor } catch (Exception e2) { if (Exceptions.getCausalChain(e2).stream().anyMatch(e3 -> e3== error)) { // is, or wraps, original error, don't need to log + } else if (e2 instanceof WorkflowFailException) { + log.debug("Workflow fail in step '"+stepTaskThrowingError.getDisplayName()+"'; new error -- "+Exceptions.collapseText(e2)+" -- replacing original error: "+Exceptions.collapseText(error)); + log.trace("Full trace of original error was: " + error, error); } else { logWarnOnExceptionOrDebugIfKnown(entity, e2, "Error in step '" + stepTaskThrowingError.getDisplayName() + "' error handler for -- " + Exceptions.collapseText(error) + " -- threw another error (rethrowing): " + Exceptions.collapseText(e2)); log.debug("Full trace of original error was: " + error, error); @@ -257,9 +261,13 @@ public class WorkflowErrorHandling implements Callable<WorkflowErrorHandling.Wor // don't consider replaying automatically here; only done at workflow level - logWarnOnExceptionOrDebugIfKnown(entity, error, + // previously logged as warning, but author has done everything right so we shouldn't warn, just debug +// logWarnOnExceptionOrDebugIfKnown(entity, error, + log.debug( currentStepInstance.getWorkflowExectionContext().getName() + ": " + "Error in step '" + stepTaskThrowingError.getDisplayName() + "'; " + problemHere + "rethrowing: " + Exceptions.collapseText(error)); + log.trace("Trace for error being rethrown", error); + throw Exceptions.propagate(error); } 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 88962ea712..e0635e174a 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 @@ -18,6 +18,8 @@ */ package org.apache.brooklyn.core.workflow; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.spi.ILoggingEvent; import com.google.common.base.Stopwatch; import com.google.common.collect.Iterables; import org.apache.brooklyn.api.entity.EntityLocal; @@ -648,18 +650,32 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl @Test public void testSimpleErrorHandlerOnWorkflowFailing() throws IOException { - lastInvocation = runSteps(MutableList.of("invoke-effector does-not-exist"), - 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("does-not-exist")).findAny().isPresent()); - return true; - }); - Asserts.assertEquals(app.sensors().get(Sensors.newSensor(Object.class, "had_error")), "yes"); - findSingleLastWorkflow(); - Asserts.assertEquals(lastWorkflowContext.status, WorkflowExecutionContext.WorkflowStatus.ERROR); + 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("invoke-effector does-not-exist"), + 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("does-not-exist")).findAny().isPresent()); + return true; + }); + + List<String> msgs = logWatcher.getMessages(); + if (!msgs.isEmpty()) { + log.info("Error handler output:\n" + msgs.stream().collect(Collectors.joining("\n"))); + Asserts.fail("No info/warn log messages expected if workflow has error handler, even if it runs an explicit fail rethrow step"); + } + + Asserts.assertEquals(app.sensors().get(Sensors.newSensor(Object.class, "had_error")), "yes"); + findSingleLastWorkflow(); + Asserts.assertEquals(lastWorkflowContext.status, WorkflowExecutionContext.WorkflowStatus.ERROR); + } } @Test @@ -705,7 +721,7 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl } @Test - public void testErrorHandlerRethrows() throws IOException { + public void testErrorHandlerRethrowsIfConditionNotMatched() throws IOException { lastInvocation = runSteps(MutableList.of( MutableMap.of("step", "fail message expected exception", "output", "should have failed",
