fix: guard best-effort cache writes and ref-count hub channel membership - #177
Merged
Conversation
H15/L1: CachingCommandDecorator now invalidates on CancellationToken.None and swallows plus logs invalidation failures, so a committed command is never turned into a failure by a cache blip or a client disconnect. M28: CachingQueryDecorator guards the post-read cache populate, so a cache fault after a successful read no longer fails the query. Only the populate is guarded; the read paths and real cancellation still propagate. H13: NotificationHubService ref-counts channel membership instead of using set semantics, so the first leaver no longer cuts the channel off for every other subscriber in the circuit. Public API surface is unchanged. Both caching decorators take an ILogger<TSelf> third constructor argument, matching the ValidatingCommandDecorator idiom. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ators The required ILogger<TSelf> parameter added in the previous commit broke the consumer source build: MMCA.Helpdesk hand-constructs both caching decorators in TicketCacheInvalidationTests, and its own CI builds against the released v1.133.0 packages where the 3-argument constructor does not exist, so a required-only parameter deadlocks the two repos. Both decorators keep the 3-argument primary constructor and regain a public 2-argument overload chaining to it with NullLogger<TSelf>.Instance, so both call shapes compile. Verified that container resolution still selects the logger-bearing constructor, so production keeps emitting cache-failure warnings rather than silently binding the no-op logger. CachingDecoratorConstructorSelectionTests pins that behavior for both decorators and pins the 2-argument call shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.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
1 participant
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.
Fixes four findings from the 2026-07-30 bug hunt (Common-side items): H15, L1, M28, H13.
Changes
RemoveByPrefixAsyncinvalidation is now wrapped in a guarded try/catch that logs a warning and swallows. A best-effort cache invalidation must never turn a committed command into a failure response (which invites a duplicate client retry).CancellationToken.Noneinstead of the request token, so a client disconnect right after commit no longer cancels the invalidation and leaves stale cache for everyone else.SetAsync) is guarded the same way (OCE still propagates via an exception filter), so a transient cache-store blip no longer fails a query whose handler already produced a valid result.ChannelReferenceCounter. The server-side JoinChannel fires only on the 0-to-1 transition and LeaveChannel only on 1-to-0, honoring the documented multicast contract (an always-mounted listener and a page can observe the same channel; navigating away from the page no longer silently kills the listener's subscription for the rest of the circuit). Reconnect replays the counter snapshot. Public API surface unchanged.Both decorators now take a required
ILogger<TSelf>constructor parameter (repo idiom, DI-resolved via Scrutor decoration; no production call-site changes). This is a constructor change on public types, so the next release is a minor bump.Tests
8 new tests including two regression locks (
HandleAsync_SuccessfulCacheInvalidatingCommand_InvalidatesWithNonCancellableToken,Release_WithOutstandingRefs_DoesNotSignalServerLeave); 13 existing construction sites moved toNullLogger<...>.Instance. Full solution: 2523 passed, 0 failed.🤖 Generated with Claude Code