CAMEL-24136: Fix FT circuit breaker exchange properties, bulkhead, dead option, docs#24800
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
There was a problem hiding this comment.
APPROVE — Thorough cleanup of the Fault Tolerance circuit breaker implementation. Each fix is well-targeted and addresses a real issue.
Fixes reviewed:
-
Bulkhead enablement bug (
FaultToleranceReifier.java):configureBulkhead()was settingmaxConcurrentCallsandwaitingTaskQueuebut never callingsetBulkheadEnabled(true)on the target config. The bulkhead was therefore never actually wired onto theTypedGuard. Bug introduced in CAMEL-21857 (TypedGuard migration). One-line fix, high impact. -
Timeout exchange properties (
FaultToleranceProcessor.java): The old code tried to setRESPONSE_TIMED_OUTand other properties insideCircuitBreakerFallbackTask.call(), but that code was guarded byif (fallbackProcessor == null)— unreachable since the task is only instantiated when a fallback exists. The new code correctly handles timeout-without-fallback in a dedicatedcatch (TimeoutException e)block at theprocess()level, and stores the guard exception on the exchange before invoking the fallback so it's visible viaEXCEPTION_CAUGHT. Dead code cleanup is clean — ~20 lines removed with equivalent behavior preserved in the right location. -
timeoutPoolSizedeprecation: Properly deprecated across model (FaultToleranceConfigurationCommon,FaultToleranceConfigurationDefinition), runtime config (FaultToleranceConfiguration), JMX (FaultToleranceProcessor), and generated metadata. All use@Deprecated(since = "4.22.0")with clear deprecation notes referencing CAMEL-21857. -
JMX
getNumberOfSlowSuccessfulCalls()bug (ResilienceProcessor.java): Was callinggetNumberOfSlowCalls()instead ofgetNumberOfSlowSuccessfulCalls()— obvious copy-paste error. Good catch. -
ResilienceConsoledisplay fix:fr > 0→fr >= 0is correct (0% failure rate should display the full stats line); theelsebranch now properly uses thefcvariable instead of hardcodingfailure: 0. -
JMX description fix:
getFailureRate()description updated from "current failure rate in percentage" to "configured failure ratio threshold (0.0-1.0)" — it returns the static config value, not live metrics. -
Documentation: Fixed incorrect default timeout claim, corrected option names (
resilience4jConfiguration,configuration), fixed YAML example ordering (steps must come beforeonFallback), and clarified no-fallback/open-circuit behavior including thethrowExceptionWhenHalfOpenOrOpenStateoption. -
Tests: Two new focused tests covering the exact scenarios fixed — timeout without fallback (verifying all 4 exchange properties + exception type) and timeout with fallback (verifying
EXCEPTION_CAUGHTvisibility to the fallback processor andRESPONSE_TIMED_OUTon the result exchange). Both use the standardCamelTestSupportpattern.
Checklist:
- Tests included (2 new targeted tests)
- Generated metadata regenerated
- No public API breakage (deprecation, not removal)
- Documentation updated (3 EIP docs + generated metadata)
- Commit convention followed
- CI: pending
Reviewed with Claude Code on behalf of gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
…ning total slow calls, and console hiding failure counts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…ty contract, bulkhead enablement, deprecate dead option, fix docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…oolSize Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
6608b4e to
ebcab89
Compare
…scription Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Summary
Fixes medium-severity findings from a review of the Circuit Breaker EIP in
camel-microprofile-fault-tolerance(CAMEL-24136).RESPONSE_TIMED_OUTis now set in all timeout paths (with and without fallback). Guard exceptions (timeout, CB open) are now stored on the exchange before fallback invocation so the fallback can see them viaEXCEPTION_CAUGHT. Removed ~20 lines of unreachable dead code fromCircuitBreakerFallbackTask.FaultToleranceReifier.configureBulkhead()never calledsetBulkheadEnabled(true)on the target config, so the bulkhead was never actually wired onto theTypedGuard. Fixed.timeoutPoolSizeoption: Deprecated across model, configuration, DSL, and JMX with@Deprecated(since = "4.22.0"). The option has been inert since the CAMEL-21857 TypedGuard migration.getFailureRate()description now says "configured failure ratio threshold" instead of "current failure rate in percentage" (it returns the static config value, not live metrics). Documented the FT dev console metrics limitation vs resilience4j (SmallRye API does not expose per-breaker counters).circuitBreaker-eip.adoc, wrong option names inresilience4j-eip.adocandfault-tolerance-eip.adoc, incorrect no-fallback exception behavior description, and a misordered YAML example.RESPONSE_TIMED_OUTandEXCEPTION_CAUGHTvisibility in both no-fallback and with-fallback timeout scenarios. All 22 module tests pass.Test plan
mvn verifypasses incamel-microprofile-fault-tolerance(22 tests, 0 failures)camel-resilience4jare unaffectedClaude Code on behalf of davsclaus
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com