Repository navigation
Conversation
Sync the new read-write-concern write-concern-errors unified test fixtures and add the mapReduce, and GridFS deleteByName operations to the unified test runner. The fixtures exposed a gap in MapReduce::Result#validate!: it only checked for the presence of results in the reply, so a writeConcernError on an ok:1 reply was silently dropped. The parser already builds an error message from a writeConcernError, so validate! now raises when the parser produced one. GridFS rename operations are skipped because the Ruby driver does not implement GridFS file rename, and the client-level bulkWrite operation is skipped as unsupported.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Map-reduce replies containing both an output result and a write-concern error raise during validation but still report success, causing command tracing to omit the error code and exception.
1 open issue
What changed in this PR
This PR addresses RUBY-3986 by tightening map-reduce error handling and adding coverage for mandatory writeConcernError reporting across the Ruby driver.
Changes:
- Raise map-reduce failures when the response parser reports an error.
- Add unified write-concern error fixtures across supported operation types.
- Extend the unified runner with map-reduce and GridFS delete-by-name operations, while skipping unsupported GridFS rename cases.
| File | Description |
|---|---|
spec/spec_tests/read_write_concern_unified_spec.rb |
Loads the new suite and skips unsupported rename cases. |
spec/spec_tests/data/read_write_concern/write_concern_errors/mapReduce.yml |
Covers map-reduce output errors. |
spec/spec_tests/data/read_write_concern/write_concern_errors/index-management.yml |
Covers index creation and deletion errors. |
spec/spec_tests/data/read_write_concern/write_concern_errors/gridfs.yml |
Covers GridFS write errors. |
spec/spec_tests/data/read_write_concern/write_concern_errors/findAndModify.yml |
Covers find-and-modify write errors. |
spec/spec_tests/data/read_write_concern/write_concern_errors/db-coll-management.yml |
Covers database and collection management errors. |
spec/spec_tests/data/read_write_concern/write_concern_errors/crud.yml |
Covers CRUD write errors. |
spec/spec_tests/data/read_write_concern/write_concern_errors/client-side-encryption.yml |
Covers encryption key-management write errors. |
spec/spec_tests/data/read_write_concern/write_concern_errors/client-bulkWrite.yml |
Covers client-level bulk-write errors. |
spec/spec_tests/data/read_write_concern/write_concern_errors/bulkWrite.yml |
Covers collection-level bulk-write errors. |
spec/spec_tests/data/read_write_concern/write_concern_errors/aggregate.yml |
Covers $out and $merge write errors. |
spec/runners/unified/grid_fs_operations.rb |
Adds delete-by-name execution support. |
spec/runners/unified/crud_operations.rb |
Adds map-reduce execution support. |
lib/mongo/operation/map_reduce/result.rb |
Checks parser errors during result validation. |
Review effort: Balanced
A map-reduce reply can contain both a result and a writeConcernError. validate! raises in that case, but successful? still returned true, so the OpenTelemetry command tracer skipped recording the error code and exception on the span. Align successful? with the base Result implementation by also checking the parser message.
This branch has not been deployed
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.
Implements RUBY-3986, the Ruby driver portion of DRIVERS-3684, which changes the Read/Write Concern spec requirement to report a
writeConcernErrorfrom a SHOULD to a MUST for all driver methods that send a command which may include aWriteConcern.