allthingssecurity opened a new pull request, #29584: URL: https://github.com/apache/camel/pull/29584
# Description [CAMEL-26447](https://issues.apache.org/jira/browse/CAMEL-26447) Follow-up of CAMEL-26434 (#28786), named in its review. When the `doTry` block calls a route (such as with `direct`) that fails unhandled (default error handler, no `onException`), that route's error handler marks the exchange as failure handled (`failureHandled=true`, `errorHandlerHandled=false`). CAMEL-26434 clears both marks while the `doCatch` block runs, but `CatchProcessor` set `failureHandled` again after the block. The mark then stayed on the exchange for the rest of the calling route, so the error handlers that came later treated the exchange as done (`RedeliveryErrorHandler.isDone`), which is not what happens when the `doTry` fails in the route itself: | case | doTry fails in a called route (before) | doTry fails in the route (control) | |---|---|---| | later failure in the calling route | `IOException`, its `onException` not run | handled by the `onException` | | route called from the `doFinally` | its dead letter channel not run | dead letter channel run | | the `doCatch` throws, the calling route has a dead letter channel | the outer dead letter channel not run | outer dead letter channel run | | a route called later with a plain `to` | its dead letter channel not run | dead letter channel run | Change: `CatchProcessor` no longer sets the failure handled mark again after the `doCatch` block: the `doCatch` handled the failure, so the exchange carries no mark, as when the `doTry` failed in the same route. The "handled by the error handler" mark is restored as before (only a `true` that the block did not set itself). A failure that the block handles itself (such as a dead letter channel of a route it calls) still marks the exchange through that error handler. This changes what the readers of the mark see at the end of such an exchange, the same as for a `doTry` that failed in the route itself: - the Idempotent Consumer (`IdempotentOnCompletion`) confirms the message id instead of removing it, so the message is not processed again; - `ResumableCompletion` records the offset; - the `failuresHandled` counters (JMX `ManagedPerformanceCounter`, the Micrometer and OpenTelemetry route policies) no longer count it; - with `exactlyOnce` on a Kafka consumer (`KafkaTransactionSynchronization`), the output transaction (including records sent in the `doTry` before the failure) is committed together with the offsets instead of being aborted, unless the caught failure was a Kafka send failure; this matches the Kafka docs, which call sends in a `doCatch` body local recovery in the normal source-output transaction; - an EIP whose parent exchange takes over the results of its copy (`multicast`, `recipientList`, `loop`, `routingSlip`, the failover load balancer, `shareUnitOfWork`) takes over the cleared mark. And as the later error handlers now handle a new failure, they redeliver and log it as usual, and a consumer whose exchange completes with such a failure handled commits or acknowledges the message, as for any handled failure. The JIRA asked for an upgrade-guide entry and a way back with the fix. As CAMEL-26434 is in 4.24 as well and its entry already says that the exchange no longer keeps the marks of the caught failure, the text is added to that entry (retitled to cover both); it names the readers above and the way back. There is no option for the old end-of-exchange outcome: for the Idempotent Consumer, rethrowing in the `doCatch` with `removeOnFailure` (the default) removes the id; as a last resort a Java processor can set the mark at the end of the `doCatch` block (`exchange.getExchangeExtension().setFailureHandled(true)`, an internal API), which applies when the block completes. No DSL option is planned unless users ask for one. Not changed, a follow-up (JIRA to be filed): a redelivery counter that the route called from the `doTry` left on the message (`CamelRedeliveryCounter`, with `maximumRedeliveries` > 0, for example a CamelContext-scoped error handler) is not reset when the `doCatch` takes over, so a later error handler counts its redeliveries on from it and may redeliver fewer times (or not at all) than when the `doTry` failed in the route. The same holds on main since CAMEL-26434 for the route called from the `doCatch`. The fix would mark the redelivery as over when the `doCatch` takes over, as CAMEL-26734 does (the header stays readable in the `doCatch` block); that needs a helper in `RedeliveryErrorHandler`, so I kept it out of this change. One more Kafka case, also a follow-up for camel-kafka (not reproduced, from reading `KafkaTransactionSynchronization`): with `exactlyOnce`, when the route called from the `doTry` redelivers a Kafka send whose retry is rejected inside the open transaction (rollback only plus its retry-rollback property), the synchronization committed only the offsets when the exchange was failure handled (the record was skipped, as for the in-route control). As the `doCatch` now clears the mark, the transaction is rolled back and the record is replayed. The synchronization could treat that retry-rollback property on its own; that belongs in camel-kafka with its own test. Main only: the change is a behaviour change with an upgrade-guide entry, on top of CAMEL-26434. Tests, new `DoCatchFailureHandledMarkTest` (7 cases, each run with the `doTry` calling a route that fails unhandled and, as a control, with the `doTry` failing in the route itself): - `laterFailureIsHandledByTheOnException`, `routeCalledFromTheDoFinallyHandlesItsFailure`, `failureOfTheDoCatchGoesToTheDeadLetterChannelOfTheOuterRoute`: the three cases of the JIRA; - `failureOfTheDoCatchIsNotFailureHandled`: the route whose `doCatch` throws, called directly, fails with that exception and is not failure handled; - `routeCalledLaterHandlesItsFailure`: a route called with a plain `to` after the `doTry`; - `processedMessageIsConfirmedByTheIdempotentConsumer`: the id is kept and a duplicate is skipped; - `doCatchCanMarkTheExchangeAsFailureHandled`: the way back (the id is removed and the message is processed again). And in camel-management, new `ManagedDoCatchFailuresHandledTest` (the same two variants): the calling route counts the exchange as completed, not as a failure handled. On main the "called route" cases of the first six tests and of `ManagedDoCatchFailuresHandledTest` fail, and their in-route controls pass; the way back passes on main too (it sets the mark that main sets anyway). Two tests of CAMEL-26434 in `DoCatchCalledRouteAfterUnhandledCalleeTest` asserted the old mark (`isFailureHandled()` true after the `doCatch`): `callerContinuesAfterTheDoTryWhenTheDoCatchDoesNotFail` and `doCatchThatFailsFailsTheExchangeAsBefore`; they now assert false, as for the in-route `doTry` (pinned by `failureOfTheDoCatchIsNotFailureHandled`). With the change the new tests pass; the camel-core tests pass (9101 run, 1 failure, 45 skipped: the failure is `FileConsumerInterceptEmptyFileTest`, a file consumer timing test without doTry that passed when run again), as do the camel-management tests on the doTry, the redelivery, the failures handled counters and the new test (21 run, 0 failures). # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it): [CAMEL-26447](https://issues.apache.org/jira/browse/CAMEL-26447). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built the core modules with the formatter and import-sort plugins and ran the camel-core tests; no generated file changes. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
