Skip to content

intent: step resilience reaches a notify: service task (#7056) - #7165

Merged
iliyan-velichkov merged 1 commit into
masterfrom
intent-7056-step-resilience-on-a-send
Sep 8, 2026
Merged

intent: step resilience reaches a notify: service task (#7056)#7165
iliyan-velichkov merged 1 commit into
masterfrom
intent-7056-step-resilience-on-a-send

Conversation

@iliyan-velichkov

Copy link
Copy Markdown
Contributor

Closes #7056.

The gap

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, and its own comment says so, 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.

The failure mode is worse than a lost mail. Ordered where it reads most naturally — before the terminal status write — an SMTP blink leaves a record whose real work completed sitting in its in-progress status with no error message and no failure status, because the incident is on the job, not the record, so nothing in the application can see it. The available workaround, making the send the last step, works and constrains process design for a reason that has nothing to do with the domain: the end of a process becomes the only safe place to put a declared send.

Why v1 was delegate-only, and where the second hook belongs

A runtime reason, not a modelling one — which is why this is issue option (1) and not "relax the check". The conversion that turns an exhausted attempt into the caught INTENT_STEP_FAILED error lives in ResilientClassDelegate, on the flowable:class path; a notify: step is emitted as flowable:delegateExpression="${JavaTask}" and never passes through ClassDelegate.

Flowable 8 exposes no ClassDelegateFactory analogue for that path, so the hook is the overridable factory method itself:

  • ResilientActivityBehaviorFactory (now what BpmFlowableConfig configures, still carrying ResilientClassDelegateFactory) overrides createServiceTaskDelegateExpressionActivityBehavior;
  • ResilientServiceTaskDelegateExpressionActivityBehavior 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 — which is exactly the predicate ResilientClassDelegate.execute already relies on. So IntentStepResilience is unchanged: same signature, same INTENT_STEP_FAILED / __errorMessage string contracts, same JobRetryCmd finality arithmetic. Both dispatchers (${JavaTask} and ${JSTask}) are covered, which is wider than the DSL lets anyone ask for and deliberately harmless: the conversion fires only on a task carrying the generated boundary, and the parser decides which shapes may ask for one.

Emission

The only addition is the retry cycle on the ${JavaTask} element, written into the extensionElements block the handler field already opens. The <error> definition, the cancelling boundary, its flow and its DI were already shape-agnostic (collectBoundaryErrors walks any step declaring onError), and the {error} setter glue was already generic.

A send keeps its async boundary — the synchronousNodes / completingTransactionNodes walk strips it only from check-gated setters and the decisions on the way to them, and stops at any other step — which is what makes a cycle on it re-run anything at all.

Three refusals, each with its reason in the message

The keys apply to delegate: and notify:. Everywhere else is a parse error, because the declaration would otherwise be authored and inert:

The generated sender's exception message also gained its cause (... process: <cause>): that string is verbatim what an onError route records through {error}, and a fixed sentence would put "the mail failed" on the record and nothing an operator can act on.

Coverage

Every new negative case was written red-first.

Layer Test
parser StepResilienceIntentTest — the send parses; the setter / bare / fan-out refusals (4 fail without the change)
emission ResilienceBpmnTest — the cycle rides the send's own element after its handler field, its boundary + DI, and both byte-identical pins still hold
BPMN contract IntentResilienceBpmnContractTest — the new delegate-expression shape through the real BpmnXMLConverter
live engine ResilientServiceTaskDelegateExpressionEngineTest — exhausted cycle routes the FINAL attempt and never dead-letters; recovery converts nothing; and a delegate raising its own BpmnError still reaches its own boundary (the must-not-break case the wrapper is responsible for)
IT, emission + parse IntentEngineIT
IT, runtime IntentEmissionCoverageIT — a send mid-process whose exhausted retry routes onto the record and lets the flow carry on to the hold
IT, sample journey sample-intent-resilience + IntentResilienceSampleIT — a third tenant title exercising the issue's own reproducer, through the browser IDE

Without the hook the doomed instance never finishes (it dead-letters) — that timeout is the red-first proof.

The sample

tests/tests-integrations/src/main/resources/sample-intent-resilience gains a send step mid-process, deliberately not last, reached by a tenant titled notify; the sample has no SMTP, so the delivery always fails. It ends Failed carrying Failed to send the notifyOwner mail of the TenantProvisioning process: … — which can only happen via the boundary route, since a dead-lettered send would leave it in Requested. The README's walk-through covers it, and the existing two tenants are unaffected.

Docs

🤖 Generated with Claude Code

`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 <error>, 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.<Step> 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) <noreply@anthropic.com>
@iliyan-velichkov iliyan-velichkov self-assigned this Sep 8, 2026
@iliyan-velichkov
iliyan-velichkov merged commit 90df968 into master Sep 8, 2026
10 checks passed
@iliyan-velichkov
iliyan-velichkov deleted the intent-7056-step-resilience-on-a-send branch September 8, 2026 08:07
delchev pushed a commit that referenced this pull request Sep 10, 2026
…t: <self> at parse, and correct the resilience-wiring text (#7226) (#7274)

Three leftovers from #7165 (#7056, step resilience on a notify: service task):

1. The SEND_YAML fixture was a self-loop. It built the send step with two
   .replace calls: the first inserted notifyOwner carrying `onError: recordFailure,
   next: done`; the second replaced EVERY `onError: recordFailure, next: done`
   with `... next: notifyOwner`, rewriting the just-inserted send too - giving
   notifyOwner a `next: notifyOwner` and leaving `done` unreachable. The tests
   passed because the parser only checked that next: names an EXISTING step. The
   second replace now targets provisionApp uniquely (its PT1M cycle), so the chain
   is provisionApp -> notifyOwner -> done; two assertFlow assertions pin it so the
   loop cannot come back silently. aSendWithoutResilienceKeysEmitsNoCycle also
   carried a dead first .replace ("onError: notifyOwner", ...) - that text never
   occurs - now deleted.

2. A step whose next: is itself was accepted with no diagnostic (it names an
   existing step), and a generated .bpmn with a self-targeting sequence flow spins.
   IntentParser now refuses it at parse with a message naming the step; a new
   StepResilienceIntentTest.aNextTargetingItselfIsRejected covers it. This is what
   made the fixture bug invisible, so the guard and the fixture fix are one change.

3. The resilience-wiring text was stale in two places: engine-intent/CLAUDE.md and
   ResilientClassDelegateFactory's javadoc described the wiring as
   `new DefaultActivityBehaviorFactory(...)`, while production (BpmFlowableConfig)
   wires `new ResilientActivityBehaviorFactory(new ResilientClassDelegateFactory())`
   - the class #7165 added. Both corrected.

Verified: mvn formatter:validate (BUILD SUCCESS, cache wiped); engine-intent unit
suite green (1188 tests, incl. the fixed ResilienceBpmnTest and the new
StepResilienceIntentTest case); IntentEngineIT green (74/74), confirming valid
intents still parse and generate. engine-bpm-flowable change is javadoc-only and
compiles clean. (A stale-state core-initializers flake seen mid-run - CREATE vs
UPDATE on leftover H2 - passed 9/9 on a clean run; it is upstream of and unrelated
to this change.)

Fixes #7226

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

intent: step resilience (retry:/onError:) is unavailable on a notify: service task

1 participant