allthingssecurity opened a new pull request, #28781: URL: https://github.com/apache/camel/pull/28781
# Description [CAMEL-26321](https://issues.apache.org/jira/browse/CAMEL-26321) When the Splitter stops because `maxFailedRecords` or `errorThreshold` is reached (CAMEL-23264, since 4.22.0), the exchange gets the message, headers, exchange properties and variables of the split item that reached the threshold, as the steps inside the split changed them, instead of the original input. With `stopOnException`, the all-or-nothing variant, the exchange keeps the original input and only gets the exception (CAMEL-6802). ```java from("direct:max").split(body()).maxFailedRecords(1) .setHeader("item", simple("${body}")).setBody(constant("changed")) .throwException(new IllegalStateException("boom")).end(); ``` Sent with body `a,b,c` and header `orig=yes`, the exchange comes back with body `changed` and headers `{orig=yes, item=a}` (with `maxFailedRecords(2)`: `item=b`). With `stopOnException()` instead it comes back with body `a,b,c` and headers `{orig=yes}`. In both cases the exception is the `CamelExchangeException` for the failed item. The split EIP page says the Splitter returns the original input message by default, and that when the threshold is reached it "stops and sets an exception on the exchange". So a `doTry`/`doCatch` around the split, an `onException` in a calling route or an outer dead letter channel that puts the failed input aside keeps one modified item instead of the input. The items after it are not processed, so they are lost. Both stops go through the same branch of the multicast task (`shouldContinueOnFailure` returns `false`). But `MulticastProcessor.doDone` only takes the stopOnException path (set the exception, do not copy the sub-exchange back) when the `stopOnException` option is set. Otherwise it runs `ExchangeHelper.copyResults(original, subExchange)`. Changes: - `Splitter.shouldContinueOnFailure` marks its failure tracker (a new volatile `stopped` flag) when `maxFailedRecords` or `errorThreshold` stops the split. A `stop()` or a rollback in a split item does not mark it, so nothing changes for these: the exchange still gets the state of that split item, including its rollback-only flag (and, with `rollback()`, its exception). - `MulticastProcessor.doDone` takes the stopOnException path when `isStopOnException()` or the new protected `isStoppedOnFailure(original)` is true. The new method returns `false` in `MulticastProcessor`. The `Splitter` returns the tracker's flag, and only when this splitter has a threshold configured, so a tracker that a nested split inherits from an outer one is not read. - The exchange then gets the same exception and the same exchange properties as with `stopOnException`. `CamelSplitResult` is set as before. An aggregation strategy sees the stop the same way in both cases: the items before the failed one are aggregated, `onCompletion` gets the failed sub-exchange, and the aggregated result is not returned. - Docs: one sentence on the split EIP page (and its catalog copy), and an upgrade guide entry (4.24), because a route now receives the original input after such a stop. - In parallel mode, the thread of the failed item that stops the split sets the flag before it calls `doDone`, so `doDone` on that thread sees it. Items that complete after the stop are not aggregated or copied back, because the task is already done. Tests: - New `SplitterThresholdStopKeepsOriginalMessageTest` (camel-core, 14 tests, no sleeps). Each split item sets a header, a property and a variable, removes a property of the input, changes the body and throws. The tests cover `maxFailedRecords(1)`, `maxFailedRecords(2)` (one failure tolerated, and the indexes of the failures in `CamelSplitResult`), `errorThreshold(0.5)`, `parallelProcessing`, `streaming`, `shareUnitOfWork`, a transacted exchange (`MulticastTransactedTask`), a split without a threshold inside a split with `maxFailedRecords(1)`, two nested splits that both have `maxFailedRecords(1)`, and a `doTry`/`doCatch` around the split (the `doCatch` gets `a,b,c`). An aggregation strategy is compared with `stopOnException`, and so are the exception and the exchange property names. Two `stopOnException` controls (direct and inside `doTry`) pass on main as well. - The 12 other tests fail on main (body `changed`, the outer split item `[a, FAIL]` or the inner one `FAIL` for the nested splits, or `[itemProp, origProp]` instead of `[origProp]`) and pass with the change. - camel-core: the Splitter, Multicast, RecipientList, errorThreshold and maxFailedRecords tests (`*Split*Test`, `*Multicast*Test`, `*RecipientList*Test`): 613 tests, 0 failures, 6 skipped. - The whole camel-core suite (run with the change in `Splitter` and `MulticastProcessor`, before the transacted, nested and shareUnitOfWork tests were added): 8328 tests, 45 skipped. 11 of them failed: `SimpleSyntaxHintsTest` (5), two OGNL hint tests of `SimpleTest`, `FileProducerChecksumOfWrittenFileTest` (3) and `FilePollEnrichDynamicFileNameTest`. They fail the same way without this change in my local offline build, and none of them uses the Splitter or the multicast. 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. 12 of 3 000 split cases differed, all with `maxFailedRecords`. The test above then reproduced it on HotSpot. Backport: camel-4.22.x has the same code. The change is visible to routes (see the upgrade guide), so I leave the 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-26321) filed for the change (usually before you start working on it): [CAMEL-26321](https://issues.apache.org/jira/browse/CAMEL-26321). # 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 split 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]
