Skip to content

template-application-events-java: stamp notify outcome on load/render failures too (#7290) - #7313

Open
nedelcho-delchev-tues wants to merge 1 commit into
eclipse-dirigible:masterfrom
nedelcho-delchev-tues:issue-7290-notification-outcome-stamp
Open

template-application-events-java: stamp notify outcome on load/render failures too (#7290)#7313
nedelcho-delchev-tues wants to merge 1 commit into
eclipse-dirigible:masterfrom
nedelcho-delchev-tues:issue-7290-notification-outcome-stamp

Conversation

@nedelcho-delchev-tues

Copy link
Copy Markdown
Contributor

Summary

  • Notification.java.template only wrapped Mail.send in the try that stamps outcome:. The relation loads and the attach: print/report render ran before it, so a load failure (a connection blip, a repository refusal) or a broken .print template propagated straight out of onMessage with no stamp: the broker redelivers forever, the record's outcome field stays empty - the silent state #7023 introduced the stamp to remove - and event: { onNotifyFailed: ... } never fires.
  • Widen the try to open right after the guard check, covering the relation loads, the attachment render and the send, mirroring the notify branch of Job.java.template (#7278) and Transition.java.template, which already open their try before the loads.

Note for the maintainers

While tracing this I checked the other two templates carrying the same stampNotifyOutcome machinery. Transition.java.template is correct, as the issue states. Send.java.template, however, appears to have the same gap in both its fan-out and non-fan-out branches - its shared per-record body still only wraps Mail.send, not the relation loads or the attachment render, in either branch. I left it untouched since the issue names only Notification.java.template and I didn't want to expand scope unilaterally on something the issue text asserts is already fixed - flagging it here in case a follow-up issue is warranted.

Test plan

  • mvn formatter:validate (repo-wide) - BUILD SUCCESS
  • Added a test to IntentEngineIT (an_event_notification_keeps_its_relation_loads_and_attachment_render_inside_the_fail_soft_try) asserting the generated Notification's try opens before the relation load and the attachment render, and both outcome stamps (sent / failed: <reason>) still land inside it
  • mvn -pl tests/tests-integrations -am -P integration-tests -Dit.test="IntentEngineIT#an_event_notification_keeps_its_relation_loads_and_attachment_render_inside_the_fail_soft_try" -D selenide.headless=true install - green

Fixes #7290

🤖 Generated with Claude Code

… failures too (eclipse-dirigible#7290)

Notification.java.template only wrapped Mail.send in the try that stamps `outcome:`.
The relation loads and the attach:print/report render ran before it, so a load
failure (a connection blip, a repository refusal) or a broken .print template
propagated straight out of onMessage with no stamp: the broker redelivers forever,
the record's outcome field stays empty - the silent state eclipse-dirigible#7023 introduced the
stamp to remove - and `event: { onNotifyFailed: ... }` never fires.

Widen the try to open right after the guard check, covering the relation loads,
the attachment render and the send, mirroring the notify branch of
Job.java.template (eclipse-dirigible#7278) and Transition.java.template, which already open their
try before the loads.

Verified: mvn formatter:validate (repo-wide, BUILD SUCCESS); added a test to
IntentEngineIT asserting the generated Notification's try opens before the
relation load and the attachment render, and both outcome stamps still land
inside it - ran green.

Fixes eclipse-dirigible#7290

Co-Authored-By: Claude Sonnet 5 <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

1 participant