From 3a1a8c3d40c541df49bb51ef97bbf7cb3246e16c Mon Sep 17 00:00:00 2001 From: Iliyan Velichkov Date: Tue, 8 Sep 2026 10:25:44 +0300 Subject: [PATCH] feat(intent): step resilience reaches a notify: service task (#7056) `retry:` / `onError:` were accepted on a `delegate:` service task only, while the generated sender for a `notify:` step *does* fail the task on a delivery error - on purpose, since the message is that step's whole work. So the send was the one step in a process whose failure had nowhere to go: it took Flowable's default job path and ended as a dead-letter incident. Ordered where it reads most naturally, before the terminal status write, an SMTP blink then left a record whose real work had completed sitting in its in-progress status with no error message and no failure status - the incident is on the job, not the record, so nothing in the application could see it. The workaround, making the send the last step, constrained process design for a reason unrelated to the domain. v1 was delegate-only for a runtime reason, not a modelling one: the conversion that turns an exhausted attempt into the caught INTENT_STEP_FAILED error lives on the flowable:class path, and a notify step is emitted as flowable:delegateExpression="${JavaTask}", which never passes through ClassDelegate. Flowable exposes no ClassDelegateFactory analogue there, so the hook is the overridable factory method: ResilientActivityBehaviorFactory (now what BpmFlowableConfig configures, still carrying ResilientClassDelegateFactory) returns a ResilientServiceTaskDelegateExpressionActivityBehavior, which wraps the single protected handleException the superclass funnels execute / trigger / the future-delegate completion through. Wrapping rather than re-implementing the catch is the whole design: the superclass propagates a BpmnError the delegate raised itself and applies any flowable:mapException on its own, and only rethrows a plain unmapped failure - exactly the predicate the flowable:class twin relies on. So IntentStepResilience is unchanged: same signature, same string contracts, same JobRetryCmd arithmetic. Emission adds only the cycle on the ${JavaTask} element, in the extensionElements block the handler field already opens; the , the boundary, its flow and its DI were already shape-agnostic. A send keeps its async boundary (the synchronousNodes / completingTransactionNodes walk strips it only from check-gated setters and the decisions reaching them), which is what makes a cycle re-run anything at all. Three refusals, each because the declaration would otherwise be authored and inert: a setField/setRelationField step (a check-gated status write is refused synchronously to the person who acted - #7014/#7063 - so routing it away would take that 400 out of the Inbox, and re-attempting a deterministic refusal recovers nothing), a call: step and the bare custom. fallback, and a fan-out send (notify.forEach), which is per-row fail-soft and therefore never fails the task - its outcome is observed with `outcome:` + `onNotifyFailed` instead. The generated sender's exception message also gained its cause, since that string is verbatim what an onError route records through {error}. Covered at every layer: the parser's shape refusals, the emitted cycle on the delegate-expression element, both emitted shapes against the real Flowable converter, a live-engine twin of the delegate test (including the must-not-break case - a delegate raising its own BpmnError still reaches its own boundary), IntentEngineIT's emission and parse-rejection tests, IntentEmissionCoverageIT's runtime scenario (a send mid-process whose exhausted retry routes onto the record and lets the flow carry on to the hold), and the sample-intent-resilience fixture - which gains a third tenant title exercising exactly the issue's reproducer. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/docs/intent-layer.md | 2 +- .../flowable/config/BpmFlowableConfig.java | 15 +- .../ResilientActivityBehaviorFactory.java | 52 ++++ ...askDelegateExpressionActivityBehavior.java | 63 +++++ .../IntentResilienceBpmnContractTest.java | 82 +++++- ...rviceTaskDelegateExpressionEngineTest.java | 241 ++++++++++++++++++ components/engine/engine-intent/CLAUDE.md | 2 +- .../generator/bpmn/BpmnIntentGenerator.java | 14 +- .../intent/parser/IntentParser.java | 68 ++++- .../main/resources/intent-assistant-guide.md | 49 +++- .../intent/generator/ResilienceBpmnTest.java | 76 +++++- .../parser/StepResilienceIntentTest.java | 89 ++++++- .../events/Send.java.template | 13 +- .../tests/api/IntentEmissionCoverageIT.java | 52 +++- .../integration/tests/api/IntentEngineIT.java | 43 +++- .../SampleIntentResilienceTestProject.java | 43 +++- .../sample-intent-resilience/README.md | 40 ++- .../sample-intent-resilience/app.intent | 21 +- 18 files changed, 894 insertions(+), 71 deletions(-) create mode 100644 components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientActivityBehaviorFactory.java create mode 100644 components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientServiceTaskDelegateExpressionActivityBehavior.java create mode 100644 components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientServiceTaskDelegateExpressionEngineTest.java diff --git a/.claude/docs/intent-layer.md b/.claude/docs/intent-layer.md index a21bcbd4c1c..ca6e979db6d 100644 --- a/.claude/docs/intent-layer.md +++ b/.claude/docs/intent-layer.md @@ -38,7 +38,7 @@ A single `app.intent` YAML file at a project root is the source of truth one alt **Event-driven create-from (`generates` + `event:`, [#6711](https://github.com/eclipse-dirigible/dirigible/issues/6711)):** a `generates` entry may declare `event: { onTransition: , when: " == " }` (guard mandatory) or `{ onCreate: }` and mint the follow-up **document — header AND items** by itself when the source reaches a state, instead of waiting for the button (`posts` is event-driven but emits flat rows and cannot reference the new header). The `map` entry copying the source's key IS the back-reference and therefore the **at-most-once** guard, derived rather than declared twice; the button is dropped unless `button: true`, and both triggers share ONE generated create-from (a new `GenerateOnEvent.java.template` listener calls `Generate.create(id)` and carries no mapping of its own). Details in the engine-intent guide's `event:` bullet. -**Declarative step resilience (`retry:` / `onError:` / `{error}` / `vars:`, [#6762](https://github.com/eclipse-dirigible/dirigible/issues/6762)):** a `delegate:` service task may declare `retry: { count, every }` (a Flowable failed-job retry cycle, `R/` — the R number counts TOTAL attempts) and `onError: ` (an error boundary event routed like a decision branch). The boundary only fires on a `BpmnError`, so `engine-bpm-flowable`'s `ResilientClassDelegateFactory` converts the FINAL failed attempt (mirroring `JobRetryCmd`'s arithmetic) into the caught `INTENT_STEP_FAILED` error, publishing the failure message as the `__errorMessage` variable — which a `setField` step on the error route reads via the whole-value `{error}` token. Process-level `vars:` + step `produces:`/`uses:` declare step data (an undeclared name is a parse error) and `clearAfter: ` removes a value — a generated credential — from the instance data once that step completes. v1 is delegate-only (parser-enforced); absent keys generate byte-identically. Details in the engine-intent guide's step-resilience bullet. +**Declarative step resilience (`retry:` / `onError:` / `{error}` / `vars:`, [#6762](https://github.com/eclipse-dirigible/dirigible/issues/6762) + [#7056](https://github.com/eclipse-dirigible/dirigible/issues/7056)):** a `delegate:` or a `notify:` service task may declare `retry: { count, every }` (a Flowable failed-job retry cycle, `R/` — the R number counts TOTAL attempts) and `onError: ` (an error boundary event routed like a decision branch). The boundary only fires on a `BpmnError`, so `engine-bpm-flowable` converts the FINAL failed attempt (mirroring `JobRetryCmd`'s arithmetic) into the caught `INTENT_STEP_FAILED` error, publishing the failure message as the `__errorMessage` variable — which a `setField` step on the error route reads via the whole-value `{error}` token. Process-level `vars:` + step `produces:`/`uses:` declare step data (an undeclared name is a parse error) and `clearAfter: ` removes a value — a generated credential — from the instance data once that step completes. **The SEND was the one step whose failure had nowhere to go** ([#7056](https://github.com/eclipse-dirigible/dirigible/issues/7056)): its generated handler fails the task on a delivery error on purpose, but the conversion existed only on the `flowable:class` path, so an SMTP blink dead-lettered the job and left a record whose real work had completed sitting in its in-progress status with no message and no failure status — the incident being on the job, not the record — and the only workaround was to make the send the last step. `ResilientActivityBehaviorFactory` adds the second hook on the `flowable:delegateExpression` path (`${JavaTask}`/`${JSTask}`) by wrapping `ServiceTaskDelegateExpressionActivityBehavior`'s own `handleException`, so a `BpmnError` a delegate raises itself and a `flowable:mapException` keep the stock behaviour and only a plain unmapped failure converts; `IntentStepResilience` is unchanged. Refused at parse, each because the declaration would otherwise never fire: a `setField`/`setRelationField` step (a check-gated status write is refused synchronously to the person who acted), a `call:` or bare service task, and a **fan-out** send (`notify.forEach`), which is per-row fail-soft and never fails the task — observe that one with `outcome:` + `onNotifyFailed`. Absent keys generate byte-identically. Details in the engine-intent guide's step-resilience bullet. **`permissions.can` is what the generated app enforces ([#6760](https://github.com/eclipse-dirigible/dirigible/issues/6760)):** the authored access model and the enforced one were two disjoint namespaces - `permissions[].role` became `.roles` while every generated controller's gate was the convention-derived `..FullAccess`, a name no intent construct mentions, so granting an authored role granted *nothing* and nothing errored or warned (the roles did show up in the UI, so it looked wired). `PermissionSupport` resolves the `can: [Resource:action]` tokens into per-resource read / write role sets that the EDM and report generators emit as the entity's / report's own `roleRead` / `roleWrite`: a **covered entity is gated entirely by the authored roles** (its convention roles are neither the gate nor declared, or `.roles` and the template's `default-roles.roles` would each declare the same name), a **composition child inherits the master's** grants when it has none of its own, a covered entity **no grant may write keeps a write gate no declared role satisfies** - what a read-only allow-list says - and an entity no token names is byte-identical to before. The action **vocabulary is closed**: `read`/`view`/`list` → read, `write`/`create`/`update`/`edit`/`delete`/`manage` → write **and** read (a caller who may change a record must be able to load it), `*`/`all` → both; anything else (`approve`, `start`) is a business action with no generated URL and becomes a generation **advisory** naming the token rather than a silent drop, an undeclared resource becomes an **issue**, and a malformed token is refused at **parse**. Since the gate may now name several roles, the rest-java controllers' entity/report gate went from `UserFacade.isInRole` to the any-of `isInAnyRole` the per-property `visibleTo:` machinery already used. The URL half - `.access` over the controller subtrees, generated pages and report pages the templates publish - is **opt-in through the project's `.settings`** (`{"access": {"generate": true}}`), not a DSL key, and carries method `*` on purpose: the generated controllers read through `POST .../search`, so a "POST means write" split would lock a read-only role out of every list. **A hand-authored `.access` at the project root is scrub-owned** (`.access` is an intent-owned extension) - hand-written constraints belong under `custom/`. diff --git a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/config/BpmFlowableConfig.java b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/config/BpmFlowableConfig.java index bc05fa02203..d2194ff8a70 100644 --- a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/config/BpmFlowableConfig.java +++ b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/config/BpmFlowableConfig.java @@ -14,12 +14,12 @@ import org.eclipse.dirigible.commons.config.DirigibleConfig; import org.eclipse.dirigible.components.data.sources.manager.DataSourcesManager; import org.eclipse.dirigible.components.engine.bpm.BpmProvider; +import org.eclipse.dirigible.components.engine.bpm.flowable.delegate.ResilientActivityBehaviorFactory; import org.eclipse.dirigible.components.engine.bpm.flowable.delegate.ResilientClassDelegateFactory; import org.eclipse.dirigible.components.engine.bpm.flowable.diagram.DirigibleProcessDiagramGenerator; import org.eclipse.dirigible.engine.java.runtime.ClientClassLoaderHolder; import org.flowable.engine.ProcessEngine; import org.flowable.engine.ProcessEngineConfiguration; -import org.flowable.engine.impl.bpmn.parser.factory.DefaultActivityBehaviorFactory; import org.flowable.spring.SpringProcessEngineConfiguration; import org.flowable.spring.boot.actuate.endpoint.ProcessEngineEndpoint; import org.flowable.spring.boot.actuate.info.FlowableInfoContributor; @@ -120,11 +120,14 @@ private SpringProcessEngineConfiguration createProcessEngineConfig(DataSource da // (FlowableClientClassLoaderRefresher additionally evicts the parsed-process cache on rebuild). config.setUseClassForNameClassLoading(false); - // Run every flowable:class task through the resilient ClassDelegate: a task carrying an intent - // onError error boundary has its FINAL failed attempt converted to the caught BPMN error - // (message published for {error}) instead of dead-lettering; everything else is untouched. The - // engine's initBehaviorFactory injects the expression manager into this factory later. - config.setActivityBehaviorFactory(new DefaultActivityBehaviorFactory(new ResilientClassDelegateFactory())); + // Run every service task through the resilient behaviours - the flowable:class ones through the + // resilient ClassDelegate, the flowable:delegateExpression ones (${JavaTask} / ${JSTask}, i.e. + // every generated handler) through the resilient delegate-expression behaviour: a task carrying + // an intent onError error boundary has its FINAL failed attempt converted to the caught BPMN + // error (message published for {error}) instead of dead-lettering; everything else is + // untouched. The engine's initBehaviorFactory injects the expression manager into this factory + // later. + config.setActivityBehaviorFactory(new ResilientActivityBehaviorFactory(new ResilientClassDelegateFactory())); return config; } diff --git a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientActivityBehaviorFactory.java b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientActivityBehaviorFactory.java new file mode 100644 index 00000000000..87c000ccbf8 --- /dev/null +++ b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientActivityBehaviorFactory.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2010-2026 Eclipse Dirigible contributors + * + * All rights reserved. This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v2.0 which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v20.html + * + * SPDX-FileCopyrightText: Eclipse Dirigible contributors SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.dirigible.components.engine.bpm.flowable.delegate; + +import org.flowable.bpmn.model.ServiceTask; +import org.flowable.common.engine.api.delegate.Expression; +import org.flowable.engine.impl.bpmn.behavior.ServiceTaskDelegateExpressionActivityBehavior; +import org.flowable.engine.impl.bpmn.helper.ClassDelegateFactory; +import org.flowable.engine.impl.bpmn.parser.factory.DefaultActivityBehaviorFactory; + +/** + * The behaviour factory the engine is configured with, so the intent DSL's {@code onError} error + * routing has its conversion hook on both service-task paths: the {@code flowable:class} + * one through the {@link ClassDelegateFactory} this is constructed with (which yields + * {@link ResilientClassDelegate}s), and the {@code flowable:delegateExpression} one through the + * override below (which yields {@link ResilientServiceTaskDelegateExpressionActivityBehavior}s). + * + *

+ * Flowable exposes no {@code ClassDelegateFactory} analogue for the delegate-expression path, so + * the hook has to be the overridable factory method itself. The construction mirrors the + * superclass's own exactly - only the behaviour type differs - and {@code expressionManager} is the + * inherited field the engine's {@code initBehaviorFactory} injects into this instance after + * configuration. + * + *

+ * Both {@code ${JavaTask}} and {@code ${JSTask}} therefore run through the conversion. That is + * wider than the DSL currently allows anyone to ask for, and deliberately harmless: the conversion + * fires only on a task carrying a boundary event catching {@code INTENT_STEP_FAILED}, which nothing + * but the intent BPMN generator emits, and which shapes may declare an {@code onError:} at all is + * decided by the intent parser. + */ +public class ResilientActivityBehaviorFactory extends DefaultActivityBehaviorFactory { + + public ResilientActivityBehaviorFactory(ClassDelegateFactory classDelegateFactory) { + super(classDelegateFactory); + } + + @Override + public ServiceTaskDelegateExpressionActivityBehavior createServiceTaskDelegateExpressionActivityBehavior(ServiceTask serviceTask) { + Expression delegateExpression = expressionManager.createExpression(serviceTask.getImplementation()); + return new ResilientServiceTaskDelegateExpressionActivityBehavior(serviceTask.getId(), delegateExpression, + getSkipExpressionFromServiceTask(serviceTask), createFieldDeclarations(serviceTask.getFieldExtensions()), + serviceTask.getMapExceptions(), serviceTask.isTriggerable()); + } +} diff --git a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientServiceTaskDelegateExpressionActivityBehavior.java b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientServiceTaskDelegateExpressionActivityBehavior.java new file mode 100644 index 00000000000..3d90cdc1b43 --- /dev/null +++ b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientServiceTaskDelegateExpressionActivityBehavior.java @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2010-2026 Eclipse Dirigible contributors + * + * All rights reserved. This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v2.0 which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v20.html + * + * SPDX-FileCopyrightText: Eclipse Dirigible contributors SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.dirigible.components.engine.bpm.flowable.delegate; + +import java.util.List; + +import org.flowable.bpmn.model.MapExceptionEntry; +import org.flowable.common.engine.api.delegate.Expression; +import org.flowable.engine.delegate.DelegateExecution; +import org.flowable.engine.impl.bpmn.behavior.ServiceTaskDelegateExpressionActivityBehavior; +import org.flowable.engine.impl.bpmn.parser.FieldDeclaration; + +/** + * The behaviour every {@code flowable:delegateExpression} service task runs through (created by + * {@link ResilientActivityBehaviorFactory}), adding the intent DSL's step resilience to the second + * of the two service-task paths: when the resolved delegate's FINAL failed attempt happens on a + * task carrying an intent {@code onError} error boundary, the failure is converted into the caught + * BPMN error instead of dead-lettering - see {@link IntentStepResilience}. + * + *

+ * This is the twin of {@link ResilientClassDelegate}, which covers the {@code flowable:class} path. + * It exists because the generated senders, setters, resolvers and loaders are all bound through the + * {@code ${JavaTask}} / {@code ${JSTask}} dispatchers, which never pass through + * {@code ClassDelegate} - so until dirigible #7056 a {@code notify:} step could not declare + * {@code retry:} / {@code onError:} at all, while its generated sender did fail the task on a + * delivery error (the send was the one step in a process whose failure had nowhere to go). + * + *

+ * The hook is {@link #handleException}, the single funnel the superclass routes {@code execute}, + * {@code trigger} and the future-delegate completion through. Wrapping it - rather than + * re-implementing the catch - is what keeps the stock semantics intact: the superclass propagates a + * {@link org.flowable.engine.delegate.BpmnError} the delegate threw itself and applies a matching + * {@code flowable:mapException} on its own, and only rethrows a plain, unmapped failure. That + * rethrow is exactly the failure destined for the retry cycle / dead-letter path, i.e. the same + * predicate the {@code flowable:class} twin relies on. + */ +class ResilientServiceTaskDelegateExpressionActivityBehavior extends ServiceTaskDelegateExpressionActivityBehavior { + + private static final long serialVersionUID = 1L; + + ResilientServiceTaskDelegateExpressionActivityBehavior(String serviceTaskId, Expression expression, Expression skipExpression, + List fieldDeclarations, List mapExceptions, boolean triggerable) { + super(serviceTaskId, expression, skipExpression, fieldDeclarations, mapExceptions, triggerable); + } + + @Override + protected void handleException(Throwable exception, DelegateExecution execution, boolean loggingSessionEnabled) { + try { + super.handleException(exception, execution, loggingSessionEnabled); + } catch (RuntimeException unhandled) { + if (!IntentStepResilience.convertFinalFailure(execution, unhandled)) { + throw unhandled; + } + } + } +} diff --git a/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/IntentResilienceBpmnContractTest.java b/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/IntentResilienceBpmnContractTest.java index 45ad26866b7..b7f394593ee 100644 --- a/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/IntentResilienceBpmnContractTest.java +++ b/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/IntentResilienceBpmnContractTest.java @@ -24,12 +24,19 @@ import org.junit.jupiter.api.Test; /** - * The BPMN shape the intent generator emits for step resilience (dirigible #6762), parsed by the - * REAL Flowable converter this engine runs: the {@code failedJobRetryTimeCycle} extension element - * must land on the service task (or the declared retry silently never runs), and the error boundary - * must resolve to the {@code INTENT_STEP_FAILED} code {@code IntentStepResilience} raises. This is - * the string contract between {@code engine-intent}'s {@code BpmnIntentGenerator} and this module - - * neither side depends on the other in code, so this test is where a drift surfaces. + * The BPMN shapes the intent generator emits for step resilience (dirigible #6762 and #7056), + * parsed by the REAL Flowable converter this engine runs: the {@code failedJobRetryTimeCycle} + * extension element must land on the service task (or the declared retry silently never runs), and + * the error boundary must resolve to the {@code INTENT_STEP_FAILED} code + * {@code IntentStepResilience} raises. This is the string contract between {@code engine-intent}'s + * {@code BpmnIntentGenerator} and this module - neither side depends on the other in code, so this + * test is where a drift surfaces. + * + *

+ * Both emission paths are covered, because both now have a conversion hook: the {@code delegate:} + * step's {@code flowable:class} element and the {@code notify:} step's {@code ${JavaTask}} + * delegate-expression element, where the cycle shares an extensionElements block with the + * {@code handler} field the dispatcher reads. */ class IntentResilienceBpmnContractTest { @@ -57,6 +64,32 @@ class IntentResilienceBpmnContractTest { """; + private static final String GENERATED_SEND_SHAPE = + """ + + + + + + + + + + + R2/PT5S + + + + + + + + + + + + """; + @Test void theGeneratedResilienceShapeParsesIntoWhatTheRuntimeReads() throws Exception { XMLStreamReader reader = XMLInputFactory.newInstance() @@ -84,4 +117,41 @@ void theGeneratedResilienceShapeParsesIntoWhatTheRuntimeReads() throws Exception // boundary events (a converter quirk) - the engine treats them as interrupting regardless, // which ResilientClassDelegateEngineTest proves against a live instance. } + + /** + * The send shape (#7056). Two things have to survive the converter here that the + * {@code flowable:class} shape does not exercise: the cycle sits in the SAME extensionElements + * block as the {@code handler} field the {@code ${JavaTask}} dispatcher reads, and the + * implementation is a delegate expression rather than a class - which is what + * {@code ResilientActivityBehaviorFactory} hooks. + */ + @Test + void theGeneratedSendShapeParsesIntoWhatTheRuntimeReads() throws Exception { + XMLStreamReader reader = XMLInputFactory.newInstance() + .createXMLStreamReader(new StringReader(GENERATED_SEND_SHAPE)); + BpmnModel model = new BpmnXMLConverter().convertToBpmnModel(reader); + + ServiceTask task = (ServiceTask) model.getMainProcess() + .getFlowElement("notifyOwner"); + assertEquals("R2/PT5S", task.getFailedJobRetryTimeCycleValue(), + "the retry cycle must land on the task even when it shares the block with the handler field"); + assertEquals("delegateExpression", task.getImplementationType(), "a send is bound through the delegate-expression dispatcher"); + assertEquals("${JavaTask}", task.getImplementation()); + assertEquals(1, task.getFieldExtensions() + .size(), + "the handler field must still parse next to the cycle"); + assertEquals("handler", task.getFieldExtensions() + .get(0) + .getFieldName()); + + BoundaryEvent boundary = (BoundaryEvent) model.getMainProcess() + .getFlowElement("notifyOwnerError"); + assertEquals("notifyOwner", boundary.getAttachedToRefId()); + ErrorEventDefinition definition = (ErrorEventDefinition) boundary.getEventDefinitions() + .get(0); + String resolved = model.getErrors() + .get(definition.getErrorCode()); + assertEquals(IntentStepResilience.ERROR_CODE, resolved != null ? resolved : definition.getErrorCode(), + "the boundary must resolve to the code IntentStepResilience raises"); + } } diff --git a/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientServiceTaskDelegateExpressionEngineTest.java b/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientServiceTaskDelegateExpressionEngineTest.java new file mode 100644 index 00000000000..7c28029f7ee --- /dev/null +++ b/components/engine/engine-bpm-flowable/src/test/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/ResilientServiceTaskDelegateExpressionEngineTest.java @@ -0,0 +1,241 @@ +/* + * Copyright (c) 2010-2026 Eclipse Dirigible contributors + * + * All rights reserved. This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v2.0 which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v20.html + * + * SPDX-FileCopyrightText: Eclipse Dirigible contributors SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.dirigible.components.engine.bpm.flowable.delegate; + +import static org.awaitility.Awaitility.await; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import org.flowable.engine.HistoryService; +import org.flowable.engine.ProcessEngine; +import org.flowable.engine.delegate.BpmnError; +import org.flowable.engine.delegate.DelegateExecution; +import org.flowable.engine.delegate.JavaDelegate; +import org.flowable.engine.impl.cfg.StandaloneInMemProcessEngineConfiguration; +import org.flowable.engine.runtime.ProcessInstance; +import org.flowable.variable.api.history.HistoricVariableInstance; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * The resilience conversion on the {@code flowable:delegateExpression} path against a REAL Flowable + * engine (in-memory, async executor on) - the second of the two hooks, added for dirigible #7056 so + * a {@code notify:} step (emitted as {@code ${JavaTask}}, which never passes through + * {@code ClassDelegate}) can declare {@code retry:} / {@code onError:} at all. + * + *

+ * Three properties, and the third is the one the wrapper is responsible for not breaking: the + * declared retry cycle re-runs the resolved delegate and its FINAL failed attempt converts to the + * caught intent error (routing through the boundary with the message published for + * {@code {error}}); a delegate that recovers within its cycle completes with no conversion at all; + * and a delegate that raises a {@link BpmnError} itself still reaches its own boundary, + * because the wrapper converts only what the superclass rethrew. + */ +class ResilientServiceTaskDelegateExpressionEngineTest { + + /** + * One shape for all three cases: an async delegate-expression task carrying a retry cycle, with two + * boundaries - the intent one the conversion raises, and a business one a delegate raises itself - + * each routing to a step that records what it saw. + */ + private static final String PROCESS_XML_TEMPLATE = + """ + + + + + + + + + %s + + + + + + + + + + + + + + + + + + + + + + """; + + private static ProcessEngine engine; + + /** Always fails; carries the attempt number so the recorded message pins WHICH attempt routed. */ + public static class DoomedDelegate implements JavaDelegate { + + static final AtomicInteger ATTEMPTS = new AtomicInteger(); + + @Override + public void execute(DelegateExecution execution) { + throw new IllegalStateException("refused (attempt " + ATTEMPTS.incrementAndGet() + ")"); + } + } + + /** Fails twice, succeeds on the third attempt - within its declared R3 cycle. */ + public static class FlakyDelegate implements JavaDelegate { + + static final AtomicInteger ATTEMPTS = new AtomicInteger(); + + @Override + public void execute(DelegateExecution execution) { + int attempt = ATTEMPTS.incrementAndGet(); + if (attempt < 3) { + throw new IllegalStateException("flaky (attempt " + attempt + ")"); + } + execution.setVariable("result", "OK-" + attempt); + } + } + + /** Raises its own BPMN error - which the wrapper must leave to its own boundary, unconverted. */ + public static class RefusingDelegate implements JavaDelegate { + + @Override + public void execute(DelegateExecution execution) { + throw new BpmnError("BUSINESS_REFUSED", "the customer is on hold"); + } + } + + /** + * The intent error route: records what {@code {error}} reads, like the generated setField delegate. + */ + public static class RecordFailureDelegate implements JavaDelegate { + + @Override + public void execute(DelegateExecution execution) { + execution.setVariable("recorded", String.valueOf(execution.getVariable(IntentStepResilience.ERROR_MESSAGE_VARIABLE))); + } + } + + /** The business error route. */ + public static class RecordRefusalDelegate implements JavaDelegate { + + @Override + public void execute(DelegateExecution execution) { + execution.setVariable("refused", "yes"); + } + } + + @BeforeAll + static void startEngine() { + StandaloneInMemProcessEngineConfiguration configuration = new StandaloneInMemProcessEngineConfiguration(); + configuration.setJdbcUrl("jdbc:h2:mem:resilience-delegate-expression-test;DB_CLOSE_DELAY=1000"); + configuration.setActivityBehaviorFactory(new ResilientActivityBehaviorFactory(new ResilientClassDelegateFactory())); + // The beans a ${...} delegate expression resolves against - the standalone counterpart of the + // Spring bean factory the platform's engine is configured with (where ${JavaTask} lives). + Map beans = new HashMap<>(); + beans.put("doomed", new DoomedDelegate()); + beans.put("flaky", new FlakyDelegate()); + beans.put("refusing", new RefusingDelegate()); + beans.put("recordFailure", new RecordFailureDelegate()); + beans.put("recordRefusal", new RecordRefusalDelegate()); + configuration.setBeans(beans); + configuration.setAsyncExecutorActivate(true); + // Tight acquire cycles so the PT1S retry waits dominate the test's wall clock. + configuration.getAsyncExecutorConfiguration() + .setDefaultAsyncJobAcquireWaitTime(Duration.ofMillis(100)); + configuration.getAsyncExecutorConfiguration() + .setDefaultTimerJobAcquireWaitTime(Duration.ofMillis(100)); + engine = configuration.buildProcessEngine(); + } + + @AfterAll + static void stopEngine() { + if (engine != null) { + engine.close(); + } + } + + @Test + void anExhaustedRetryCycleRoutesTheFinalAttemptsMessageThroughTheErrorBoundary() { + deploy("doomed-expression", "doomed", "R2/PT1S"); + ProcessInstance instance = engine.getRuntimeService() + .startProcessInstanceByKey("doomed-expression"); + + // R2 = two total attempts; the SECOND one converts instead of dead-lettering, so the flow + // ends through the error route with that attempt's message recorded. + assertEquals("refused (attempt 2)", historicVariable(instance, "recorded"), + "the error route must record the FINAL attempt's failure message"); + assertEquals(0, engine.getManagementService() + .createDeadLetterJobQuery() + .count(), + "the converted failure must never dead-letter"); + } + + @Test + void aDelegateThatRecoversWithinItsCycleCompletesWithNoConversion() { + deploy("flaky-expression", "flaky", "R3/PT1S"); + ProcessInstance instance = engine.getRuntimeService() + .startProcessInstanceByKey("flaky-expression"); + + assertEquals("OK-3", historicVariable(instance, "result"), + "the declared cycle must re-run the delegate until it succeeds on its last attempt"); + assertNull(historicVariable(instance, "recorded"), "a recovered step must not have routed through the error boundary"); + } + + /** + * The must-not-break case of wrapping {@code handleException}: a {@link BpmnError} the delegate + * raises itself is the superclass's business, and the wrapper sees nothing to convert - so it + * reaches ITS OWN boundary on the first attempt rather than being retried or re-coded as the intent + * error. + */ + @Test + void aDelegateRaisingItsOwnBpmnErrorStillReachesItsOwnBoundary() { + deploy("refusing-expression", "refusing", "R3/PT1S"); + ProcessInstance instance = engine.getRuntimeService() + .startProcessInstanceByKey("refusing-expression"); + + assertEquals("yes", historicVariable(instance, "refused"), "the delegate's own BPMN error must take its own route"); + assertNull(historicVariable(instance, "recorded"), "it must not be converted into the intent error"); + } + + private static void deploy(String processId, String delegateBean, String retryCycle) { + String xml = PROCESS_XML_TEMPLATE.formatted(processId, delegateBean, retryCycle); + engine.getRepositoryService() + .createDeployment() + .addString(processId + ".bpmn20.xml", xml) + .deploy(); + } + + /** Wait for the instance to end and return the historic value of the named variable. */ + private static String historicVariable(ProcessInstance instance, String name) { + HistoryService history = engine.getHistoryService(); + await().atMost(Duration.ofSeconds(60)) + .pollInterval(Duration.ofMillis(250)) + .until(() -> history.createHistoricProcessInstanceQuery() + .processInstanceId(instance.getId()) + .finished() + .count() == 1); + HistoricVariableInstance variable = history.createHistoricVariableInstanceQuery() + .processInstanceId(instance.getId()) + .variableName(name) + .singleResult(); + return variable == null ? null : String.valueOf(variable.getValue()); + } +} diff --git a/components/engine/engine-intent/CLAUDE.md b/components/engine/engine-intent/CLAUDE.md index 2821bf387dd..ea014e1275e 100644 --- a/components/engine/engine-intent/CLAUDE.md +++ b/components/engine/engine-intent/CLAUDE.md @@ -413,7 +413,7 @@ Semantics worth knowing: - **`abortOn:` on a process = cancel the in-flight instance when the document transitions into a terminal status (BPM events wave 2).** `abortOn: { status: [4, 5], then: markVoid }` — a `-transitioned` of the trigger entity into any listed EntityStatus seed id cancels the whole running instance (pending user tasks, parked waits, armed boundary timers). Emitted as an **interrupting message event subprocess** (`` with an `isInterrupting="true"` message start on `Abort` → optional cleanup serviceTask → `terminateEventDefinition`), NOT by wrapping the main flow — chosen over the proposal's subProcess-wrap sketch because it needs no restructuring of the flat step layout and still kills everything in scope. Glue: `ProcessAbortSupport` → the `aborts` collection in `.glue` → `Abort.java.template` — a `MessageHandler` on the entity's `-transitioned` topic (the channel transitions/setters already publish) that matches the status list (`entity. == || …`) and correlates `Abort` on the instance THIS process stamped in `ProcessIds` (`ProcessStamps.idFor`, falling back to `ProcessId` for records stamped before that column existed), fail-soft. `then:` omitted or `end` = terminate; a declared `serviceTask` cleanup (setField/setRelationField) is **abort-only** — `BpmnIntentGenerator` filters it out of the main linear chain (`steps.removeIf`) and re-emits it inside the event subprocess (its setter glue is still generated by `SetFieldSupport`). Parser (`validateAbortOn`): integer `status` (scalar or list), trigger entity with a `function: EntityStatus` relation, `then` = `end`/a setField-setRelationField serviceTask that is NOT explicitly routed to from the main flow (`next`/`then`/`else`). DI: the event subprocess is a fixed-placement container box below the main lane (BPMN-2.0 expanded-subprocess children carry absolute plane coordinates). Requested/shaped in upstream discussion #6340. **Consumers:** the orphaned-Inbox-task hole (cancel a SalesOrder mid-confirm), and the structural replacement for a cancelling `expire:` guard (kf quotations drops its `custom/` guard delegate). Caveat: `then` cleanup is one serviceTask (a multi-step cleanup chain is future work). - **`whenDeleted: abort | refuse` on a process = what a DELETE of the trigger row does to the in-flight instance (#7074).** `abortOn` is deaf to deletes - a `-deleted` is not a `-transitioned` - so deleting a DRAFT under approval left its Inbox task alive, claimable, opening a form of `—`s and completable over a row that no longer existed. Now EVERY entity-triggered process gets a `AbortOnDelete` `MessageHandler` on the trigger entity's `-deleted` topic (`deleteAborts` glue collection, `AbortOnDelete.java.template`, the pipeline's collection case) that reads its own `ProcessIds` stamp off the deleted payload and `Process.cancel`s the instance if `Process.isRunning` - generated whatever `whenDeleted` says, because `refuse` guards only the REST surface and a cascade or a reaction still deletes rows. `refuse` lands on the trigger entity as the `processDeleteGuards` scalar (`:

+ * Both keys apply to a {@code delegate:} or a {@code notify:} service task - the two shapes whose + * work is a call that can fail transiently and whose failure nobody is synchronously waiting on. + * The send was added in dirigible #7056: its generated handler already failed the task on a + * delivery error, so before the runtime conversion reached the {@code delegateExpression} path it + * was the one step in a process whose failure had nowhere to go but a dead-letter incident. + * + *

+ * The other service-task shapes are refused, each for its own reason. A {@code setField} / + * {@code setRelationField} step writes through the model's own gates, and a check-gated status + * write is deliberately emitted without the async boundary (#7014 / #7063) so its refusal + * reaches the person who acted - converting it into a routed BPMN error would take that 400 away + * from the Inbox, and re-attempting a deterministic refusal recovers nothing. A {@code call:} step + * and the bare {@code custom.} fallback are simply not covered yet; a hand-written handler + * that wants resilience is bound with {@code delegate:}. And a fan-out send + * ({@code notify.forEach}) is per-row fail-soft by construction - it never fails the task - so a + * declared cycle could not fire and a boundary could not be reached; its outcome is observed with + * {@code outcome:} and an {@code event: { onNotifyFailed: }} consumer instead (#7023). */ private static void validateStepResilience(ProcessIntent process, List issues) { Set stepNames = new HashSet<>(); @@ -6410,6 +6447,10 @@ private static void validateStepResilience(ProcessIntent process, List i String subject = "process [" + process.getName() + "] step [" + step.getName() + "]"; String delegate = stepArg(step, "delegate"); boolean hasDelegate = delegate != null && !delegate.isBlank(); + NotificationIntent notify = NotifySupport.stepNotify(step); + boolean hasFanOutNotify = notify != null && notify.getForEach() != null && !notify.getForEach() + .isBlank(); + boolean resilienceApplies = hasDelegate || notify != null; Object retryRaw = step.getArgs() .get("retry"); // A misplaced retry/onError (a non-serviceTask kind) is already reported by the by-kind @@ -6418,9 +6459,10 @@ private static void validateStepResilience(ProcessIntent process, List i if (!(retryRaw instanceof Map retry)) { issues.add(subject + " retry must be a map (e.g. `retry: { count: 3, every: PT30S }`)"); } else { - if (!hasDelegate) { - issues.add(subject + " declares retry but no delegate - step resilience applies to delegate service tasks" - + " only (v1)"); + if (!resilienceApplies) { + issues.add(subject + STEP_RESILIENCE_SHAPE_ISSUE.formatted("retry")); + } else if (hasFanOutNotify) { + issues.add(subject + STEP_RESILIENCE_FAN_OUT_ISSUE.formatted("retry")); } Object count = retry.get("count"); if (count == null) { @@ -6443,8 +6485,10 @@ private static void validateStepResilience(ProcessIntent process, List i } String onError = ProcessResilienceSupport.onError(step); if (onError != null && "serviceTask".equals(step.getKind())) { - if (!hasDelegate) { - issues.add(subject + " declares onError but no delegate - step resilience applies to delegate service tasks only (v1)"); + if (!resilienceApplies) { + issues.add(subject + STEP_RESILIENCE_SHAPE_ISSUE.formatted("onError")); + } else if (hasFanOutNotify) { + issues.add(subject + STEP_RESILIENCE_FAN_OUT_ISSUE.formatted("onError")); } if (!isRoutingLiteral(onError) && !stepNames.contains(onError)) { issues.add(subject + " `onError` references unknown step [" + onError + "]"); diff --git a/components/engine/engine-intent/src/main/resources/intent-assistant-guide.md b/components/engine/engine-intent/src/main/resources/intent-assistant-guide.md index 2857a89452b..ba02b0ba436 100644 --- a/components/engine/engine-intent/src/main/resources/intent-assistant-guide.md +++ b/components/engine/engine-intent/src/main/resources/intent-assistant-guide.md @@ -1427,10 +1427,11 @@ entity-agnostic helpers (e.g. a number generator over its own repository) belong and are called from the delegate (client Java compiles across all published projects). `delegate` cannot be combined with `setField` / `setRelationField` / `call`; `fields` values must be scalars. -**Step resilience on a delegate: `retry:`, `onError:`, `{error}` and declared step data.** A -delegate that talks to something remote - provision a schema, register a client in an identity -provider, call a partner API - fails sometimes, and what happens then should be modeled, not left to -the runtime's defaults. Both attributes apply to `delegate:` service tasks only: +**Step resilience on a `delegate:` or a `notify:` step: `retry:`, `onError:`, `{error}` and declared +step data.** A step that talks to something remote - provision a schema, register a client in an +identity provider, call a partner API, send a mail - fails sometimes, and what happens then should be +modeled, not left to the runtime's defaults. Both attributes apply to the two service-task shapes +whose work is such a call: `delegate:` and `notify:`. ```yaml processes: @@ -1445,6 +1446,15 @@ processes: - name: provisionApp kind: serviceTask args: { delegate: custom.AppProvisioner, uses: [dbPassword], retry: { count: 5, every: PT1M }, onError: recordFailure, next: done } + # a SEND may declare the same two keys: its whole work is the message, so a delivery failure + # fails the task - and SMTP blinks exactly as any of the calls above does. + - name: notifyOwner + kind: serviceTask + args: + notify: { to: owner.email, subject: "Tenant {title} is ready", body: "..." } + retry: { count: 3, every: PT30S } + onError: recordFailure + next: done - { name: recordFailure, kind: serviceTask, args: { setField: failureMessage, value: "{error}", next: markFailed } } - { name: markFailed, kind: serviceTask, args: { setRelationField: Status, value: Failed, next: end } } - { name: done, kind: end } @@ -1464,6 +1474,23 @@ processes: error. `clearAfter: ` removes the value once that serviceTask/userTask completes normally, so a generated credential does not survive in the process history. +**Where the two keys are refused, and what to author instead.** Every refusal is a parse error, so +you never ship a declaration that silently never fires: + +- **A `setField` / `setRelationField` step.** A status write is refused by the model's own gates + (`checks:`, `lifecycle:`), and a gated one runs inside the transaction of the user action that + reached it precisely so its refusal reaches the person who acted. Routing that failure away would + take the message out of their hands, and re-attempting a deterministic refusal recovers nothing. +- **A `call:` step or a bare service task** (no `delegate:`, no `notify:`). Not covered; bind the + handler with `delegate:` if it needs resilience. +- **A fan-out send** (`notify:` carrying `forEach:`). A fan-out is fail-soft **per row** by + construction - one unreachable mailbox must not abort the rows after it, and re-attempting the + whole step would mail every recipient who already received the message a second time - so the step + never fails and neither key could fire. Observe the deliveries instead: `outcome: ` + stamps `sent` / `failed: ` per row, and `event: { onNotifyFailed: }` is the axis a + reaction binds to. +- A non-`serviceTask` kind at all: `retry:`/`onError:` are serviceTask arguments. + **Waiting for a data event: `wait`.** A `wait` step **parks the process** until an entity lifecycle event resumes it - a support case waiting for the requester's reply, a dunning flow waiting for a payment, an order flow waiting for its goods receipt. Never model this as a user task looping back to @@ -2606,7 +2633,14 @@ language, read off the record, since there is only one render for the whole fan- fails. A row with no address is skipped, a failed send is logged, and the step completes with a summary count. That is deliberate: failing the task would have the engine retry the WHOLE fan-out and mail everyone who already received their message a second time, and a partial send cannot be made -idempotent. +idempotent. It is also why a fan-out send may declare neither `retry:` nor `onError:` (refused at +parse): the step never fails, so neither could ever fire - use `outcome:` and `onNotifyFailed` there. + +**A non-fan-out send on a `serviceTask` DOES fail the task**, deliberately - its whole work is the +message - and that is what makes it one of the two shapes step resilience applies to: give it +`retry: { count, every }` so a transient SMTP failure recovers by itself, and `onError: ` so an +exhausted one lands on the record instead of a dead-letter incident. See the step-resilience section. +Without either, the failure takes the engine's default path and the flow stops at the send. Where the block can sit - the three places an intent acts, plus the standalone `notifications` entry: @@ -3655,8 +3689,8 @@ or a seeded name. | step `kind` | `userTask`, `serviceTask`, `decision`, `script`, `wait`, `end` | | wait event | `onCreate`, `onUpdate`, `onTransition` (never `onDelete`) | | userTask timers | `timeout: { after: , then: }`, `expire: { until: , then: }` | -| serviceTask `retry` | `{ count: = 1>, every: }` - `delegate:` steps only | -| serviceTask `onError` | a declared step or `end` - `delegate:` steps only; `{error}` (a whole-value `setField` value) is readable on the route | +| serviceTask `retry` | `{ count: = 1>, every: }` - `delegate:` and non-fan-out `notify:` steps only | +| serviceTask `onError` | a declared step or `end` - `delegate:` and non-fan-out `notify:` steps only; `{error}` (a whole-value `setField` value) is readable on the route | | process `vars` | `[{ name: , clearAfter: }]`; step `produces:`/`uses:` list declared var names | | process `abortOn` | `{ status: \| [ids], then: \| end }` (trigger entity needs a `function: EntityStatus` relation) | | relation `whenMasterDeleted` | `cascade` (default - a delete of the master deletes the children it owns), `refuse` (the master's delete is rejected while children exist); composition relations only | @@ -3698,6 +3732,7 @@ or a seeded name. - "deleting a document under approval must kill the approval / must be refused while it runs" -> **processes** (`whenDeleted: abort | refuse`; the cancelling `-deleted` listener is generated regardless) - "deleting a header must delete its lines / must be refused while it has lines" -> the child's **composition relation** (`whenMasterDeleted: cascade | refuse`; cascade is the default, so nothing is ever orphaned) - "retry the flaky external call, and record the failure on the record instead of an incident" -> **processes** (`delegate:` serviceTask with `retry:` + `onError:`, the failure message via `{error}`) +- "if the mail cannot go out, retry it and then record why - don't leave the process stuck" -> **processes** (the `notify:` serviceTask takes the same `retry:` + `onError:`; a fan-out send instead uses `outcome:` + `onNotifyFailed`) - "a screen to enter / edit X" -> **forms** - "a button on X's view that opens a custom page / action" -> **actions** - "void / cancel / close / reopen a finished document (a guarded manual status change, per record)" -> **transitions** diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/ResilienceBpmnTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/ResilienceBpmnTest.java index d593a3bfdb2..47ed44ea140 100644 --- a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/ResilienceBpmnTest.java +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/ResilienceBpmnTest.java @@ -29,12 +29,15 @@ import org.mockito.ArgumentCaptor; /** - * Declarative step resilience in the emitted BPMN - dirigible #6762: {@code retry:} becomes a - * Flowable failed-job retry cycle on the delegate service task, {@code onError:} an error boundary + * Declarative step resilience in the emitted BPMN - dirigible #6762 and #7056: {@code retry:} + * becomes a Flowable failed-job retry cycle on the service task, {@code onError:} an error boundary * event routed like a decision branch (catching the {@code INTENT_STEP_FAILED} error the runtime * conversion raises for the final failed attempt), and a var's {@code clearAfter} an - * {@code event="end"} execution listener removing the value once its step completes. An intent - * without the keys must emit none of it. + * {@code event="end"} execution listener removing the value once its step completes. Both emission + * paths are covered: the {@code delegate:} task's {@code flowable:class} element and the + * {@code notify:} task's {@code ${JavaTask}} delegate-expression element, whose cycle has to ride + * the same extensionElements block its {@code handler} field does. An intent without the keys must + * emit none of it. */ class ResilienceBpmnTest { @@ -66,6 +69,17 @@ class ResilienceBpmnTest { - { name: done, kind: end } """; + /** + * The #7056 fixture: the same process with a non-fan-out {@code notify:} step declaring the keys. + * Its work is the message, so a delivery failure fails the task - which is why the send is one of + * the two shapes step resilience applies to. + */ + private static final String SEND_YAML = YAML.replace(" - { name: recordFailure,", + " - { name: notifyOwner, kind: serviceTask, args: { notify: { to: owner@example.com, subject: \"Tenant provisioned\"," + + " body: \"Ready.\" }, retry: { count: 1, every: PT5S }, onError: recordFailure, next: done } }\n" + + " - { name: recordFailure,") + .replace("onError: recordFailure, next: done", "onError: recordFailure, next: notifyOwner"); + private static String bpmn(String yaml) { IntentModel model = IntentParser.parse(yaml); IRepository repository = mock(IRepository.class); @@ -199,8 +213,62 @@ void onErrorEndRoutesTheFailureToTheEndEvent() { assertFlow(bpmn, "createSchemaError", "end"); } + /** + * A send step is emitted on the {@code ${JavaTask}} delegate-expression path, so its cycle has to + * be written into the extensionElements block the {@code handler} field already opens - next to it, + * and inside the send's own element rather than the next task's. + */ + @Test + void aSendStepCarriesItsRetryCycleNextToItsHandlerField() { + String bpmn = bpmn(SEND_YAML); + + int task = bpmn.indexOf("= 0, "the send step must be emitted in:\n" + bpmn); + assertTrue(bpmn.contains("R2/PT5S"), + "count: 1 must emit an R2 cycle on the send in:\n" + bpmn); + assertTrue(task < handler && handler < cycle && cycle < nextTask, + "the cycle must ride the send's own element, after its handler field, in:\n" + bpmn); + assertTrue(bpmn.contains(""), + "the send needs its own cancelling boundary in:\n" + bpmn); + assertFlow(bpmn, "notifyOwnerError", "recordFailure"); + assertTrue(bpmn.contains("BPMNShape_notifyOwnerError"), "the boundary needs a shape or the modeler opens broken:\n" + bpmn); + assertTrue(bpmn.contains("BPMNEdge_flow_notifyOwnerError_then"), "the error route needs its edge:\n" + bpmn); + } + + /** A send that declares nothing emits no cycle - the send path stays byte-identical too. */ + @Test + void aSendWithoutResilienceKeysEmitsNoCycle() { + String bpmn = bpmn(SEND_YAML.replace("retry: { count: 1, every: PT5S }, onError: notifyOwner, ", "") + .replace("retry: { count: 1, every: PT5S }, onError: recordFailure, ", "")); + + int task = bpmn.indexOf("= 0 && nextTask > task, "both steps must still be emitted in:\n" + bpmn); + assertFalse(bpmn.substring(task, nextTask) + .contains("failedJobRetryTimeCycle"), + "no cycle on a send that declares none in:\n" + bpmn); + assertFalse(bpmn.contains("} on a delegate service task, and the {@code {error}} placeholder a - * {@code setField} on the error route reads. The parser must reject a malformed retry cycle, a - * dangling error route, resilience without a delegate (v1 - the runtime conversion lives on the - * {@code flowable:class} path), and an {@code {error}} nothing would ever populate. + * Declarative step resilience - dirigible #6762 and #7056: {@code retry: { count, every }} and + * {@code onError: } on a {@code delegate:} or a {@code notify:} service task, and the + * {@code {error}} placeholder a {@code setField} on the error route reads. The parser must reject a + * malformed retry cycle, a dangling error route, resilience on a service-task shape it does not + * apply to, resilience on a fan-out send (which never fails, so neither key could fire), and an + * {@code {error}} nothing would ever populate. */ class StepResilienceIntentTest { @@ -110,12 +111,82 @@ void onErrorMayRouteToEnd() { .replace("onError: recordFailure,", "onError: end,"))); } + /** + * A bare service task (its work a hand-written {@code custom.} handler) is not one of the two + * shapes the keys apply to; the message names both, since that is what the author's next move is. + */ @Test - void retryWithoutADelegateIsRejected() { + void resilienceOnNeitherADelegateNorANotifyIsRejected() { String yaml = YAML.replace("delegate: custom.SchemaProvisioner, produces: [dbPassword], ", ""); - String issue = assertIssue(yaml, "declares retry but no delegate"); - assertTrue(issue.contains("delegate service tasks only"), "the message must state the v1 rule: " + issue); - assertIssue(yaml, "declares onError but no delegate"); + String issue = assertIssue(yaml, "declares retry but is neither a `delegate:` nor a `notify:` service task"); + assertTrue(issue.contains("applies to those two shapes"), "the message must name the two supported shapes: " + issue); + assertIssue(yaml, "declares onError but is neither a `delegate:` nor a `notify:` service task"); + } + + /** + * The #7056 widening: a send's generated handler fails the task on a delivery error, so the send + * may now say what happens next instead of leaving that failure to a dead-letter incident. + */ + @Test + void aSendMayDeclareItsOwnResilience() { + assertDoesNotThrow(() -> IntentParser.parse(withSendStep("retry: { count: 1, every: PT5S }, onError: recordFailure, "))); + } + + /** + * A setter step is refused, and the message says why rather than only that it is: a check-gated + * status write is emitted without the async boundary on purpose (#7014 / #7063) so its refusal + * reaches the person who acted - routing it away would take that 400 out of the Inbox. + */ + @Test + void resilienceOnASetterStepIsRejected() { + String issue = assertIssue( + YAML.replace("setField: failureMessage, value: \"{error}\", next: markFailed", + "setField: failureMessage, value: failed, next: markFailed, retry: { count: 1, every: PT5S }"), + "declares retry but is neither a `delegate:` nor a `notify:` service task"); + assertTrue(issue.contains("refused synchronously to the person who acted"), + "the message must state why a status write is not routed away: " + issue); + } + + /** + * A fan-out send is per-row fail-soft by construction - it never fails the task - so a declared + * cycle could not fire and a boundary could not be reached. Refused rather than authored and + * silently inert, with the observable alternative named. + */ + @Test + void resilienceOnAFanOutSendIsRejected() { + String issue = assertIssue(fanOutSend("retry: { count: 1, every: PT5S }, "), "declares retry on a fan-out notify (forEach)"); + assertTrue(issue.contains("`outcome:`") && issue.contains("onNotifyFailed"), + "the message must name the observable alternative: " + issue); + assertIssue(fanOutSend("onError: recordFailure, "), "declares onError on a fan-out notify (forEach)"); + } + + /** The fan-out send itself stays authorable - only its resilience keys are refused. */ + @Test + void aFanOutSendWithoutResilienceKeysParses() { + assertDoesNotThrow(() -> IntentParser.parse(fanOutSend(""))); + } + + /** The showcase plus a non-fan-out send step carrying the given resilience keys. */ + private static String withSendStep(String resilienceKeys) { + return YAML.replace(" - { name: recordFailure,", + " - { name: notifyOwner, kind: serviceTask, args: { notify: { to: owner@example.com, subject: \"Tenant provisioned\"," + + " body: \"Ready.\" }, " + resilienceKeys + "next: done } }\n - { name: recordFailure,"); + } + + /** The showcase plus a contacts child and a send that fans out over it, carrying the given keys. */ + private static String fanOutSend(String resilienceKeys) { + return YAML.replace("processes:", """ + - name: TenantContact + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: email, type: string } + relations: + - { name: tenant, kind: manyToOne, to: TenantApplication } + processes:""") + .replace(" - { name: recordFailure,", + " - { name: notifyContacts, kind: serviceTask, args: { notify: { forEach: TenantContact, to: email," + + " subject: \"Provisioned\", body: \"Ready.\" }, " + resilienceKeys + "next: done } }\n" + + " - { name: recordFailure,"); } /** A misplaced retry keeps the by-kind vocabulary message and gets no second, blunter line. */ diff --git a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Send.java.template b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Send.java.template index 9cb2a12f95c..4096fe2fcff 100644 --- a/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Send.java.template +++ b/components/template/template-application-events-java/src/main/resources/META-INF/dirigible/template-application-events-java/events/Send.java.template @@ -75,9 +75,14 @@ import gen.${attachLanguageJavaGenFolder}.data.${attachLanguageJavaTargetPerspec * A delivery failure is logged per row and the step still completes - deliberately. Failing the task * would have the engine retry the WHOLE fan-out and mail every recipient that already received their * message a second time; a partial send is not idempotent, so the summary in the log is the report. + * That is also why a fan-out send may declare neither `retry:` nor `onError:` - the step never fails, + * so neither could ever fire; its outcome is observed with `outcome:` and an `onNotifyFailed` consumer. #else - * A real delivery failure DOES fail the task, so the process engine's retry applies - unlike a - * transition's notify, a send step's whole purpose is the message. + * A real delivery failure DOES fail the task - unlike a transition's notify, a send step's whole + * purpose is the message. So the step's declared resilience applies: a `retry:` cycle re-attempts the + * send with the declared spacing, and an `onError:` route takes the exhausted failure (with this + * message readable as `{error}`) instead of a dead-letter incident. Declaring neither leaves the + * failure to the engine default, which ends as an incident on the job rather than on the record. #end */ public class ${className} implements JavaDelegate { @@ -288,7 +293,9 @@ public class ${className} implements JavaDelegate { // a retry that finally succeeds overwrites it with `sent`. stampNotifyOutcome(entity.${notifyOutcomeKeyProperty}, ex); #end - throw new RuntimeException("Failed to send the ${step} mail of the ${process} process", ex); + // The message is what an `onError:` route records through {error}, so it names the cause: a + // fixed sentence would put "the mail failed" on the record and nothing an operator can act on. + throw new RuntimeException("Failed to send the ${step} mail of the ${process} process: " + ex.getMessage(), ex); } #end } diff --git a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java index b7e6f121bae..ed7df13594a 100644 --- a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java +++ b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEmissionCoverageIT.java @@ -687,6 +687,7 @@ class IntentEmissionCoverageIT extends IntegrationTest { - { name: title, type: string, length: 200 } - { name: generatedKey, type: string, length: 100 } - { name: failureMessage, type: string, length: 500 } + - { name: sendFailure, type: string, length: 500 } # The non-HTTP inbound arrivals (#6537) ingest into an entity of their own: an ingested # record must not start a process, or the queue/file scenarios would seed extra Inbox @@ -1101,7 +1102,21 @@ class IntentEmissionCoverageIT extends IntegrationTest { next: storeKey - name: storeKey kind: serviceTask - args: { delegate: custom.ProvisionKeyWriter, uses: [apiKey], next: hold } + args: { delegate: custom.ProvisionKeyWriter, uses: [apiKey], next: notifyOwner } + # a SEND declaring its own resilience (#7056): this instance has no SMTP, so the + # delivery cannot succeed - which is the case being asserted. The exhausted retry + # routes the FINAL attempt's message onto the record and the flow CARRIES ON to the + # hold, instead of the send dead-lettering and stopping the process where it stands. + - name: notifyOwner + kind: serviceTask + args: + notify: { to: owner@example.com, subject: "Provisioned {title}", body: "Ready." } + retry: { count: 1, every: PT1S } + onError: recordSendFailure + next: hold + - name: recordSendFailure + kind: serviceTask + args: { setField: sendFailure, value: "{error}", next: hold } - name: hold kind: userTask args: { assignee: operator, next: doomedCall } @@ -3245,6 +3260,24 @@ private void assertEmission() { String failureSetter = contentOf("gen/events/emission/ProvisionFlowRecordFailure.java"); assertTrue(failureSetter.contains("execution.getVariable(\"__errorMessage\")"), "the {error} setter must read the failure message the conversion published"); + + // #7056: the send is the second shape step resilience applies to, and it is emitted on the + // ${JavaTask} delegate-expression path - so its cycle has to share the extensionElements block + // with the handler field, and the boundary machinery has to treat it like any other step. + int send = provisionBpmn.indexOf("= 0 && send < sendHandler && sendHandler < sendCycle && sendCycle < provisionBpmn.indexOf("") + && provisionBpmn.contains("sourceRef=\"notifyOwnerError\" targetRef=\"recordSendFailure\""), + "the send must carry its own cancelling boundary, routed like a delegate's"); + String sendCode = contentOf("gen/events/emission/ProvisionFlowNotifyOwnerSend.java"); + assertTrue(sendCode.contains("process: \" + ex.getMessage()"), + "the send's failure must name its cause - that message is what the error route records via {error}"); } /** @@ -5310,6 +5343,13 @@ private void assertBpmEventsRuntime() { * single retry, the runtime conversion turns the SECOND attempt's failure into the caught BPMN * error, and the {@code onError} route records that exact message on the record via {@code {error}} * - instead of the dead-letter incident it would be without the declaration. + * + *

+ * The send between the writer and the hold is the #7056 half, and it is the case the issue is + * about: this instance has no SMTP, so the delivery cannot succeed, and without the declaration the + * step would dead-letter and the process would stop there with nothing on the record to say so. + * With it, the exhausted retry routes the failure onto the record and the flow carries on to the + * hold - which the hold assertions below then prove it reached. */ private void assertStepResilienceRuntime() { String provisionApi = API + "/provision/ProvisionController"; @@ -5332,6 +5372,16 @@ private void assertStepResilienceRuntime() { .body("GeneratedKey", equalTo("KEY-3")), 180); + // #7056: the send's exhausted retry converted instead of dead-lettering, so its error route + // recorded the FINAL attempt's message - which names the cause, not just "the mail failed". + restAssuredExecutor.execute(() -> given().when() + .get(provisionApi + "/" + provision.get()) + .then() + .statusCode(200) + .body("SendFailure", containsString( + "Failed to send the notifyOwner mail of the" + " ProvisionFlow process: ")), + 180); + // clearAfter: the instance parks at the hold task with the secret already removed from its // live variables - it must not survive in the process data (or its history). AtomicReference processId = new AtomicReference<>(); diff --git a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEngineIT.java b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEngineIT.java index 1a8264e60ac..7da0eb16e5f 100644 --- a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEngineIT.java +++ b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/api/IntentEngineIT.java @@ -1667,7 +1667,8 @@ void step_resilience_emits_retry_cycle_error_boundary_clear_listener_and_error_g - { name: dbPassword, clearAfter: provisionApp } steps: - { name: createSchema, kind: serviceTask, args: { delegate: custom.SchemaProvisioner, produces: [dbPassword], retry: { count: 3, every: PT30S }, onError: recordFailure } } - - { name: provisionApp, kind: serviceTask, args: { delegate: custom.AppProvisioner, uses: [dbPassword], retry: { count: 5, every: PT1M }, onError: recordFailure, next: done } } + - { name: provisionApp, kind: serviceTask, args: { delegate: custom.AppProvisioner, uses: [dbPassword], retry: { count: 5, every: PT1M }, onError: recordFailure, next: notifyOwner } } + - { name: notifyOwner, kind: serviceTask, args: { notify: { to: owner@example.com, subject: "Tenant provisioned", body: "Ready." }, retry: { count: 1, every: PT5S }, onError: recordFailure, next: done } } - { name: recordFailure, kind: serviceTask, args: { setField: failureMessage, value: "{error}", next: markFailed } } - { name: markFailed, kind: serviceTask, args: { setRelationField: Status, value: 3, next: end } } - { name: done, kind: end } @@ -1703,6 +1704,24 @@ void step_resilience_emits_retry_cycle_error_boundary_clear_listener_and_error_g assertTrue(bpmn.contains("BPMNShape_createSchemaError") && bpmn.contains("BPMNEdge_flow_createSchemaError_then"), "the error boundary needs its DI shape and edge or the modeler opens it detached"); + // #7056: a `notify:` step is the second shape step resilience applies to. Its element is the + // ${JavaTask} delegate-expression one, so the cycle has to share the extensionElements block + // with the handler field the dispatcher reads - and the boundary machinery is shape-agnostic. + int send = bpmn.indexOf("= 0 && send < sendHandler && sendHandler < sendCycle && sendCycle < afterSend, + "the send's retry cycle should ride its own element, after its handler field"); + assertTrue(bpmn.contains("R2/PT5S"), + "the send's retry count: 1 should emit an R2 failed-job retry cycle"); + assertTrue(bpmn.contains("") + && bpmn.contains("sourceRef=\"notifyOwnerError\" targetRef=\"recordFailure\""), + "the send should carry its own cancelling error boundary, routed like a delegate's"); + // clearAfter: an end-listener on the completing step removes the credential from the // instance data (and thereby from the history). assertTrue(bpmn.contains( @@ -1781,6 +1800,12 @@ void parse_rejects_malformed_step_resilience_and_undeclared_vars() { fields: - { name: id, type: integer, primaryKey: true, generated: true } - { name: failureMessage, type: string } + - name: TenantContact + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: email, type: string } + relations: + - { name: tenant, kind: manyToOne, to: TenantApplication } processes: - name: TenantProvisioning trigger: { onCreate: TenantApplication } @@ -1788,7 +1813,8 @@ void parse_rejects_malformed_step_resilience_and_undeclared_vars() { - { name: dbPassword, clearAfter: nowhere } steps: - { name: createSchema, kind: serviceTask, args: { delegate: custom.SchemaProvisioner, produces: [dbPasword], retry: { cout: 3, every: 30seconds }, onError: recordFailur } } - - { name: recordFailure, kind: serviceTask, args: { setField: failureMessage, value: "{error}", next: end } } + - { name: recordFailure, kind: serviceTask, args: { setField: failureMessage, value: "{error}", next: end, retry: { count: 1, every: PT5S } } } + - { name: hold, kind: serviceTask, args: { notify: { forEach: TenantContact, to: email, subject: "Held", body: "." }, onError: end, next: end } } """; restAssuredExecutor.execute(() -> given().contentType("text/plain") .body(yaml) @@ -1802,7 +1828,18 @@ void parse_rejects_malformed_step_resilience_and_undeclared_vars() { "process [TenantProvisioning] step [createSchema] `onError` references unknown step [recordFailur]", "process [TenantProvisioning] step [createSchema] produces names undeclared var [dbPasword] - declare it under the process `vars:`", "process [TenantProvisioning] var [dbPassword] clearAfter references unknown step [nowhere]", - "process [TenantProvisioning] step [recordFailure] setField value {error} is only resolvable on a step reachable from an onError route"))); + "process [TenantProvisioning] step [recordFailure] setField value {error} is only resolvable on a step reachable from an onError route", + // #7056: the keys apply to a delegate: or a notify: step. A setter is refused + // because a + // check-gated status write is refused synchronously to the person who acted, and a + // fan-out send because it never fails the task, so neither key could ever fire + // there. + "process [TenantProvisioning] step [recordFailure] declares retry but is neither a `delegate:` nor a `notify:` service task" + + " - step resilience applies to those two shapes (a check-gated status write is refused synchronously to the" + + " person who acted, so its failure must not be routed away)", + "process [TenantProvisioning] step [hold] declares onError on a fan-out notify (forEach) - a fan-out sends per row and is" + + " fail-soft per row, so the step never fails and nothing would retry or route; observe the delivery with" + + " `outcome:` and an `event: { onNotifyFailed: }` consumer"))); } @Test diff --git a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/ui/tests/SampleIntentResilienceTestProject.java b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/ui/tests/SampleIntentResilienceTestProject.java index 0445806addf..aa916b454c9 100644 --- a/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/ui/tests/SampleIntentResilienceTestProject.java +++ b/tests/tests-integrations/src/main/java/org/eclipse/dirigible/integration/tests/ui/tests/SampleIntentResilienceTestProject.java @@ -31,12 +31,12 @@ import org.springframework.stereotype.Component; /** - * The {@code sample-intent-resilience} fixture project (dirigible #6762), configured exactly as a - * developer would in the browser IDE - copy into the workspace, open {@code app.intent} in the - * Intent Editor and click its Generate, then Publish all from the Workbench - and verified over - * REST. The fixture doubles as the manual-testing sample (see its README), so this project is what - * keeps the sample from silently rotting - it asserts both outcomes the sample was built to - * demonstrate: + * The {@code sample-intent-resilience} fixture project (dirigible #6762 and #7056), configured + * exactly as a developer would in the browser IDE - copy into the workspace, open + * {@code app.intent} in the Intent Editor and click its Generate, then Publish all from the + * Workbench - and verified over REST. The fixture doubles as the manual-testing sample (see its + * README), so this project is what keeps the sample from silently rotting - it asserts every + * outcome the sample was built to demonstrate, one tenant title per outcome: * *

    *
  • a tenant titled normally recovers by RETRY: the schema delegate fails its first two attempts, @@ -45,7 +45,10 @@ * removes the credential so it does not survive in the process history; *
  • a tenant titled with "fail" EXHAUSTS its retry: the app delegate refuses every attempt, its * {@code retry: { count: 2 }} allows exactly three, and the {@code onError} route records the FINAL - * attempt's message on the record via {@code {error}} - instead of a dead-letter incident. + * attempt's message on the record via {@code {error}} - instead of a dead-letter incident; + *
  • a tenant titled "notify" reaches the SEND step, which has no SMTP to reach: its exhausted + * retry routes the delivery failure onto the record the same way, which is the whole point of #7056 + * - the send is mid-process, not last, and a dead-lettered send no longer strands the flow. *
*/ @Lazy @@ -70,9 +73,10 @@ class SampleIntentResilienceTestProject extends BaseIntentTestProject { @Override public void verify() { - // Both tenants up front, so their retry cycles run concurrently. + // Every tenant up front, so their retry cycles run concurrently. AtomicInteger recovering = new AtomicInteger(); AtomicInteger doomed = new AtomicInteger(); + AtomicInteger sending = new AtomicInteger(); restAssuredExecutor.execute(() -> recovering.set(given().contentType("application/json") .body("{\"Title\":\"acme\"}") .when() @@ -89,6 +93,14 @@ public void verify() { .statusCode(200) .extract() .path("Id"))); + restAssuredExecutor.execute(() -> sending.set(given().contentType("application/json") + .body("{\"Title\":\"notify\"}") + .when() + .post(TENANT_API) + .then() + .statusCode(200) + .extract() + .path("Id"))); // Happy path: the schema delegate failed twice on purpose; the declared PT10S cycle re-ran // it to success, and the produced dbPassword reached the app delegate through `uses:` - an @@ -134,5 +146,20 @@ public void verify() { .body("FailureMessage", equalTo("no capacity for 'please fail' (attempt 3)")) .body("Status", equalTo(STATUS_FAILED)), 180); + + // The send path (#7056): this sample has no SMTP, so the delivery cannot succeed. Its + // declared cycle allowed exactly two attempts and the conversion then routed the failure to + // the SAME onError step the delegates use - so the record carries the reason and the status + // moved. Without the declaration this step would dead-letter and the tenant would sit in + // Requested with an empty FailureMessage, the invisible stall the issue is about. + restAssuredExecutor.execute(() -> given().when() + .get(TENANT_API + "/" + sending.get()) + .then() + .statusCode(200) + .body("FailureMessage", + startsWith("Failed to send the notifyOwner mail of the TenantProvisioning" + + " process: ")) + .body("Status", equalTo(STATUS_FAILED)), + 180); } } diff --git a/tests/tests-integrations/src/main/resources/sample-intent-resilience/README.md b/tests/tests-integrations/src/main/resources/sample-intent-resilience/README.md index 13de1fe2d8e..799dbe88e6f 100644 --- a/tests/tests-integrations/src/main/resources/sample-intent-resilience/README.md +++ b/tests/tests-integrations/src/main/resources/sample-intent-resilience/README.md @@ -1,16 +1,17 @@ # sample-intent-resilience A minimal intent project exercising **declarative step resilience** -([eclipse-dirigible/dirigible#6762](https://github.com/eclipse-dirigible/dirigible/issues/6762)): -`retry: { count, every }` and `onError:` on delegate service tasks, the `{error}` placeholder, and +([eclipse-dirigible/dirigible#6762](https://github.com/eclipse-dirigible/dirigible/issues/6762) and +[#7056](https://github.com/eclipse-dirigible/dirigible/issues/7056)): `retry: { count, every }` and +`onError:` on a **`delegate:`** and on a **`notify:`** service task, the `{error}` placeholder, and declared step data (`vars:` + `produces:`/`uses:`/`clearAfter`). This folder is both the **manual-testing project** (import it into a workspace and follow "Run it" below) and the fixture of **`IntentResilienceSampleIT`**, which drives the very same journey through the browser IDE - opens `app.intent` in the Intent Editor, clicks Generate, publishes via the -Workbench - and asserts the same two outcomes automatically, so the sample can never silently rot. +Workbench - and asserts the same outcomes automatically, so the sample can never silently rot. -One process, two hand-written delegates, both outcomes reachable from the UI: +One process, two hand-written delegates and one send, every outcome reachable from the UI: - **`SchemaProvisioner`** fails its first two attempts per tenant and succeeds on the third — the declared `retry: { count: 3, every: PT10S }` recovers it with no incident, producing the @@ -19,14 +20,25 @@ One process, two hand-written delegates, both outcomes reachable from the UI: unless the tenant's **title contains "fail"**, in which case every attempt throws: its `retry: { count: 2 }` exhausts after three attempts and the `onError` route records the FINAL attempt's message into `failureMessage` via `{error}`, then sets the status to Failed. +- **`notifyOwner`** is a **send** step (`notify:`), reached when the tenant's title is exactly + **"notify"**. This sample configures no SMTP, so the delivery always fails - and a send step's + whole work is the message, so that fails the task. Its `retry: { count: 1, every: PT5S }` + re-attempts once and the `onError` route then records the FINAL attempt's message the same way a + delegate's does. That is #7056: before it, `retry:`/`onError:` were refused on a send, so this + step could only dead-letter - the flow stopped at it, the tenant stayed in **Requested**, and + `failureMessage` stayed empty. Note the send is deliberately **not** the last step: making it last + was the available workaround, and it constrained process design for a reason unrelated to the + domain. - `clearAfter: provisionApp` removes `dbPassword` from the instance data once the app step completes, so the credential does not survive in the process history. ## Prerequisite -A Dirigible build that includes the #6762 change -([PR #6783](https://github.com/eclipse-dirigible/dirigible/pull/6783) or any later build) — the -runtime half (converting the exhausted failure into the caught BPMN error) is new. +A Dirigible build that includes both changes — #6762 +([PR #6783](https://github.com/eclipse-dirigible/dirigible/pull/6783)) for the delegate half and +#7056 for the send half. Both add runtime code (the conversion of an exhausted failure into the +caught BPMN error, on the `flowable:class` and the `flowable:delegateExpression` path respectively), +so an older build refuses the intent at Generate or dead-letters the send. ## Run it @@ -49,6 +61,12 @@ runtime half (converting the exhausted failure into the caught BPMN error) is ne curl -s -u admin:admin -H 'Content-Type: application/json' \ -d '{"Title":"please fail"}' \ http://localhost:8080/services/java/sample-intent-resilience/gen/provisioning/api/tenantapplication/TenantApplicationController + + # The send path: provisioning succeeds, then the mail cannot be delivered - one retry, then the + # same onError route. Without #7056 this step would dead-letter and the flow would stop here. + curl -s -u admin:admin -H 'Content-Type: application/json' \ + -d '{"Title":"notify"}' \ + http://localhost:8080/services/java/sample-intent-resilience/gen/provisioning/api/tenantapplication/TenantApplicationController ``` 5. Watch it settle (each retry waits ~10s, plus the async executor's acquire cycle): @@ -63,6 +81,11 @@ runtime half (converting the exhausted failure into the caught BPMN error) is ne - **"please fail"** ends with `FailureMessage: "no capacity for 'please fail' (attempt 3)"` — the FINAL attempt's message, not the first — and status **Failed**. No dead-letter job, no incident. + - **"notify"** ends with `FailureMessage` starting + `"Failed to send the notifyOwner mail of the TenantProvisioning process: "` and status + **Failed** — again with no dead-letter job. Configure a working SMTP + (`DIRIGIBLE_MAIL_SMTP_HOST` and friends) and the same tenant ends **Provisioned** instead, the + send having succeeded on its first attempt. 6. The cleared credential: while an instance is still running (or via the successful record's `ProcessId` right after the app step), list its variables — @@ -81,4 +104,5 @@ runtime half (converting the exhausted failure into the caught BPMN error) is ne - `app.intent` — the whole model; Generate derives `.edm`/`.model`, `TenantProvisioning.bpmn` (retry cycles, error boundary events, the clearing end-listener), seeds and the app code. - `custom/SchemaProvisioner.java`, `custom/AppProvisioner.java` — the hand-written delegates the - intent binds via `delegate:`; `custom/` is developer-owned and survives regeneration. + intent binds via `delegate:`; `custom/` is developer-owned and survives regeneration. The send + needs no hand-written code at all: `notify:` generates its own handler. diff --git a/tests/tests-integrations/src/main/resources/sample-intent-resilience/app.intent b/tests/tests-integrations/src/main/resources/sample-intent-resilience/app.intent index 694dd10fd1c..5ae37504bca 100644 --- a/tests/tests-integrations/src/main/resources/sample-intent-resilience/app.intent +++ b/tests/tests-integrations/src/main/resources/sample-intent-resilience/app.intent @@ -1,5 +1,6 @@ name: provisioning -description: Step resilience demo - retry, onError, {error} and cleared step data (dirigible #6762) +description: Step resilience demo - retry, onError, {error} and cleared step data on a delegate + and on a send (dirigible #6762, #7056) entities: - name: ProvisioningStatus @@ -44,6 +45,24 @@ processes: uses: [dbPassword] retry: { count: 2, every: PT10S } onError: recordFailure + next: announce + # Only here so the sample can show both endings side by side: a tenant titled "notify" takes + # the send, every other one goes straight to Provisioned. + - name: announce + kind: decision + args: { if: "title == 'notify'", then: notifyOwner, else: markDone } + # A SENDING step declaring its own resilience (#7056). Its whole work is the message, so a + # delivery failure fails the task - and this sample has no SMTP, so it always does. The + # declared cycle re-attempts it once and the onError route then records the FINAL attempt's + # message; without the two keys this step would dead-letter and the flow would stop here, + # with the record left in Requested and nothing anywhere to say why. Note it is NOT the last + # step: that workaround is exactly what the declaration makes unnecessary. + - name: notifyOwner + kind: serviceTask + args: + notify: { to: owner@example.com, subject: "Tenant {title} provisioned", body: "The application is ready." } + retry: { count: 1, every: PT5S } + onError: recordFailure next: markDone - name: markDone kind: serviceTask