Fix bugs: corrupt entry telemetry, negative TTL docstring, RuntimeError crash, etc

[emiliano-go]

Bug: Corrupt cache entry records double telemetry (hit + miss)

File: src/redis_fastapi/cache.py:401-418

When a cached entry contained corrupt JSON, the code path emitted two record_cache_request metrics, first "hit" then "miss", for a single request. The cache.hit span attribute was also set to True and then overwritten to False.

Root cause: record_cache_request(result="hit") and span.set_attribute("cache.hit", True) were called before attempting to deserialize the cached entry. When deserialization failed (caught json.JSONDecodeError/KeyError), execution fell through to the MISS path, which recorded a second metric and flipped the span attribute.

Fix: Moved response deserialization before telemetry emission. If deserialization fails, only "miss" is recorded. If it succeeds, "hit" is recorded and CacheHitException is raised. the MISS path is never reached.

Bug: Negative TTL docstring promises ValueError that never fires

File: src/redis_fastapi/cache_backend.py:190-191

The docstring for CacheBackend.set() stated "Raises ValueError: If ttl is negative", but the code silently treated negative TTLs as "no expiry" (identical to ttl=0 or ttl=None):

if ttl_seconds is not None and ttl_seconds > 0:
    await self._redis.set(full_key, encoded, ex=ttl_seconds)
else:
    await self._redis.set(full_key, encoded)  # no expiry

Fix: Updated the docstring to accurately reflect the actual behavior: "None or a value of 0 or below means the key will not be automatically expired."

Bug: _store_cache_entry crashes with unhandled RuntimeError (500)

File: src/redis_fastapi/cache.py:683-703

When pending.redis is None, _store_cache_entry falls back to _get_pool_state(app).get_async_client(). If no lifespan has been registered, this raises RuntimeError. The surrounding exception handler only caught (RedisError, OSError), so the RuntimeError propagated unhandled, crashing the response with a 500 error.

Fix: Added RuntimeError to the caught exception types. The error is now logged as a warning and the response is delivered without caching, matching the graceful degradation behavior of the other Redis error paths.

Additional changes

• Added 25 edge-case regression tests in tests/unit/test_edge_cases.py covering all reported bugs, their fixes, and related edge cases (corrupt telemetry, negative TTL, middleware crash, key ambiguity, thundering herd, 304 re-caching, zero TTL, etc.).
• All 257 unit tests pass (original suite + new tests).

[emiliano-go]

More Bug Fixes

Bug: Timedelta TTL sub-second precision loss

Problem:
CacheBackend.set() converted timedelta TTLs to seconds via int(ttl.total_seconds()), discarding sub-second precision. TTL values like timedelta(milliseconds=500) would round down to 0 and become unbounded.

Fix:
When a timedelta is provided, compute milliseconds via int(ttl / timedelta(milliseconds=1)) and pass px= to redis.set() instead of ex=. The ex= path (seconds) is still used for integer TTLs.

Files: src/redis_fastapi/cache_backend.py

---

Bug: Warning for default TTL=0 (unbounded growth)

Problem:
Storing cache entries without an expiry (TTL=0, the default) silently persists them forever with no indication to the caller, risking unbounded Redis memory growth.

Fix:
Added logger.warning(...) calls in both _store_cache_entry() (cache.py) and CacheBackend.set() (cache_backend.py) when no TTL is set. The warning includes the key name for traceability.

Files: src/redis_fastapi/cache.py, src/redis_fastapi/cache_backend.py

---

Bug: Missing reset_settings() for testing

Problem:
get_settings() uses @lru_cache, caching the RedisSettings instance forever. Tests that mutate environment variables between cases have no way to force a fresh reload.

Fix:
Added reset_settings() that calls get_settings.cache_clear(). Exported from the public API via redis_fastapi.__init__ and __all__.

Files: src/redis_fastapi/config.py, src/redis_fastapi/__init__.py

---

Bug: cache_evict() with no args wipes ALL keys

Problem:
Calling cache_evict() with no eviction_group and no key_builder silently deletes every cache key under the global prefix. The old docstring warned about this but provided no guard.

Fix:
Added an early ValueError in cache_evict() when both eviction_group and key_builder are empty or missing. To intentionally wipe all keys, provide an explicit eviction_group. Updated tests to match the new contract.

Files: src/redis_fastapi/cache.py

---

Bug: Thundering herd / cache stampede protection

Problem:
When many concurrent requests arrive just as a cached entry is about to expire, all of them miss and recompute the value simultaneously, overwhelming the origin.

Fix:
Added stampede_protection: bool = False parameter to cache(). When enabled and the remaining TTL drops below 10% of the original TTL, a hit is probabilistically promoted to a miss with probability 1 - (remaining_ttl / threshold). Only a fraction of concurrent requests recompute, keeping origin load manageable.

Files: src/redis_fastapi/cache.py

---

Bug: Swallowed telemetry exceptions at DEBUG level

Problem:
OpenTelemetry metric helpers (record_cache_request, record_cache_eviction, record_cache_write, record_cache_latency) swallow exceptions at logger.debug level, making OTel errors invisible in production without explicit DEBUG logging enabled.

Fix:
Changed all 4 error handlers from logger.debug(...) to logger.warning(...) so OTel failures are visible by default.

Files: src/redis_fastapi/telemetry.py

[emiliano-go]

I don't know how strict you guys are with AI usage, but PR messages were written partially by AI as English is not my native language, and some of the bugs were found with AI.

[tishun]

I don't know how strict you guys are with AI usage, but PR messages were written partially by AI as English is not my native language, and some of the bugs were found with AI.

Hey @emiliano-go , no problem, the descriptions are short and concise enough, which is important. AI usage is encouraged, when used in a sustainable way.

I only have a problem with the amount of issues that are in the same PR - it is hard to comment on them when they are all grouped together; and it is harder to review. On the other side having one PR per issue is also a bit wasteful.

Can we perhaps split them up to - let's say - no more than 3 separate issues per PR?

[emiliano-go]

Sure, willco!

I'll leave it ready in 20ish min

[emiliano-go]

Per your feedback, the original set of changes has been split into 4 focused PRs:

• fix: telemetry recording on corrupt entries and exception log level #28: corrupt entry telemetry + telemetry exception log level
• fix: negative TTL docstring, timedelta precision, and zero TTL warning #29: negative TTL docstring, timedelta precision, zero TTL warning
• fix: key builder encoding, reset_settings, and cache_evict guard #30: builder encoding, reset_settings, cache_evict guard
• fix: RuntimeError fallback and stampede protection #31: fallback, stampede protection

[tishun]

Awesome! Can we close this PR? I think all the issues from it are already part of other PRs?

[emiliano-go]

Awesome! Can we close this PR? I think all the issues from it are already part of other PRs?

Yes, everything as been moved.

[tishun]

Thanks!