Repository navigation
CAMEL-26332: camel-core - doCatch is skipped after a route called from the doTry continued a failure - #28755
Merged
davsclaus merged 2 commits intoOct 31, 2026
Conversation
…m the doTry continued a failure After a route called from a doTry body continued a failure (onException continued(true)), no doCatch of that doTry caught a later exception in the same doTry body (regression in 4.23.0 from CAMEL-25115). CAMEL-25115 made the error handler store the continued exception in the exchange property CamelExceptionHandled, so that it is not added as a suppressed exception of a later failure. CatchProcessor uses that property as its marker that a previous doCatch of the doTry handled the exception, and skips the clause when it is set. The doTry clears the property only when it starts, and a direct call shares the exchange. The error handler now keeps the continued exception under a property of its own (CamelErrorHandlerContinuedException), so CamelExceptionHandled is again only set by a doCatch (and by a circuit breaker fallback), as in 4.22. This also closes a gap of the CAMEL-25115 check: a doTry removed CamelExceptionHandled when it started, so a failure continued before a doTry was added as a suppressed exception to a failure inside the doTry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔁 1 test passed only after a retry on JDK 21 (1 retried attempt) Recovered flaky tests on JDK 21 (1)
🔬 Scalpel shadow comparison — Scalpel: 572 of 713 tested, 27 compile-only — current: 573 all testedMaveniverse Scalpel detected 572 affected modules (current approach: 573). Skip-tests mode would test 572 modules (2 direct + 570 downstream), skip tests for 27 (generated code, meta-modules) Modules only in current approach (1)
Modules Scalpel would test (572)
Modules with tests skipped (27)
Build reactor — dependencies compiled but only changed modules were tested (2 modules, 7.1s total)Total reactor time: 7.1s
Top 20 slowest modules:
|
davsclaus
approved these changes
Oct 31, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks, a nice, minimal fix for a real 4.23.0 regression.
I traced it: since CAMEL-25115, RedeliveryTask.prepareExchangeForContinue stores the continued exception in CamelExceptionHandled, which CatchProcessor (line 135) treats as "a previous doCatch already handled it" for any non-null value, and TryProcessor only clears it when the doTry starts. So a continued failure in a direct: callee disables every doCatch of the caller. 4.22.1 only writes that property on the unreachable SimpleTask continue path, so the regression analysis is right. Moving the continued exception to a private key and comparing against it in handleException restores the 4.22 doCatch semantics without touching CatchProcessor/TryProcessor/CatchDefinition.acceptDebugger, and also closes the gap where a doTry wiped the "continued" marker. Good call explaining why the Boolean.TRUE-only variant is not enough, with a control test.
One small note, not blocking: unlike CamelExceptionHandled (an ExchangePropertyKey, stored internally), CamelErrorHandlerContinuedException is a plain string key, so after a continued failure it shows up in exchange.getProperties() and tracer/debugger listings, and removeProperties("*") removes it. That seems acceptable for an internal marker; a short comment on the constant saying it is internal on purpose would help future readers.
Agree the circuit-breaker fallbacks setting CamelExceptionHandled=true (resilience4j, MP fault tolerance) are a separate, pre-4.23 issue; a follow-up JIRA would be good.
Claude Code on behalf of Claus Ibsen. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
Contributor
Author
|
@davsclaus thanks for the review. The circuit-breaker follow-up is filed as CAMEL-26422 (resilience4j / MP fault tolerance inside a doTry setting Claude Code on behalf of allthingssecurity |
…s internal on purpose Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
@davsclaus added the comment on Claude Code on behalf of allthingssecurity |
gnodet-bot
reviewed
Oct 31, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Correct, minimal fix that decouples the "continued exception" marker from the "doCatch handled" marker (EXCEPTION_HANDLED) by introducing a dedicated CONTINUED_EXCEPTION property.
What the fix does:
- Introduces a new
private static final String CONTINUED_EXCEPTION = "CamelErrorHandlerContinuedException"constant (with clear Javadoc explaining the separation rationale) - Applies it symmetrically across all 4 change sites: 2
setPropertywrites inprepareExchangeForContinueand 2getPropertyreads inhandleException(both inner and outer class variants) - Preserves the existing reference-equality (
==) semantics for the lifecycle check - The property lifecycle is safe: it's a plain string property (not an
ExchangePropertyKey) soremoveProperties('*')cleans it up, preventing cross-exchange leakage
No issues found. ✅
This review was generated by an AI agent, Hermès on behalf of @gnodet.
davsclaus
approved these changes
Oct 31, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
TL;DR: Approve again. Since my approval of 5a0be40 the only change is the comment on CONTINUED_EXCEPTION, which addresses my earlier note. No blockers.
Thanks for the comment and for filing CAMEL-26422 for the circuit-breaker fallbacks.
[Note] The PR says no backport is needed because 4.22.x/4.18.x lack CAMEL-25115, which is right. camel-4.23.x does have it (RedeliveryErrorHandler writes the continued exception to EXCEPTION_HANDLED), so the doCatch regression is present there too; we'll decide separately whether this goes to 4.23.x.
[Nit] For the record, removeProperties("*") also clears internal properties such as CamelExceptionHandled, so that part of the comment holds for every property, not only plain ones. My earlier note suggested otherwise. No change needed.
Claude Code on behalf of Claus Ibsen. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
4 of 5 tasks
This was referenced Nov 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
CAMEL-26332
After a route called from a
doTrybody continued a failure (onException(...).continued(true)), nodoCatchof thatdoTrycatches a later exception in the samedoTrybody. The exception escapes thedoTry. This is a regression in 4.23.0 (CAMEL-25115); 4.22.1 catches.The same happens when the callee is called with a routing slip, a recipient list or a dynamic router, and when the
onExceptionredelivers before it continues.Cause: CAMEL-25115 made
RedeliveryErrorHandlerstore the continued exception in the exchange propertyCamelExceptionHandled(Exchange.EXCEPTION_HANDLED), so that it is not added as a suppressed exception of a later failure.CatchProcessoruses the same property as its marker that a previousdoCatchof thedoTryhandled the exception, and skips the clause when the property has any value.TryProcessorclears the property only when thedoTrystarts, and adirect:call shares the exchange, so the continued failure in the called route disabled everydoCatchof the caller'sdoTry.Changes:
RedeliveryErrorHandlerkeeps 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.CamelExceptionHandledis again set only by adoCatch(and by a circuit breaker fallback), as in 4.22.CatchProcessor,TryProcessorandCatchDefinition.acceptDebugger(the debugger reads the property the same way) are not changed.doTryremovedCamelExceptionHandledwhen it started, so a failure that was continued before adoTrywas still added as a suppressed exception to a failure inside thedoTry.CatchProcessorskipping the clause only when the property isBoolean.TRUE: the error handler overwrites thetrueof adoCatchwhen thatdoCatchcalls a route that continues a failure. An exception that thisdoCatchthen throws was caught by the nextdoCatchof the samedoTry(on main it is not, as in Java). The new control test checks it.doCatchcatches again as in 4.22. The propertyCamelExceptionHandledno longer holds the continued exception after a continued failure; it did so only in 4.23.0 and is not documented.Tests:
DoCatchAfterContinuedFailureInCalledRouteTest(camel-core, 9 tests, every send bounded, no sleeps):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 matchingdoCatchruns, that it sees theIllegalStateExceptionas the caught exception without a suppressed exception, and that the route continues after thedoTry. Thetotest also checks thatCamelExceptionHandledis not set after the call (what thedoCatchand the debugger read).doTryis not added as a suppressed exception to a failure inside thedoTry(as without thedoTry,ErrorHandlerFailurePathsTest.testContinuedThenUnrelatedFailure).doCatch(after it called a route that continued a failure) is not caught by the nextdoCatch. The last one fails with theBoolean.TRUEvariant above.*Try*,*Catch*,*Finally*,*OnException*,*Redelivery*,*DeadLetter*,*ErrorHandler*,*Continue*,*RoutingSlip*,*RecipientList*,*DynamicRouter*,*Pipeline*): 881 tests, 0 failures, 10 skipped (includingErrorHandlerFailurePathsTest, 10 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.SimpleSyntaxHintsTest(5), two OGNL hint tests ofSimpleTest,FileProducerChecksumOfWrittenFileTest(3),FilePollEnrichDynamicFileNameTestandRouteWatcherReloadChangedRoutesTest.ModelWriterTest(4) andYamlModelWriterTest(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
CamelExceptionHandledtotrueon the exchange, like adoCatch. So after a circuit breaker inside adoTrybody ran its fallback, a later exception in the samedoTrybody is not caught by adoCatcheither. 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
onExceptionand without redelivery stores an exception in the property, and it never continues an exchange.Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally 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
Co-authored-bytrailers) 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-Bytrailer.Claude Code on behalf of allthingssecurity
🤖 Generated with Claude Code