[
https://issues.apache.org/jira/browse/CAMEL-26333?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Work on CAMEL-26333 started by shashank.
----------------------------------------
> camel-core - doTry: when the doTry calls a route whose error handler handled
> a failure (dead letter channel or handled onException), the doFinally block
> runs only its first step
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-26333
> URL: https://issues.apache.org/jira/browse/CAMEL-26333
> Project: Camel
> Issue Type: Bug
> Components: camel-core
> Reporter: shashank
> Assignee: shashank
> Priority: Major
>
> When a {{doTry}} calls a route (for example with {{direct:}}) whose error
> handler handles a failure (a dead letter channel, or
> {{onException(...).handled(true)}}), the {{doFinally}} block runs only its
> first step and skips the rest, silently (main 8e0ce2d21371, and the released
> 4.22.1 and 4.23.0 jars):
> {code:java}
> errorHandler(deadLetterChannel("mock:dead")); // or:
> onException(IllegalArgumentException.class).handled(true)
> from("direct:a")
> .doTry()
> .to("direct:b")
> .doFinally()
> .to("mock:f1").to("mock:f2").setHeader("f3",
> constant("yes")).to("mock:f4")
> .end()
> .to("mock:end");
> from("direct:b").setBody(constant("in-b")).throwException(new
> IllegalArgumentException("x"));
> direct:b fails: mock:dead 1, mock:f1 1, mock:f2 0, header f3 null,
> mock:f4 0, mock:end 0
> direct:b does not fail: mock:dead 0, mock:f1 1, mock:f2 1, header f3 yes,
> mock:f4 1, mock:end 1
> {code}
> The doFinally EIP is described (model and catalog) as steps "to always
> execute after a doTry block completes, regardless of whether an exception was
> thrown or caught". The steps that are skipped are the ones users put there
> because they must always run (release a lock, write an audit record,
> compensate).
> Cause: the called route's error handler marks the exchange as handled by the
> error handler ({{errorHandlerHandled}}) and as failure handled
> ({{failureHandled}}). {{FinallyProcessor}} clears the exception before it
> runs the block, but not those marks. The doFinally block is a {{Pipeline}},
> which always runs its first step and then stops before each later step when
> the exchange is handled by the error handler
> ({{PipelineHelper.continueProcessing}}, "Handled by the error handler"). And
> an error handler does not handle a new failure of an exchange that is failure
> handled, so a route called from such a doFinally cannot handle or continue
> its own failure with its error handler.
> A second consequence: a routing slip or dynamic router can route to the
> endpoints of a recipient list instead of its own. A recipient list is a
> pipeline of two processors: the first evaluates the recipients into the
> exchange property {{CamelEvaluateExpressionResult}}, the second sends and
> removes the property. When the recipient list is the first step of such a
> doFinally, only the evaluation runs, and nothing is sent. A routing slip or
> dynamic router that runs later on the same exchange takes that property as
> its slip ({{RoutingSlip.process}}), so it sends to the recipient list's
> endpoints and not to its own:
> {code:java}
> errorHandler(deadLetterChannel("mock:dead"));
> from("direct:r0").doTry().to("direct:r2").doFinally().routingSlip(constant("mock:slip")).end();
> from("direct:r2").doTry().to("direct:r3").doFinally().recipientList(constant("mock:a,mock:b")).end();
> from("direct:r3").throwException(new IOException("r3 failed"));
> -> mock:slip 0, mock:a 1 (sent by the routing slip), mock:b 0
> {code}
> When the stale value is not a valid endpoint list for the routing slip's
> delimiter (a recipient list with {{delimiter(";")}}), the routing slip sends
> to one {{direct:}} endpoint without a consumer and blocks for the direct
> timeout (30 s). The same happens with {{onException(...).handled(true)}}
> instead of the dead letter channel, and with a dynamic router.
> CAMEL-19441 (Not A Bug) decided that after a called route handled the
> failure, the caller does not continue routing after the {{doTry}}. That stays
> true with the fix ({{mock:end}} stays 0); this issue is only about the
> {{doFinally}} block, which today runs exactly one step.
> *Fix* (proposed): {{FinallyProcessor}} clears both marks while the
> {{doFinally}} block runs (as {{OnCompletionProcessor}} does for an
> onCompletion), and sets them again when the block is done (the handled mark
> only if the block did not set it itself), so every step runs, a route that
> the block calls handles or continues its own failure as usual, and the route
> then stops after the {{doTry}} as before. The recipient list in the
> {{doFinally}} then sends and removes its property, so nothing stale is left
> for a routing slip. No upgrade guide entry: a {{doFinally}} that now runs all
> its steps is the fix. Routes that relied on the old behaviour see the second
> and later steps of such a {{doFinally}} run (and a recipient list or routing
> slip there send to its endpoints), once per exchange.
> Not changed: {{RoutingSlip.process}} still reads
> {{CamelEvaluateExpressionResult}}, because camel-bean's {{@RoutingSlip}} and
> {{@DynamicRouter}} annotations hand their result to the routing slip in that
> property ({{MethodInfo}}; {{RoutingSlipPOJOTest}}). A dedicated key for that
> hand-off ("TODO: Maybe use a new constant than EVALUATE_EXPRESSION_RESULT",
> from CAMEL-13449) would stop a routing slip from reading another EIP's value
> at all; it touches camel-bean and can be a separate minor issue.
> *Tests*: {{DoFinallyAfterHandledCalledRouteTest}} (6 tests: dead letter
> channel, the same with asynchronous delayed redelivery, handled
> {{onException}}, a failing route called from the doFinally that its dead
> letter channel handles, one that its onException continues, and a control
> where the called route does not fail; the bug tests also check that the
> caller does not continue after the {{doTry}}, CAMEL-19441) and
> {{RoutingSlipInDoFinallyAfterRecipientListTest}} (5 tests: routing slip with
> a dead letter channel and with a handled {{onException}}, dynamic router, the
> blocking case with a delimiter, and a control). The 9 non-control tests fail
> on main and pass with the fix; with a fix that does not set the handled mark
> again after the block, three {{doFinally}} tests fail on {{mock:end}}, and
> with a fix that clears only the handled mark, the two tests with a failing
> route called from the doFinally fail. The doTry/doCatch/doFinally,
> onException, redelivery, dead letter channel, error handler, routing slip,
> recipient list, dynamic router and pipeline tests of camel-core pass (984
> tests with the onCompletion tests, including {{RoutingSlipPOJOTest}}); the
> camel-core suite (the run was stopped by a resource guard of the machine
> after 2951 of about 3030 test classes) has only the known failures of the
> local offline build.
> *How it was found*: a Rocq specification of Camel's error handling, written
> from the documentation, was compared with real Camel on generated routes,
> then reproduced on HotSpot.
> *Affected*: main, 4.23.0, and all of camel-4.22.x, 4.18.x and 4.14.x
> ({{FinallyProcessor}}, {{Pipeline}} and {{PipelineHelper}} have the same code
> there).
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
