allthingssecurity opened a new pull request, #29668: URL: https://github.com/apache/camel/pull/29668
# Description [CAMEL-26866](https://issues.apache.org/jira/browse/CAMEL-26866) Follow-up of #29525 (CAMEL-26741) and CAMEL-26824, found in our review of CAMEL-26824. Two more places caught only `Exception`, so an `Error` that is not a `VirtualMachineError` (an `AssertionError`, a `LinkageError` such as `NoClassDefFoundError`, a custom `Error`) of user code did not fail the exchange: - **`ContractAdvice`**, the advice of the input and output type of a route (`inputType`, `outputType`): an `Error` of a transformer or validator escaped from the advice. `CamelInternalProcessor` catches only exceptions around its advices, so the `Error` was thrown to the caller of the route, without the after tasks of the advices that already ran (such as the end of the unit of work), and the exchange could stay inflight. - **`CamelInternalProcessor.processTransacted`**: an `Error` that the processor of a transacted exchange throws from its synchronous `process` was thrown on after the callback. A plain `Processor` in a step without an error handler of its own (in a `doTry`, `doCatch` or `doFinally` block, an `onException`, a circuit breaker, the outputs of a multicast) gets there when the exchange is transacted: `DelegateSyncProcessor.process(Exchange)` has no catch (#29525's catch is in its asynchronous `process`), so a `doCatch` did not get the `Error`, and it was thrown to the caller. Change: both catch a `VirtualMachineError` first and throw it on, as before, and set any other `Throwable` on the exchange (wrapped in a `CamelExecutionException` by `Exchange.setException`, as for the adapters of #29525). An exception is handled as before. As for an exception today, an input-type `Error` set by the advice is then handled by the error handler of the first step of the route. The 4.24 upgrade guide gets a section of its own, just before the one of #29525, so it merges cleanly with CAMEL-26824 (which extends #29525's section); it can be folded into that section once both are merged. The error handler page is left as it is: #29525's section there is about processors. Not in this PR, each with a follow-up JIRA I will file: - the reactive executor's worker and `MulticastTask` (an `Error` of an `AggregationStrategy` or of the iterator of a streaming split): the worker cannot fail an exchange it does not know about, so that fix belongs in the catches of `MulticastTask`, in `MulticastProcessor`, which CAMEL-26824 changes too; - the other advices of `CamelInternalProcessor` and the code around them, which also catch only `Exception`: the loop over the advices in `CamelInternalProcessor.process` and `AdviceIterator.runAfterTask`, `RestBindingAdvice` (an `Error` of a data format of the rest-dsl binding), the REST OpenAPI and Postman advices, `RoutePolicyAdvice`, `VariableAdvice`, and `SharedCamelInternalProcessor`. Tests, new `ContractAndTransactedThrowErrorTest` (camel-core): - `errorOfAnInputTransformerFailsTheExchange` and `errorOfAnOutputTransformerFailsTheExchange`: a custom `Transformer` throws an `AssertionError`; the exchange fails with it, and nothing stays inflight. - `exceptionOfAnInputTransformerFailsTheExchangeTheSameWay`: a control with an `IllegalStateException` (passes before the change too). - `doCatchCatchesAnErrorOfAProcessorOfATransactedExchange`: a transacted exchange, a plain `Processor` in a `doTry` throws an `AssertionError`: the `doCatch` gets it and the next step of the `doTry` does not run. - `errorOfTheProcessorOfATransactedExchangeFailsTheExchange`: `CamelInternalProcessor` with a transacted exchange and an `AsyncProcessor` whose `process(Exchange)` throws an `AssertionError`: the callback is called once and the exchange fails with it. - `virtualMachineErrorOfTheProcessorOfATransactedExchangeIsLeftAsBefore`: a control, a `StackOverflowError` is thrown on after the callback. Without the change the four `Error` cases fail (the `AssertionError` reaches the caller). With it the camel-core tests pass (9171 run, 0 failures, 45 skipped, before the `doCatch` test was added; the production code is the same), and the transformer, validator, transacted and step tests of camel-core pass with the final test class (109 tests). The new class also passes with the sanitizer on (`-Psanitizer`). A validator that throws an `Error` goes through the same catch as a transformer and has no test of its own. # 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-26866](https://issues.apache.org/jira/browse/CAMEL-26866). # 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. 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]
