Add community knowledge file: avoid raising events inside try functions#31
Add community knowledge file: avoid raising events inside try functions#31jeffreybulanadi wants to merge 1 commit into
Conversation
| @@ -0,0 +1,26 @@ | |||
| --- | |||
| bc-version: [all] | |||
| domain: events | |||
There was a problem hiding this comment.
I wonder if events are a domain, or if it should rather be extensibility?
JesperSchulz
left a comment
There was a problem hiding this comment.
This one's solid — the core lesson is correct and worth having. Raising an integration event synchronously inside a [TryFunction] does mean a subscriber's Error() gets caught by the try boundary and turned into a false return, so it never reaches the caller. Raising the event before entering the try scope is the right fix. 👍
Two things I'd tighten up before merging:
1. The Cancel := true comment in .bad.al contradicts its own code.
Cancel := false;
OnBeforeSubmitPayment(PaymentAmount, Cancel);
if Cancel then
exit;The comment says "A subscriber setting Cancel := true is also lost when TryFunction returns false." But trace it: a subscriber that sets Cancel := true without erroring hits if Cancel then exit, so the try method returns true, not false — Cancel is honored, not lost. The only path that returns false is a subscriber raising an error, and in that case if Cancel is never reached, so Cancel is irrelevant. So that sentence doesn't describe a real failure mode here. (It also leans on "var changes are discarded on a false return," which isn't documented — the try methods doc actually says database writes inside a try method are not rolled back.)
If anything, the real second footgun in that path is the opposite: a subscriber setting Cancel := true makes the try method exit and return true, so SubmitPayment treats a silently-skipped payment as success. I'd just drop the misleading line and keep the file focused on the one clear lesson — the swallowed Error().
2. Minor wording: the .md says "A TryFunction catches all errors." I'd soften that to "catches errors thrown during its execution" — "all errors" overstates it.
Optional nit: in .good.al, HttpClient.Get() returns a Boolean rather than throwing on a transport failure, so the try method is really isolating the explicit Error('HTTP %1'…), not Get() itself. The pattern's still fine — the "operation that can fail" framing is just a touch loose.
Net: good content, just fix the Cancel comment and the "all errors" wording.
Adds
avoid-raising-events-inside-try-functionstocommunity/knowledge/events/.When an
[IntegrationEvent]is raised inside aTryFunctionbody, any error a subscriber throws is silently caught and discarded. The subscriber's logic fails but the caller never knows. The fix is to raise the event before entering the TryFunction, so only the operation that genuinely needs error isolation lives inside it.Includes
.good.aland.bad.alsibling samples.