PYTHON-5875 - Wrap SSL handshake errors before labeling with overload#2866
PYTHON-5875 - Wrap SSL handshake errors before labeling with overload#2866NoahStapp wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adjusts connection-establishment error handling in the connection pool so that certain connection failures are first wrapped into AutoReconnect/NetworkTimeout and then processed for SystemOverloadedError labeling, ensuring the label is applied to the exception that is ultimately propagated to callers.
Changes:
- In both sync and async pools, wrap
(IOError, OSError, *SSLErrors)via_raise_connection_failure(...)before calling_handle_connection_error(...). - Ensure
_handle_connection_error(...)runs on the wrappedAutoReconnect/NetworkTimeout(so any overload labels are attached to the raised exception).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pymongo/synchronous/pool.py | Updates connect() error path to label the wrapped connection failure exception. |
| pymongo/asynchronous/pool.py | Mirrors the same connect() error-path change for async pooling. |
| if isinstance(error, (IOError, OSError, *SSLErrors)): | ||
| details = _get_timeout_details(self.opts) | ||
| _raise_connection_failure(self.address, error, timeout_details=details) | ||
| # Wrap to AutoReconnect/NetworkTimeout BEFORE labeling so the | ||
| # SystemOverloadedError label lands on the propagated exception. | ||
| try: | ||
| _raise_connection_failure(self.address, error, timeout_details=details) | ||
| except (AutoReconnect, NetworkTimeout) as wrapped: | ||
| self._handle_connection_error(wrapped) | ||
| raise | ||
| self._handle_connection_error(error) |
There was a problem hiding this comment.
Per the relevant part of the CMAP spec, the only types of establishment errors that MUST be excluded from having the backpressure labels applied are the following:
- DNS lookup failures: excluded by the
socket.gaierrorcheck in_handle_connection_error. - Non-I/O TLS errors (certificate validation, hostname mismatch failures): excluded the
_CertificateError, SSLErrorscheck in `_handle_connection_errors. - SOCKS5 errors: PyMongo does not currently support SOCKS5.
This change then correctly addresses a bug in the driver where errors that MUST be labeled as possible overload errors are not.
PYTHON-5875
Changes in this PR
Brings the driver into backpressure spec alignment for SSL handshake errors caused by the connection establishment rate limiter. The raised error by such a rejection can be a raw SSLError or OSError that would be rejected for the correct labeling, causing the test failure described in the ticket. This fixes that by correctly wrapping the raised error first so that the proper labeling can occur.
Test Plan
Existing tests.
Checklist
Checklist for Author
[ ] Did you update the changelog (if necessary)?[ ] Is any followup work tracked in a JIRA ticket? If so, add link(s).Checklist for Reviewer