RUBY-3602 Include server address in OperationFailure and BulkWriteError messages#3027
RUBY-3602 Include server address in OperationFailure and BulkWriteError messages#3027comandeo-mongo merged 12 commits intomongodb:masterfrom
Conversation
Task 5 extracted the seed eagerly via connection_description&.address&.seed,
which blew up in the existing operation_failure_spec #labels context where
the test Result is built from Mongo::Server::Description.new('') — address
is a String, not a Mongo::Address. Pass connection_description through and
let normalize_server_address handle the type dispatch. Harden the
Description branch to tolerate a non-Address address instead of calling
.seed blindly.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in configuration flag to include the originating server address in OperationFailure and BulkWriteError messages, improving diagnosability in multi-server/topology scenarios.
Changes:
- Introduces
Mongo::Config.include_server_address_in_errors(defaultfalse) and delegates it viaMongo.include_server_address_in_errors. - Extends
Mongo::Error::OperationFailureandMongo::Error::BulkWriteErrorto accept server address metadata and conditionally suffix error messages with(on host:port[, ...]). - Accumulates unique server addresses across split bulk batches via
Mongo::BulkWrite::ResultCombinerand propagates them into raisedBulkWriteErrors.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
lib/mongo/config.rb |
Adds the new opt-in config flag. |
lib/mongo.rb |
Delegates the new config option via Mongo. |
lib/mongo/operation/result.rb |
Plumbs connection description into OperationFailure as server_address. |
lib/mongo/error/operation_failure.rb |
Stores/normalizes server_address and conditionally appends it to messages. |
lib/mongo/bulk_write/result_combiner.rb |
Collects server address seeds while combining results. |
lib/mongo/bulk_write/result.rb |
Carries server addresses into BulkWriteError on validation failure. |
lib/mongo/error/bulk_write_error.rb |
Stores/normalizes server addresses and conditionally appends them to messages. |
spec/mongo/config_spec.rb |
Adds tests for the new config option and delegation. |
spec/mongo/error/operation_failure_spec.rb |
Adds unit tests for address storage/normalization and message rendering. |
spec/mongo/error/bulk_write_error_spec.rb |
Adds unit tests for address normalization and message rendering. |
spec/mongo/bulk_write/result_combiner_spec.rb |
Adds unit tests for server address accumulation. |
spec/integration/operation_failure_message_spec.rb |
Adds integration coverage for message suffix + exposed address. |
spec/integration/bulk_write_error_message_spec.rb |
Adds integration coverage for bulk error suffix + collected addresses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def initialize(result, server_addresses: nil) | ||
| @result = result | ||
| @server_addresses = normalize_server_addresses(server_addresses) | ||
|
|
There was a problem hiding this comment.
Good catch. Fixed in 6535216 — normalize_server_addresses now ends with .uniq, so the reader matches the documented contract even for direct callers, and the redundant .uniq inside build_message is gone.
| def initialize(results, acknowledged, server_addresses = []) | ||
| @results = results | ||
| @acknowledged = acknowledged | ||
| @server_addresses = server_addresses |
There was a problem hiding this comment.
Fixed in 6535216. The initializer now stores Array(server_addresses).compact.uniq, so the public reader honors the "deduplicated" docstring regardless of what callers pass in.
| seed = result.connection_description&.address&.seed | ||
| @server_addresses << seed if seed && !@server_addresses.include?(seed) |
There was a problem hiding this comment.
Leaving this as-is. The complexity here is O(n²) in the number of distinct server addresses, not the number of batches — dedup keeps n tiny. A bulk write targets a primary in a replset or a handful of shards (typically n ≤ 10), so even thousands of combined batches do only a few thousand comparisons total. Switching to a Set would add a second data structure without a measurable win; happy to revisit if profiling ever turns this up.
Both BulkWriteError and BulkWrite::Result document their server_addresses reader as a deduplicated list. Enforce that at the normalization/ initializer boundary so direct callers match the contract, and drop the redundant .uniq inside BulkWriteError#build_message.
Summary
Mongo::Config.include_server_address_in_errorsflag (defaultfalse). Whentrue,Mongo::Error::OperationFailureandMongo::Error::BulkWriteErrormessages are suffixed with(on host:port)so users can identify which server produced the error.#messageand#to_srender the suffix, since it's baked into the string passed tosuperat construction.ResultCombineraccumulates unique addresses across split bulk batches so multi-server bulks produce(on h1:27017, h2:27017).Test Plan
bundle exec rspec spec/mongo/config_spec.rb spec/mongo/error spec/mongo/bulk_write spec/integration/operation_failure_message_spec.rb spec/integration/bulk_write_error_message_spec.rb— 180 examples, 0 failures against a local replica set atmongodb://localhost:27017,27018,27019/.Notes
Operation::ResponseHandling#add_server_diagnosticsstill addsadd_note("on ..."). When the new flag is on,#to_soutput will include the host twice (in-message suffix + note). Deduplicating is deferred to a follow-up ticket per the design spec.Mongo::Configpattern (broken_view_options,validate_update_replace, etc.).Jira: https://jira.mongodb.org/browse/RUBY-3602