allthingssecurity opened a new pull request, #28751: URL: https://github.com/apache/camel/pull/28751
# Description [CAMEL-26328](https://issues.apache.org/jira/browse/CAMEL-26328) The failover load balancer with a list of exceptions fails over when the exception that failed the attempt is not in the list but one of its suppressed exceptions is. Since CAMEL-25114 (4.23.0) an exception thrown in a `doFinally` block is added as a suppressed exception to the pending exception of the `doTry` block. So a cleanup step that throws one of the listed types now makes the load balancer fail over, and when the next processor succeeds, the exception that failed the attempt is lost. ```java from("direct:start") .loadBalance().failover(IllegalArgumentException.class) .to("direct:attempt") .to("mock:second") .end() .to("mock:after"); from("direct:attempt") .doTry() .throwException(new IOException("original")) .doFinally() .throwException(new IllegalArgumentException("cleanup")) .end(); ``` On main and 4.23.0 the exchange completes without an exception, through `mock:second` and `mock:after`. On 4.22.1 it fails with the `IOException` and does not fail over. `onException(IllegalArgumentException.class)` and `doCatch(IllegalArgumentException.class)` do not match this `IOException`, because they look at the exception and its causes only (the doTry EIP page documents this for `doCatch`). `FailOverLoadBalancer.shouldFailOver` checked each listed type with `exchange.getException(type)`. That calls `ObjectHelper.getException(Class, Throwable)`, which looks at the suppressed exceptions first and then at the cause chain. CAMEL-6406 (2.16.0) added the suppressed lookup to that general API, not for failover. CAMEL-25114 did not change the load balancer. Changes: - `FailOverLoadBalancer.shouldFailOver` matches the listed types against the exception and its causes only, with `ObjectHelper.createExceptionIterable` (the walk `DefaultExceptionPolicyStrategy` and `CatchProcessor` use). Without a list it fails over on any exception, as before. - Docs: one paragraph on the failover EIP page (and its catalog copy) on what is matched, with the `doFinally`, `onException` and `onWhen` cases. - Upgrade guide (4.24): the `doFinally` case, and the cases where Camel or a processor puts a listed exception as a suppressed exception of the exception the attempt fails with, which no longer cause failover: an `onException` block that fails while it handles the exception (the new exception carries the original one as suppressed; this is older than 4.23, see `FatalFallbackErrorHandler`), a `doCatch` whose `onWhen` throws (since CAMEL-25114 the exchange fails with the predicate's exception and the original one is its suppressed exception), and a processor or component that adds one itself (for example try-with-resources whose `close()` failed). A failing `onCompletion` in `modeBeforeConsumer` and a failing saga compensation also add their exception as suppressed, like `doFinally`. A route that relied on failover in these cases adds the type of the exception the attempt fails with to the list. Not changed: `ThrottlingExceptionRoutePolicy` checks its `throttledExceptions` with the same `exchange.getException(type)` loop, so it also counts a failure whose listed exception is only suppressed. I will file a follow-up JIRA for it rather than change a route policy in this PR. Tests: - New `FailOverLoadBalanceSuppressedExceptionTest` (camel-core, 11 tests, no sleeps). - No failover, and the exchange fails with the exception of the attempt (the `IOException`, or the new exception of the `onException` block or of the `onWhen`), which has the listed exception as its only suppressed exception: a `doFinally` that throws a listed exception, a `doFinally` that calls a route that throws one, the same with `inheritErrorHandler`, round robin and `maximumFailoverAttempts` (the inherited error handler redelivers twice first), a processor that adds one as suppressed, an `onException` block that fails while handling a listed exception, and a `doCatch` whose `onWhen` throws for a listed exception. These 6 fail on main (`expected: <java.io.IOException> but was: <null>`, and `<java.lang.IllegalStateException>` for the `onWhen` case) and pass with the change. - Controls, passing on main as well: a listed exception as the cause of an unlisted one fails over, a listed exception thrown directly fails over, a listed exception from `doTry` with an unlisted one from `doFinally` fails over, an unlisted exception does not, and `onException` does not match the suppressed exception. - camel-core: the load balancer, failover and doTry/doCatch/doFinally tests (`*LoadBalanc*Test`, `*FailOver*Test`, `*Failover*Test`, `*Try*Test`, `*Finally*Test`, `*Catch*Test`, `*Suppressed*Test`, `EipBodyTypesTest`): 196 tests, 0 failures. - The whole camel-core suite: 8329 tests, 45 skipped. 12 of them failed: `SimpleSyntaxHintsTest` (5), two OGNL hint tests of `SimpleTest`, `FileProducerChecksumOfWrittenFileTest` (3), `FilePollEnrichDynamicFileNameTest`, and `ValidatorExternalResourceTest` (it loads an XSD from raw.githubusercontent.com, which my offline build cannot reach). They fail the same way without this change in my local offline build, and none of them uses the load balancer. The formatter and import-sort plugins ran. How it was found: a Rocq specification of Camel's EIPs, written from the documentation, was compared with real Camel on generated routes. A failover that monitored `IllegalArgumentException` failed over because the doFinally of the attempt called a route that threw one. The test above then reproduced it on HotSpot. Backport: the doFinally case needs CAMEL-25114, which is only on main and 4.23.0, and the change is visible to routes (see the upgrade guide), so I leave any backport to the reviewers. # 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-26328) filed for the change (usually before you start working on it): [CAMEL-26328](https://issues.apache.org/jira/browse/CAMEL-26328). # 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 and ran the tests above, including the formatter and import-sort plugins. The failover EIP page change is copied to its catalog copy. 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]
