allthingssecurity opened a new pull request, #28755:
URL: https://github.com/apache/camel/pull/28755

   # Description
   
   [CAMEL-26332](https://issues.apache.org/jira/browse/CAMEL-26332)
   
   After a route called from a `doTry` body continued a failure 
(`onException(...).continued(true)`), no `doCatch` of that `doTry` catches a 
later exception in the same `doTry` body. The exception escapes the `doTry`. 
This is a regression in 4.23.0 (CAMEL-25115); 4.22.1 catches.
   
   ```java
   onException(NumberFormatException.class).continued(true);
   
   from("direct:start")
       .doTry()
           .to("direct:callee")          // fails, the onException continues it
           .to("mock:next")              // runs
           .throwException(new IllegalStateException("after the call"))
       .doCatch(IllegalStateException.class)
           .to("mock:catch")             // never runs on main and 4.23.0
       .end();
   from("direct:callee").throwException(new NumberFormatException("in 
callee")).to("mock:calleeEnd");
   ```
   
   The same happens when the callee is called with a routing slip, a recipient 
list or a dynamic router, and when the `onException` redelivers before it 
continues.
   
   Cause: CAMEL-25115 made `RedeliveryErrorHandler` store the continued 
exception in the exchange property `CamelExceptionHandled` 
(`Exchange.EXCEPTION_HANDLED`), so that it is not added as a suppressed 
exception of a later failure. `CatchProcessor` uses the same property as its 
marker that a previous `doCatch` of the `doTry` handled the exception, and 
skips the clause when the property has any value. `TryProcessor` clears the 
property only when the `doTry` starts, and a `direct:` call shares the 
exchange, so the continued failure in the called route disabled every `doCatch` 
of the caller's `doTry`.
   
   Changes:
   - `RedeliveryErrorHandler` keeps the continued exception under an exchange 
property of its own (`CamelErrorHandlerContinuedException`, a private 
constant), and compares with that property in the check of CAMEL-25115. 
`CamelExceptionHandled` is again set only by a `doCatch` (and by a circuit 
breaker fallback), as in 4.22. `CatchProcessor`, `TryProcessor` and 
`CatchDefinition.acceptDebugger` (the debugger reads the property the same way) 
are not changed.
   - This also closes a gap in the CAMEL-25115 check: a `doTry` removed 
`CamelExceptionHandled` when it started, so a failure that was continued before 
a `doTry` was still added as a suppressed exception to a failure inside the 
`doTry`.
   - Why not the smaller change of `CatchProcessor` skipping the clause only 
when the property is `Boolean.TRUE`: the error handler overwrites the `true` of 
a `doCatch` when that `doCatch` calls a route that continues a failure. An 
exception that this `doCatch` then throws was caught by the next `doCatch` of 
the same `doTry` (on main it is not, as in Java). The new control test checks 
it.
   - No upgrade guide entry: the `doCatch` catches again as in 4.22. The 
property `CamelExceptionHandled` no longer holds the continued exception after 
a continued failure; it did so only in 4.23.0 and is not documented.
   
   Tests:
   - New `DoCatchAfterContinuedFailureInCalledRouteTest` (camel-core, 9 tests, 
every send bounded, no sleeps):
     - A continued failure in a route called with `to`, a routing slip, a 
recipient list and a dynamic router, and a continued failure after a 
redelivery. Each send checks that only the first matching `doCatch` runs, that 
it sees the `IllegalStateException` as the caught exception without a 
suppressed exception, and that the route continues after the `doTry`. The `to` 
test also checks that `CamelExceptionHandled` is not set after the call (what 
the `doCatch` and the debugger read).
     - A failure continued before the `doTry` is not added as a suppressed 
exception to a failure inside the `doTry` (as without the `doTry`, 
`ErrorHandlerFailurePathsTest.testContinuedThenUnrelatedFailure`).
     - These 6 fail on main and pass with the change.
     - Three controls pass on main as well: nothing before the exception; a 
called route that does not fail; and an exception thrown by a `doCatch` (after 
it called a route that continued a failure) is not caught by the next 
`doCatch`. The last one fails with the `Boolean.TRUE` variant above.
   - camel-core: the doTry/doCatch/doFinally, onException, redelivery, dead 
letter channel, error handler, routing slip, recipient list, dynamic router and 
pipeline tests (`*Try*`, `*Catch*`, `*Finally*`, `*OnException*`, 
`*Redelivery*`, `*DeadLetter*`, `*ErrorHandler*`, `*Continue*`, 
`*RoutingSlip*`, `*RecipientList*`, `*DynamicRouter*`, `*Pipeline*`): 881 
tests, 0 failures, 10 skipped (including `ErrorHandlerFailurePathsTest`, 10 
tests).
   - The whole camel-core suite (one run, with the core modules it depends on): 
8327 tests, 45 skipped, 36 failed, none of them a doTry, error handler or EIP 
test.
     - 24 are log tests (`LogEndpointTest`, `LogDebugBodyStreamsTest`, 
`LogEipMaskTest`, ...) that read INFO log output; I had set the camel-core test 
log level to WARN locally to keep the logs small.
     - The other 12 are known failures of my local offline build, also on main: 
`SimpleSyntaxHintsTest` (5), two OGNL hint tests of `SimpleTest`, 
`FileProducerChecksumOfWrittenFileTest` (3), 
`FilePollEnrichDynamicFileNameTest` and `RouteWatcherReloadChangedRoutesTest`.
     - In camel-xml-io and camel-yaml-io, `ModelWriterTest` (4) and 
`YamlModelWriterTest` (1) fail because my offline build regenerates the model 
writers differently; the regenerated files are not part of the commit.
   
   Not changed (follow-up if wanted): the circuit breaker fallbacks of 
camel-resilience4j and camel-microprofile-fault-tolerance set 
`CamelExceptionHandled` to `true` on the exchange, like a `doCatch`. So after a 
circuit breaker inside a `doTry` body ran its fallback, a later exception in 
the same `doTry` body is not caught by a `doCatch` either. This is older than 
CAMEL-25115 (4.22 does the same) and is not touched here; I found it by reading 
the code and did not test it.
   
   How it was found: a Rocq specification of Camel's error handling, written 
from the documentation, was compared with real Camel on generated routes. The 
case was then reduced and reproduced on HotSpot.
   
   Backport: not needed. camel-4.22.x and 4.18.x do not have the CAMEL-25115 
change; there, only the error handler task used without `onException` and 
without redelivery stores an exception in the property, and it never continues 
an exchange.
   
   # 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-26332) 
filed for the change (usually before you start working on it): 
[CAMEL-26332](https://issues.apache.org/jira/browse/CAMEL-26332).
   
   # 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. 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]

Reply via email to