Repository navigation
fix: ignore change-feed lease-lost errors in event subscriptions - #61
Open
rickykaare wants to merge 1 commit into
Open
rickykaare wants to merge 1 commit into
rickykaare wants to merge 1 commit into
Conversation
Lease ownership moving to another instance (scale-out, rollout) is expected and the new owner resumes from the last checkpoint. These errors are no longer forwarded to IEventSubscriptionExceptionHandler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The change is localized, has focused regression coverage, and has no unresolved blocking issues.
Review effort: Balanced
Findings: None
What changed in this PR
Reduces deployment-related error noise by filtering expected Cosmos DB lease-loss notifications in event subscriptions.
Changes:
- Suppresses lease-loss errors while preserving user-processing error reporting.
- Adds five focused regression tests.
- Documents the behavior under Unreleased.
| File | Description |
|---|---|
| test/Chronicles.Tests/EventStore/Internal/EventDocumentProcessorTests.cs | Tests error filtering and forwarding. |
| src/Chronicles/EventStore/Internal/EventDocumentProcessor.cs | Filters expected lease-loss notifications. |
| CHANGELOG.md | Documents the subscription behavior change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
Problem
During rollouts and scale-out, Cosmos change-feed leases are rebalanced between instances. The SDK reports this via the change-feed error notification, either as a
CosmosException412 ("lease token was taken over by owner ...") or as an exception wrappingLeaseLostException("Lease was lost due to load balancing ..."). Chronicles forwarded these toIEventSubscriptionExceptionHandler, and consumers log them as errors, which creates App Insights noise on every deployment (seen in session-processor and cdr-processor; six Teal services share the same handler pattern).Change
EventDocumentProcessor.ErrorAsyncno longer forwards lease-lost errors (a 412CosmosExceptionorLeaseLostExceptionanywhere in the exception chain). The new lease owner resumes from the last checkpoint, so nothing is lost.ChangeFeedProcessorUserException(exceptions from our own processing code) is always forwarded, even if it wraps a 412.IDocumentProcessorimplementations are unchanged.Tests
5 new tests in
EventDocumentProcessorTests(generic error forwarded, non-lease Cosmos error forwarded, 412 ignored, wrapped lease-lost ignored, user exception with 412 forwarded). All 222 tests pass, and the two "ignore" tests fail without the fix.