shashank created CAMEL-26333:
--------------------------------
Summary: 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
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)
