#49 Address unserializable entries

[matthewdevenny]

Closes #49.

Follow-ups from the review of #48 (compact binary cache envelope): define the runtime behavior for present-but-undeserializable entries, harden the deserializer against corrupt sliding ticks, and — surfaced during review of this PR — bound expirations to what the NATS TTL wire format can actually encode.

1. Undeserializable entries → logged cache miss

When a present entry can't be deserialized (a legacy JSON envelope from a pre-binary release, or genuine corruption), GetAndRefreshAsync now logs at Debug (EventId 102 UndeserializableEntry) and returns a cache miss. It deliberately does not evict or throw:

• No eviction — an older node must not delete an entry written in a newer format during a rolling deploy; a node never deletes bytes it can't read, so upgrades stay safe.
• No throw — a cache should degrade to a miss, not fail the caller's operation. Throwing would also turn the JSON→binary migration into per-read failures for session-state / direct IDistributedCache consumers.
• Debug, not Warning — avoids flooding logs during the JSON→binary migration, when every legacy key transiently lands here.

The entry self-heals when the key is next written (Set overwrites unconditionally); TTL'd entries are reaped by NATS. Documented in code and in a new README "Cache Entry Format and Upgrades" section.

2. Sliding-ticks validation

CacheEntryBinarySerializer.Deserialize now fails closed on non-positive or out-of-range sliding ticks, consistent with the existing absolute-ticks bounds check.

3. Expiration ceiling = NATS TTL encoding limit (review-driven)

NATS message TTLs are encoded as (int)ttl.TotalSeconds (NatsExtensions.ToTtlString), which overflows above int.MaxValue seconds (~68 years) and emits an invalid header. A shared MaxTtlTicks ceiling is now enforced end-to-end so any accepted value round-trips through both the serializer and the TTL header:

• GetTtl throws for sliding and absolute expirations beyond the ceiling (write path).
• Deserialize fails closed on stored sliding ticks beyond it (read path).

This keeps write and read symmetric: nothing that can be Set() reads back as an undeserializable miss, and no accepted expiration overflows the TTL header.

Range-check exceptions state the limit (e.g. "The maximum is 24855.03:14:07."), and the MaxValue sentinels — DateTimeOffset.MaxValue (absolute) and TimeSpan.MaxValue (sliding/relative) — are treated as "never expire" (no TTL) rather than rejected, matching the common "cache forever" idiom.

Tests

• Unit — sliding out-of-range / non-positive / truncated + inclusive boundary; absolute and relative "too far" throws; far-future-absolute paired with a small sliding is not over-rejected (effective TTL is the sliding minimum); MaxValue sentinels normalize to no expiration in GetTtl/CreateCacheEntry.
• Integration — legacy entry reads as a miss + Debug log; entry is left in place (verified via a direct KV-store read) and self-heals on the next write.

All green after rebasing on main: 95 unit + 65 integration, 0 build warnings.

[Copilot]

Pull request overview

This PR addresses follow-ups from issue #49 by defining and documenting the runtime behavior for present-but-undeserializable cache entries, and by hardening the binary envelope deserializer against corrupt sliding-expiration tick values.

Changes:

• Add bounds validation for sliding-expiration ticks during binary deserialization, with unit tests covering corrupt/truncated cases.
• Treat undeserializable entries as cache misses, emit a Debug log event, and document the intended upgrade/migration behavior.
• Add an integration test covering the “legacy JSON entry reads as miss + debug log” behavior and the “self-heals on next write” path.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/UnitTests/Serialization/CacheEntryBinarySerializerTests.cs	Adds unit coverage for invalid/truncated sliding ticks and validates the accepted upper bound.
test/IntegrationTests/Cache/UndeserializableEntryTests.cs	Adds integration coverage for legacy/undeserializable entries being treated as misses and logged.
src/NatsDistributedCache/NatsCache.Log.cs	Introduces a new Debug log event for undeserializable entries.
src/NatsDistributedCache/NatsCache.cs	Logs and returns a miss when a present entry cannot be deserialized.
src/NatsDistributedCache/CacheEntryBinarySerializer.cs	Adds sliding-ticks validation to fail closed on corrupt values.
README.md	Documents the cache entry format and upgrade/migration behavior for legacy/corrupt entries.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

[mtmk]

LGTM with optional comments:

Should we accept DateTimeOffset.MaxValue or Timeout.Infinite to mean no TTL. as it is it would throw.

[matthewdevenny]

Re: accepting DateTimeOffset.MaxValue / Timeout.Infinite as no-TTL — implemented in 3300660. DateTimeOffset.MaxValue (absolute) and TimeSpan.MaxValue (sliding/relative) now normalize to no expiration ("cache forever") instead of throwing as out-of-range, which lines up with ToTtlString already mapping TimeSpan.MaxValue → "never".

I skipped Timeout.InfiniteTimeSpan (-1 ms) since it is negative and would collide with the existing "must be positive" validation — happy to add it as well if you would prefer it supported.

[mtmk]

sounds good thanks. I think we can leave Timeout.InfiniteTimeSpan out since it doesn't make sense here that much and i believe it's not used often if ever in caching libraries (just guessing).