diff --git a/README.md b/README.md index 557985b..d558662 100644 --- a/README.md +++ b/README.md @@ -410,9 +410,9 @@ let cache = CacheKit::production("redis://localhost:6379").await? }) .build()?; -// Opt a preset out: a config with all layers `None` applies no wrapping. +// Opt a preset out: a disabled config applies no wrapping. let bare = CacheKit::production("redis://localhost:6379").await? - .reliability(ReliabilityConfig { retry: None, circuit_breaker: None, backpressure: None }) + .reliability(ReliabilityConfig::disabled()) .build()?; ``` diff --git a/crates/cachekit/src/client.rs b/crates/cachekit/src/client.rs index 634dd61..6d811ba 100644 --- a/crates/cachekit/src/client.rs +++ b/crates/cachekit/src/client.rs @@ -1074,15 +1074,13 @@ impl CacheKitBuilder { }; // Apply the reliability stack last so it decorates the final backend. - // A config with no layer set is the documented opt-out: skip the - // (no-op) decorator entirely. + // A disabled config is the documented opt-out: skip the (no-op) + // decorator entirely. The layer check lives on ReliabilityConfig + // itself so a future layer can't be missed here (panel finding — + // this gate shipped that exact bug once already). #[cfg(all(feature = "reliability", not(target_arch = "wasm32")))] let backend = match self.reliability { - Some(config) - if config.retry.is_some() - || config.circuit_breaker.is_some() - || config.backpressure.is_some() => - { + Some(config) if !config.is_disabled() => { crate::reliability::wrap_reliable(backend, config) } _ => backend, diff --git a/crates/cachekit/src/intents.rs b/crates/cachekit/src/intents.rs index 1008f8f..95b8524 100644 --- a/crates/cachekit/src/intents.rs +++ b/crates/cachekit/src/intents.rs @@ -15,8 +15,9 @@ //! ¹ Retry with backoff + jitter, a circuit breaker, and backpressure //! (bounded backend concurrency) around backend ops (requires the //! `reliability` feature, on by default — see [`crate::reliability`]). -//! Override via [`CacheKitBuilder::reliability`]; a config with all layers -//! `None` disables the stack entirely. +//! Override via [`CacheKitBuilder::reliability`]; +//! [`ReliabilityConfig::disabled()`](crate::reliability::ReliabilityConfig::disabled) +//! turns the stack off entirely. use std::time::Duration; diff --git a/crates/cachekit/src/reliability.rs b/crates/cachekit/src/reliability.rs index 51bc89b..99affc2 100644 --- a/crates/cachekit/src/reliability.rs +++ b/crates/cachekit/src/reliability.rs @@ -122,7 +122,9 @@ impl Default for CircuitBreakerConfig { #[derive(Debug, Clone, PartialEq)] pub struct BackpressureConfig { /// Maximum backend data operations in flight at once (default: 100). - /// `0` behaves as `1`. + /// `0` behaves as `1`; values above tokio's `Semaphore::MAX_PERMITS` + /// (`usize::MAX >> 3`) are clamped to it, so `usize::MAX` reads as + /// "effectively unbounded" rather than panicking the builder. pub max_concurrent: usize, /// Maximum callers waiting for a permit before further calls are shed /// immediately (default: 1000). `0` disables waiting entirely: a call @@ -178,6 +180,37 @@ impl Default for ReliabilityConfig { } } +impl ReliabilityConfig { + /// A config with every layer off — the documented preset opt-out. + /// + /// Prefer this over spelling out a struct literal with all-`None` + /// fields: a literal breaks downstream code every time the stack gains + /// a layer (it has, twice). + /// + /// ``` + /// use cachekit::reliability::ReliabilityConfig; + /// + /// assert!(ReliabilityConfig::disabled().is_disabled()); + /// assert!(!ReliabilityConfig::default().is_disabled()); + /// ``` + #[must_use] + pub fn disabled() -> Self { + Self { + retry: None, + circuit_breaker: None, + backpressure: None, + } + } + + /// `true` when no layer is enabled — the builder skips the (no-op) + /// `ReliableBackend` decorator entirely. Lives here, next to the fields, + /// so adding a layer cannot silently miss the builder gate again. + #[must_use] + pub fn is_disabled(&self) -> bool { + self.retry.is_none() && self.circuit_breaker.is_none() && self.backpressure.is_none() + } +} + // ── RetryPolicy ────────────────────────────────────────────────────────────── /// Retries an operation on errors where [`crate::error::BackendErrorKind::is_retryable`] is @@ -485,8 +518,15 @@ impl ConcurrencyLimiter { Self { // `Semaphore::new(0)` would shed every call after acquire_timeout // with nothing ever admitted — clamp like RetryConfig's "0 - // behaves as 1". - semaphore: tokio::sync::Semaphore::new(config.max_concurrent.max(1)), + // behaves as 1". The upper clamp matters too: `Semaphore::new` + // PANICS above `MAX_PERMITS` (usize::MAX >> 3), and usize::MAX + // is the natural "effectively unbounded" sentinel a caller will + // reach for — a config value must never panic the builder. + semaphore: tokio::sync::Semaphore::new( + config + .max_concurrent + .clamp(1, tokio::sync::Semaphore::MAX_PERMITS), + ), waiting: std::sync::atomic::AtomicUsize::new(0), config, } @@ -811,6 +851,19 @@ mod tests { drop(permit); } + #[tokio::test] + async fn limiter_clamps_huge_max_concurrent_instead_of_panicking() { + // usize::MAX is the natural "unbounded" sentinel; Semaphore::new + // panics above MAX_PERMITS, so the constructor must clamp. + let limiter = ConcurrencyLimiter::new(BackpressureConfig { + max_concurrent: usize::MAX, + max_queue: 0, + acquire_timeout: Duration::from_millis(10), + }); + let permit = limiter.acquire().await.expect("clamped limiter admits"); + drop(permit); + } + #[tokio::test] async fn limiter_sheds_immediately_when_queue_disabled() { let limiter = ConcurrencyLimiter::new(BackpressureConfig {