RUBY-3601 Wrap error when mongocrypt_kms_ctx_fail returns false#3082
Open
comandeo-mongo wants to merge 2 commits into
Open
RUBY-3601 Wrap error when mongocrypt_kms_ctx_fail returns false#3082comandeo-mongo wants to merge 2 commits into
comandeo-mongo wants to merge 2 commits into
Conversation
When KMS retries are exhausted, wrap the status message from mongocrypt_kms_ctx_status (which indicates the retry occurred) with the error from the last attempt, instead of re-raising only the last network error. See MONGOCRYPT-752.
Contributor
There was a problem hiding this comment.
Pull request overview
Improves client-side encryption KMS retry error reporting by preserving the libmongocrypt KMS context status message when retries are exhausted, and wrapping it together with the last network failure message to provide more actionable diagnostics.
Changes:
- Added
Mongo::Crypt::Binding.kms_ctx_statusto retrieve a KMS contextStatuswithout raising, and refactoredcheck_kms_ctx_statusto use it. - Updated
Mongo::Crypt::Context#feed_kmsto raise a wrappedMongo::Error::KmsErroron retry exhaustion viaraise_kms_retry_error. - Tightened the KMS retry prose spec to assert the new wrapped message includes
last attempt failed with:.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/mongo/crypt/binding.rb | Exposes KMS ctx status retrieval without raising and reuses it in existing status-check helper. |
| lib/mongo/crypt/context.rb | Wraps retry-exhaustion status message with the last network error for improved KMS failure diagnostics. |
| spec/integration/client_side_encryption/kms_retry_prose_spec.rb | Verifies the new wrapped error message format. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
OpenSSL surfaces the missing-client-cert failure as a 'tlsv13 alert certificate required' message (space, not underscore) rather than a socket reset in some environments. Broaden the regex to accept it so the test is robust to how the peer rejects the handshake.
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.
When a KMS request exhausts its retries,
mongocrypt_kms_ctx_failreturnsfalse. Previously the driver re-raised only the last network error, discarding
the status message from
mongocrypt_ctx_kms_statusthat indicates a retryoccurred (e.g. "KMS request failed after 3 retries due to a network error").
This wraps the status message with the last attempt's error so users get
actionable information about both the retry exhaustion and the underlying
cause. Mirrors the approach suggested in MONGOCRYPT-752 and used by the Python
and C drivers.
Changes
lib/mongo/crypt/binding.rb: addBinding.kms_ctx_statusto read the KMScontext status into a
Statuswithout raising;check_kms_ctx_statusnowreuses it.
lib/mongo/crypt/context.rb: on retry exhaustion,feed_kmsraises aKmsErrorcombining the status message with the last error, via the newraise_kms_retry_errorhelper.spec/integration/client_side_encryption/kms_retry_prose_spec.rb: assert thewrapped message includes "last attempt failed with:".
Test plan
bundle exec rubocop lib/mongo/crypt/binding.rb lib/mongo/crypt/context.rb spec/integration/client_side_encryption/kms_retry_prose_spec.rb— no offensesbundle exec rspec spec/integration/client_side_encryption/kms_retry_prose_spec.rbagainst local cluster + KMS failpoint mock — 3 AWS examples pass (GCP/Azure pending as before)Jira: https://jira.mongodb.org/browse/RUBY-3601