Skip to content

Conversation

@ntark
Copy link
Contributor

@ntark ntark commented Oct 21, 2025

Chose to compare by CodeName for claity vs ex.Code == 85.
Change is Backwards-Compatible.

Closes #98

@coderabbitai
Copy link

coderabbitai bot commented Oct 21, 2025

Walkthrough

Exception handling in MongoDB helper methods updated to match exceptions using CodeName property instead of substring matching in ErrorMessage, improving reliability across MongoDB versions 4.4 through 7.0+.

Changes

Cohort / File(s) Summary
MongoDB exception handler condition refinement
src/Serilog.Sinks.MongoDB/Helpers/MongoDbHelpers.cs
Updated two catch blocks to check CodeName property instead of substring matching in ErrorMessage: VerifyCollectionExists checks CodeName == "NamespaceExists", and VerifyExpireTTLSetup checks CodeName == "IndexOptionsConflict".

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A rabbit hops through MongoDB's land,
Finding CodeNames is so grand!
No more substring searches in the night,
Exception handling, shiny and bright! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix: expire index already exists not triggering on v7 (#98)" accurately describes the primary change: addressing a compatibility issue where the expire index exception detection was failing on MongoDB 7.0.23. The title is specific, clear, and directly reflects the core problem being solved—using CodeName-based exception matching instead of error message string matching. It is concise and contains no unnecessary noise or vague terminology.
Linked Issues Check ✅ Passed The PR directly addresses all requirements from issue #98. The primary objective—fixing the "expire index already exists" catch block in VerifyExpireTTLSetup to use CodeName == "IndexOptionsConflict" instead of string matching—is implemented. Additionally, the issue noted that the same problem could surface in the "collection already exists" catch block, and the PR proactively applies the same fix to VerifyCollectionExists by using CodeName == "NamespaceExists". Both changes replace error message string matching with CodeName comparison for improved cross-version compatibility, exactly as recommended in the issue.
Out of Scope Changes Check ✅ Passed All changes in the PR are directly within scope of issue #98. The modifications are limited to replacing exception filter conditions in two catch blocks—VerifyCollectionExists and VerifyExpireTTLSetup—changing from ErrorMessage string matching to CodeName-based comparisons. No changes are made to function signatures, return values, public APIs, or overall behavior; the error handling logic remains identical in intent. These changes precisely address the MongoDB version compatibility issue described in the linked issue.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0e5402b and 0c95902.

📒 Files selected for processing (1)
  • src/Serilog.Sinks.MongoDB/Helpers/MongoDbHelpers.cs (2 hunks)
🔇 Additional comments (2)
src/Serilog.Sinks.MongoDB/Helpers/MongoDbHelpers.cs (2)

61-61: Excellent improvement for cross-version compatibility.

Using CodeName == "NamespaceExists" is more robust than substring matching on error messages, which can vary across MongoDB versions and locales. This correctly handles the race condition when a collection is created between the existence check and creation attempt.


90-90: Code changes are correct and verified against MongoDB documentation.

The CodeName values "NamespaceExists" and "IndexOptionsConflict" are valid and occur when an existing index has different options than the one being created. The code correctly implements the recommended fix of dropping the conflicting index then recreating it with the desired options.

Using CodeName matching instead of error message substring comparison is version-agnostic and properly addresses MongoDB 7.0.23 compatibility (issue #98).


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Jaben
Copy link
Member

Jaben commented Oct 24, 2025

@ntark Thank you for the PR with the fix to this issue. LGTM! Merging.

@Jaben Jaben merged commit e2fc22d into ChangemakerStudios:dev Oct 24, 2025
2 checks passed
Jaben added a commit that referenced this pull request Oct 24, 2025
Add unit tests for PR #99 to verify MongoDB error code handling works
correctly across different MongoDB versions. Tests validate:

- VerifyCollectionExists handles NamespaceExists (error code 48)
- VerifyExpireTTLSetup handles IndexOptionsConflict (error code 85)
- Race condition handling when collections are created concurrently
- TTL index creation, updates, and removal scenarios
- Error code validation to ensure MongoDB driver compatibility

The tests use the actual MongoDB driver behavior to confirm that
CodeName-based exception filtering is more reliable than string
matching and works across MongoDB versions 4.x through 7.x.

Changes:
- Add NSubstitute 5.3.0 for mocking support
- Add MongoDbHelperErrorHandlingTests.cs with 12 comprehensive tests
- Update GlobalUsings.cs to include NSubstitute

All 27 tests pass (15 existing + 12 new).
Jaben added a commit that referenced this pull request Oct 24, 2025
#100)

* test: add comprehensive unit tests for MongoDB error handling

Add unit tests for PR #99 to verify MongoDB error code handling works
correctly across different MongoDB versions. Tests validate:

- VerifyCollectionExists handles NamespaceExists (error code 48)
- VerifyExpireTTLSetup handles IndexOptionsConflict (error code 85)
- Race condition handling when collections are created concurrently
- TTL index creation, updates, and removal scenarios
- Error code validation to ensure MongoDB driver compatibility

The tests use the actual MongoDB driver behavior to confirm that
CodeName-based exception filtering is more reliable than string
matching and works across MongoDB versions 4.x through 7.x.

Changes:
- Add NSubstitute 5.3.0 for mocking support
- Add MongoDbHelperErrorHandlingTests.cs with 12 comprehensive tests
- Update GlobalUsings.cs to include NSubstitute

All 27 tests pass (15 existing + 12 new).

* ci: skip NuGet package generation during test runs

Update the GitHub Actions workflow to add `-p:GeneratePackageOnBuild=false`
to the dotnet test command. This prevents unnecessary package creation during
CI test runs, improving build performance.

The project has `<GeneratePackageOnBuild>true</GeneratePackageOnBuild>` set,
which causes packages to be created on every build, including test runs.
Packages are still properly created during the "Pack" step for releases.

Benefits:
- Faster CI test execution
- Cleaner build output
- Packages only created when actually needed (Release builds)

* test: address CodeRabbit review suggestions

Apply all CodeRabbit nitpick suggestions to improve test quality:

1. **Test Isolation**: Add [NonParallelizable] attribute to prevent
   concurrent test execution issues when using shared database name

2. **Dependency Management**: Add PrivateAssets="all" to NSubstitute
   package reference to prevent transitive dependency exposure

3. **Cleanup Optimization**: Remove redundant DropDatabase calls within
   test methods since cleanup is handled by [TearDown]

4. **Cross-Version Compatibility**: Fix TTL assertion type handling to
   support MongoDB's Int64/Double serialization variations across server
   versions by using Convert.ToInt64()

5. **Race Condition Testing**: Improve authenticity of race condition
   test by using Parallel.Invoke() to force genuine concurrent collection
   creation attempts, exercising the actual NamespaceExists error path

All 12 tests continue to pass. These changes improve test robustness,
reduce runtime overhead, and ensure compatibility across MongoDB versions.
Jaben added a commit that referenced this pull request Oct 26, 2025
Update version and release notes for v7.2.0 release:

Changes:
- MongoDB v7.x compatibility fix using error codes (#98, #99)
  Thanks to @ntark for the PR!
- Comprehensive unit tests for error handling (#100)
- CI workflow optimization

Updated files:
- CHANGES.md: Added v7.2.0 release notes with contributor attribution
- README.md: Updated "What's New" section and added link to CHANGES.md
- Serilog.Sinks.MongoDB.csproj: Bumped version to 7.2.0, updated
  copyright year to 2025, and updated PackageReleaseNotes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MongoDB - 7.0.23 expire index already exists catch block not triggering

2 participants