CAMEL-24137: Circuit Breaker EIP modernization (phase 1)#24957
Merged
Conversation
gnodet
requested changes
Jul 20, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Excellent modernization work — the Vavr removal, exchange property bug fixes, and new resilience4j 2.x feature exposure are all clean and well-tested. However, there's a gap in the duration unit migration that would silently break camel-main users.
Critical: Resilience4jConfigurationProperties not updated — silently breaks application.properties users
core/camel-main/src/main/java/org/apache/camel/main/Resilience4jConfigurationProperties.java was not modified by this PR. It still uses Integer type with default 60 for waitDurationInOpenState and slowCallDurationThreshold.
The flow for a user setting camel.resilience4j.waitDurationInOpenState=60 in application.properties:
Resilience4jConfigurationProperties.waitDurationInOpenStateis set to Integer60- Copied to
Resilience4jConfigurationDefinitionas String"60" - The reifier now calls
Duration.ofMillis(parseDuration("60"))= 60 milliseconds
Before this PR, step 3 was Duration.ofSeconds(parseLong("60")) = 60 seconds. The same raw value now produces a 1000× shorter duration with no warning. This is especially impactful since camel-main is the common configuration path for Camel Quarkus and Camel Spring Boot users.
This class needs:
waitDurationInOpenStateandslowCallDurationThreshold: change type toString, default to"60000"(or"60s")- Add the three new fields:
slidingWindowSynchronizationStrategy,maxWaitDurationInHalfOpenState,bulkheadFairCallHandlingEnabled - Regenerate
Resilience4jConfigurationPropertiesConfigurer
Important: int overloads silently change semantics
The waitDurationInOpenState(int) and slowCallDurationThreshold(int) Java API overloads still exist and store Integer.toString(value). Before this PR, the reifier parsed with Duration.ofSeconds(), now with Duration.ofMillis(parseDuration()). So waitDurationInOpenState(60) changes from 60 seconds to 60 milliseconds — compiles without error, fails at runtime with a circuit breaker that re-closes nearly instantly.
Consider either deprecating these int overloads or explicitly mentioning them in the upgrade guide (the current upgrade guide covers string-based configuration but not the Java API).
Suggestion: Upgrade guide could mention the camel-main properties path
The upgrade guide examples show the Java DSL migration clearly but don't mention that users configuring via camel.resilience4j.* in application.properties are also affected. Calling this out would help since it's a common configuration path.
What looks great
- Z1 (Vavr removal): Clean replacement —
Try.ofCallable().andThen().recover().get()→ try-catch preserves identical semantics. - Z8 (Exchange property fixes): Excellent root cause analysis —
RESPONSE_TIMED_OUTwas only set in the no-fallback path, andEXCEPTION_CAUGHTwas null during timeouts because theTimeoutExceptionis thrown on the caller thread rather than set on the exchange. The fix usingexchange.getException() != null ? exchange.getException() : throwableis correct. - Z6 (Deprecation):
forRemoval = trueis appropriate for a Hystrix-era concept both implementations reject withUnsupportedOperationException. - Test coverage for Z8: Four new tests cover the timeout × fallback × pooled-exchange matrix nicely.
Minor
- No tests for the three new resilience4j 2.x options (Z5) — a basic wiring test verifying DSL options propagate to
CircuitBreakerConfig/BulkheadConfigwould catch incorrect enum names or method references. - CI is still 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.
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
github-actions
Bot
added
components
core
catalog
docs
dsl
core-build-and-dependencies
labels
gnodet
approved these changes
Jul 20, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
All review feedback addressed in 4a0738698ea6 — thank you for the thorough response:
-
Resilience4jConfigurationPropertiesfixed — all duration fields changed fromIntegertoStringwith"60s"defaults. Three new fields (slidingWindowSynchronizationStrategy,maxWaitDurationInHalfOpenState,bulkheadFairCallHandlingEnabled) added. Generated configurer regenerated. ✅ -
intoverloads deprecated — all four overloads now carry@Deprecated(since = "4.22", forRemoval = true)with Javadoc pointing to theStringalternatives. ✅ -
Upgrade guide expanded — explicit
application.propertiesmigration section with before/after examples, plus deprecation notice for Java DSLintoverloads. ✅ -
Wiring test added —
ResilienceNewOptionsTest(144 lines) verifies all three new resilience4j 2.x options propagate correctly through the DSL toCircuitBreakerConfig/BulkheadConfig. ✅
CI still pending on the new commit.
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.
davsclaus
commented
Jul 20, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
Author
There was a problem hiding this comment.
Thanks @gnodet — all four items from your review have been addressed in 4a07386:
1. Critical: Resilience4jConfigurationProperties updated — Changed waitDurationInOpenState and slowCallDurationThreshold from Integer to String type with defaults "60s" instead of 60 (Integer). bulkheadMaxWaitDuration and timeoutDuration also changed to String. Added the three new Z5 fields (slidingWindowSynchronizationStrategy, maxWaitDurationInHalfOpenState, bulkheadFairCallHandlingEnabled) with @Metadata annotations matching the model. Regenerated Resilience4jConfigurationPropertiesConfigurer.
2. int overloads deprecated — All four int overloads (waitDurationInOpenState(int), slowCallDurationThreshold(int), bulkheadMaxWaitDuration(int), timeoutDuration(int)) now carry @Deprecated(since = "4.22", forRemoval = true) with Javadoc pointing to the String overload.
3. Upgrade guide expanded — Added application.properties migration examples showing camel.resilience4j.waitDurationInOpenState = 60 → 60s or 60000. Also documented the deprecated int Java API overloads.
4. Z5 wiring test added — ResilienceNewOptionsTest with 3 tests verifying DSL options propagate to CircuitBreakerConfig/BulkheadConfig: slidingWindowSynchronizationStrategy(LOCK_FREE), maxWaitDurationInHalfOpenState(30s), bulkheadFairCallHandlingEnabled(false).
Full reactor build passes (mvn clean install -DskipTests).
Claude Code on behalf of davsclaus
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 547 tested, 25 compile-only — current: 543 all testedMaveniverse Scalpel detected 572 affected modules (current approach: 543).
|
davsclaus
force-pushed
the
fix/CAMEL-24137-circuit-breaker-modernize
branch
from
July 20, 2026 17:39
4a07386 to
ca760c1
Compare
3 tasks
Contributor
Author
|
Need to be rebased on #24960 |
davsclaus
force-pushed
the
fix/CAMEL-24137-circuit-breaker-modernize
branch
from
July 20, 2026 20:41
ca760c1 to
6d8c71e
Compare
gnodet
approved these changes
Jul 20, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Follow-up review: commit 6d8c71e — address review feedback
The follow-up commit cleanly addresses remaining gaps from the initial circuit breaker modernization:
Changes reviewed
-
ConfigurationProperties alignment —
waitDurationInOpenState,slowCallDurationThreshold,bulkheadMaxWaitDuration,timeoutDurationmigrated fromIntegertoStringinResilience4jConfigurationProperties, aligning with theConfigurationDefinitionchanges from the first commit. This meanscamel.resilience4j.*application properties now accept Camel duration expressions (e.g.60s,1m). -
Backward compatibility — 4 int-accepting overloads properly deprecated with
@Deprecated(since = "4.22", forRemoval = true)and@linkto the String replacements. Good migration path. -
New options wired to ConfigurationProperties —
slidingWindowSynchronizationStrategy,maxWaitDurationInHalfOpenState,bulkheadFairCallHandlingEnabledall added with correct@Metadataannotations (defaults, enums). -
ResilienceNewOptionsTest — Good wiring test covering all 3 new options: verifies
LOCK_FREEstrategy,"30s"→ 30000ms parsing, and bulkhead fair call handling flag. Test follows Camel conventions (CamelTestSupport, Navigate traversal for processor discovery). -
Documentation —
main.adoctable updated with new types/options, upgrade guide formatting improved.
LGTM ✅
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.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…eprecate int overloads, upgrade guide, wiring test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
davsclaus
force-pushed
the
fix/CAMEL-24137-circuit-breaker-modernize
branch
from
July 21, 2026 05:29
6d8c71e to
d62ca4f
Compare
…dering fix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
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
2 participants
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.
Summary
Circuit Breaker EIP modernization — phase 1 of CAMEL-24137. Covers sub-items Z1, Z4, Z5, Z6, Z7, and Z8.
Z1 — Remove Vavr dependency: Replaced the single Vavr
Trymonad usage inResilienceProcessorwith a plain try-catch. Removedio.vavr:vavrandio.vavr:vavr-matchdirect dependencies and thevavr-versionproperty fromparent/pom.xml(no other module used Vavr).Z4 — Duration type migration: Changed 4 duration fields from
int(seconds/millis) toStringwithjavaType = "java.time.Duration"inResilience4jConfigurationCommon. The reifier now usesparseDuration()which accepts Camel duration expressions (60s,1m,500ms) and also plain numbers as milliseconds. Oldintoverloads are deprecated with@Deprecated(since = "4.22", forRemoval = true).Z5 — Expose new resilience4j 2.x options: Added
slidingWindowSynchronizationStrategy(LOCK_FREE/SYNCHRONIZED),maxWaitDurationInHalfOpenState, andbulkheadFairCallHandlingEnabledto the model, definition, reifier, and camel-main configuration properties. Includes wiring test verifying DSL → resilience4j config propagation.Z6 — Deprecate
onFallbackViaNetwork(): Marked as@Deprecated(forRemoval = true)with descriptive Javadoc — a Hystrix-era concept both current implementations reject.Z7 — Improve
micrometerEnableddescription: Updated the Javadoc/metadata description to clarify this is a global setting requiring thecamel-resilience4j-micrometerJAR.Z8 — Fix exchange property bugs: Fixed
RESPONSE_TIMED_OUTonly being set in the no-fallback path, andEXCEPTION_CAUGHTbeing null during timeouts (exception was on caller thread, not the exchange). Added 4 tests covering the timeout × fallback × pooled-exchange matrix.Review feedback addressed:
Resilience4jConfigurationProperties— changed 4 duration fields fromIntegertoString(defaults like"60s") to prevent silent 1000× duration reduction forapplication.propertiesusersResilience4jConfigurationPropertieswith@MetadataannotationsResilience4jConfigurationPropertiesConfigurerintoverloads inResilience4jConfigurationDefinitionapplication.propertiesmigration examples and deprecated overloads documentationResilienceNewOptionsTestwith 3 wiring tests for Z5 optionsTest plan
ResilienceTimeoutWithFallbackTest— timeout with fallback setsRESPONSE_TIMED_OUT=trueandRESPONSE_SUCCESSFUL_EXECUTION=falseResilienceTimeoutWithFallbackPooledExchangeTest— same with pooled exchangeResilienceTimeoutWithoutFallbackTest— timeout without fallback sets both properties correctlyResilienceTimeoutWithoutFallbackPooledExchangeTest— same with pooled exchangeResilienceNewOptionsTest— Z5 options propagate from DSL to resilience4j config objectsmvn clean install -DskipTests)🤖 Generated with Claude Code