update error behavior and add tests for server description changed

Author: blink1073

pymongo/asynchronous/pool.py

@@ -1022,12 +1022,9 @@ async def remove_stale_sockets(self, reference_generation: int) -> None:
                     self.requests -= 1
                     self.size_cond.notify()
 
-    def _handle_connection_error(self, error: BaseException, phase: str, conn_id: int) -> None:
+    def _handle_connection_error(self, error: BaseException) -> None:
         # Handle system overload condition for non-sdam pools.
-        # Look for an AutoReconnect error raised from a ConnectionResetError with
-        # errno == errno.ECONNRESET or raised from an OSError that we've created due to
-        # a closed connection.
-        # If found, add error labels.
+        # Look for errors of type AutoReconnect and add error labels if appropriate.
         if self.is_sdam or type(error) != AutoReconnect:
             return
         error._add_error_label("SystemOverloadedError")
@@ -1062,13 +1059,8 @@ async def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> A
                 driverConnectionId=conn_id,
             )
 
-        # Pass a context to determine if we successfully create a configured socket.
-        context = dict(has_created_socket=False)
-
         try:
-            networking_interface = await _configured_protocol_interface(
-                self.address, self.opts, context=context
-            )
+            networking_interface = await _configured_protocol_interface(self.address, self.opts)
         # Catch KeyboardInterrupt, CancelledError, etc. and cleanup.
         except BaseException as error:
             async with self.lock:
@@ -1089,8 +1081,7 @@ async def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> A
                     reason=_verbose_connection_error_reason(ConnectionClosedReason.ERROR),
                     error=ConnectionClosedReason.ERROR,
                 )
-            if context["has_created_socket"]:
-                self._handle_connection_error(error, "handshake", conn_id)
+            self._handle_connection_error(error)
             if isinstance(error, (IOError, OSError, *SSLErrors)):
                 details = _get_timeout_details(self.opts)
                 _raise_connection_failure(self.address, error, timeout_details=details)
@@ -1117,7 +1108,7 @@ async def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> A
             async with self.lock:
                 self.active_contexts.discard(conn.cancel_context)
             if not completed_hello:
-                self._handle_connection_error(e, "hello", conn_id)
+                self._handle_connection_error(e)
             await conn.close_conn(ConnectionClosedReason.ERROR)
             raise
 

pymongo/pool_shared.py

@@ -250,7 +250,6 @@ async def _configured_protocol_interface(
     address: _Address,
     options: PoolOptions,
     protocol_kls: type[PyMongoBaseProtocol] = PyMongoProtocol,
-    context: dict[str, bool] | None = None,
 ) -> AsyncNetworkingInterface:
     """Given (host, port) and PoolOptions, return a configured AsyncNetworkingInterface.
 
@@ -262,10 +261,6 @@ async def _configured_protocol_interface(
     ssl_context = options._ssl_context
     timeout = options.socket_timeout
 
-    # Signal that we have created the socket successfully.
-    if context:
-        context["has_created_socket"] = True
-
     if ssl_context is None:
         return AsyncNetworkingInterface(
             await asyncio.get_running_loop().create_connection(
@@ -379,7 +374,7 @@ def _create_connection(address: _Address, options: PoolOptions) -> socket.socket
 
 
 def _configured_socket_interface(
-    address: _Address, options: PoolOptions, *args: Any, context: dict[str, bool] | None = None
+    address: _Address, options: PoolOptions, *args: Any
 ) -> NetworkingInterface:
     """Given (host, port) and PoolOptions, return a NetworkingInterface wrapping a configured socket.
 
@@ -390,10 +385,6 @@ def _configured_socket_interface(
     sock = _create_connection(address, options)
     ssl_context = options._ssl_context
 
-    # Signal that we have created the socket successfully.
-    if context:
-        context["has_created_socket"] = True
-
     if ssl_context is None:
         sock.settimeout(options.socket_timeout)
         return NetworkingInterface(sock)

pymongo/synchronous/pool.py

@@ -1018,12 +1018,9 @@ def remove_stale_sockets(self, reference_generation: int) -> None:
                     self.requests -= 1
                     self.size_cond.notify()
 
-    def _handle_connection_error(self, error: BaseException, phase: str, conn_id: int) -> None:
+    def _handle_connection_error(self, error: BaseException) -> None:
         # Handle system overload condition for non-sdam pools.
-        # Look for an AutoReconnect error raised from a ConnectionResetError with
-        # errno == errno.ECONNRESET or raised from an OSError that we've created due to
-        # a closed connection.
-        # If found, add error labels.
+        # Look for errors of type AutoReconnect and add error labels if appropriate.
         if self.is_sdam or type(error) != AutoReconnect:
             return
         error._add_error_label("SystemOverloadedError")
@@ -1058,13 +1055,8 @@ def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> Connect
                 driverConnectionId=conn_id,
             )
 
-        # Pass a context to determine if we successfully create a configured socket.
-        context = dict(has_created_socket=False)
-
         try:
-            networking_interface = _configured_socket_interface(
-                self.address, self.opts, context=context
-            )
+            networking_interface = _configured_socket_interface(self.address, self.opts)
         # Catch KeyboardInterrupt, CancelledError, etc. and cleanup.
         except BaseException as error:
             with self.lock:
@@ -1085,8 +1077,7 @@ def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> Connect
                     reason=_verbose_connection_error_reason(ConnectionClosedReason.ERROR),
                     error=ConnectionClosedReason.ERROR,
                 )
-            if context["has_created_socket"]:
-                self._handle_connection_error(error, "handshake", conn_id)
+            self._handle_connection_error(error)
             if isinstance(error, (IOError, OSError, *SSLErrors)):
                 details = _get_timeout_details(self.opts)
                 _raise_connection_failure(self.address, error, timeout_details=details)
@@ -1113,7 +1104,7 @@ def connect(self, handler: Optional[_MongoClientErrorHandler] = None) -> Connect
             with self.lock:
                 self.active_contexts.discard(conn.cancel_context)
             if not completed_hello:
-                self._handle_connection_error(e, "hello", conn_id)
+                self._handle_connection_error(e)
             conn.close_conn(ConnectionClosedReason.ERROR)
             raise
 

test/discovery_and_monitoring/unified/backpressure-network-error-fail.json

@@ -49,6 +49,7 @@
                   "useMultipleMongoses": false,
                   "observeEvents": [
                     "serverHeartbeatSucceededEvent",
+                    "serverDescriptionChangedEvent",
                     "poolReadyEvent",
                     "poolClearedEvent"
                   ],
@@ -89,6 +90,17 @@
             "count": 1
           }
         },
+        {
+          "name": "waitForEvent",
+          "object": "testRunner",
+          "arguments": {
+            "client": "client",
+            "event": {
+              "serverDescriptionChangedEvent": {}
+            },
+            "count": 1
+          }
+        },
         {
           "name": "failPoint",
           "object": "testRunner",
@@ -139,6 +151,19 @@
               "poolReadyEvent": {}
             }
           ]
+        },
+        {
+          "client": "client",
+          "eventType": "sdam",
+          "ignoreExtraEvents": false,
+          "events": [
+            {
+            "serverHeartbeatSucceededEvent": {}
+            },
+            {
+            "serverDescriptionChangedEvent": {}
+            }
+          ]
         }
       ]
     }

test/discovery_and_monitoring/unified/backpressure-network-timeout-fail.json

@@ -49,6 +49,7 @@
                   "useMultipleMongoses": false,
                   "observeEvents": [
                     "serverHeartbeatSucceededEvent",
+                    "serverDescriptionChangedEvent",
                     "poolReadyEvent",
                     "poolClearedEvent"
                   ],
@@ -91,6 +92,17 @@
             "count": 1
           }
         },
+        {
+          "name": "waitForEvent",
+          "object": "testRunner",
+          "arguments": {
+            "client": "client",
+            "event": {
+              "serverDescriptionChangedEvent": {}
+            },
+            "count": 1
+          }
+        },
         {
           "name": "failPoint",
           "object": "testRunner",
@@ -141,6 +153,19 @@
               "poolReadyEvent": {}
             }
           ]
+        },
+        {
+          "client": "client",
+          "eventType": "sdam",
+          "ignoreExtraEvents": false,
+          "events": [
+            {
+            "serverHeartbeatSucceededEvent": {}
+            },
+            {
+            "serverDescriptionChangedEvent": {}
+            }
+          ]
         }
       ]
     }