From a62171c67ba6c00624632a713913609dada866d4 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Fri, 28 Mar 2025 10:48:16 -0400 Subject: [PATCH 1/3] PYTHON-4924 - PoolClearedError should have TransientTransactionError label appended to it --- pymongo/asynchronous/pool.py | 16 ++++++++++------ pymongo/synchronous/pool.py | 16 ++++++++++------ test/asynchronous/test_pooling.py | 17 ++++++++++++++++- test/test_pooling.py | 17 ++++++++++++++++- 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/pymongo/asynchronous/pool.py b/pymongo/asynchronous/pool.py index d06c528e78..1476934c33 100644 --- a/pymongo/asynchronous/pool.py +++ b/pymongo/asynchronous/pool.py @@ -190,6 +190,10 @@ def _raise_connection_failure( ) -> NoReturn: """Convert a socket.error to ConnectionFailure and raise it.""" host, port = address + if isinstance(error, PyMongoError) and error._error_labels: + labels = error._error_labels + else: + labels = None # If connecting to a Unix socket, port will be None. if port is not None: msg = "%s:%d: %s" % (host, port, error) @@ -200,15 +204,15 @@ def _raise_connection_failure( if "configured timeouts" not in msg: msg += format_timeout_details(timeout_details) if isinstance(error, socket.timeout): - raise NetworkTimeout(msg) from error + raise NetworkTimeout(msg, errors={"errorLabels": labels}) from error elif isinstance(error, SSLError) and "timed out" in str(error): # Eventlet does not distinguish TLS network timeouts from other # SSLErrors (https://github.com/eventlet/eventlet/issues/692). # Luckily, we can work around this limitation because the phrase # 'timed out' appears in all the timeout related SSLErrors raised. - raise NetworkTimeout(msg) from error + raise NetworkTimeout(msg, errors={"errorLabels": labels}) from error else: - raise AutoReconnect(msg) from error + raise AutoReconnect(msg, errors={"errorLabels": labels}) from error def _get_timeout_details(options: PoolOptions) -> dict[str, float]: @@ -1420,9 +1424,9 @@ def _raise_if_not_ready(self, checkout_started_time: float, emit_event: bool) -> ) details = _get_timeout_details(self.opts) - _raise_connection_failure( - self.address, AutoReconnect("connection pool paused"), timeout_details=details - ) + error = AutoReconnect("connection pool paused") + error._add_error_label("TransientTransactionError") + _raise_connection_failure(self.address, error, timeout_details=details) async def _get_conn( self, checkout_started_time: float, handler: Optional[_MongoClientErrorHandler] = None diff --git a/pymongo/synchronous/pool.py b/pymongo/synchronous/pool.py index cd78e26fea..f7a7547305 100644 --- a/pymongo/synchronous/pool.py +++ b/pymongo/synchronous/pool.py @@ -190,6 +190,10 @@ def _raise_connection_failure( ) -> NoReturn: """Convert a socket.error to ConnectionFailure and raise it.""" host, port = address + if isinstance(error, PyMongoError) and error._error_labels: + labels = error._error_labels + else: + labels = None # If connecting to a Unix socket, port will be None. if port is not None: msg = "%s:%d: %s" % (host, port, error) @@ -200,15 +204,15 @@ def _raise_connection_failure( if "configured timeouts" not in msg: msg += format_timeout_details(timeout_details) if isinstance(error, socket.timeout): - raise NetworkTimeout(msg) from error + raise NetworkTimeout(msg, errors={"errorLabels": labels}) from error elif isinstance(error, SSLError) and "timed out" in str(error): # Eventlet does not distinguish TLS network timeouts from other # SSLErrors (https://github.com/eventlet/eventlet/issues/692). # Luckily, we can work around this limitation because the phrase # 'timed out' appears in all the timeout related SSLErrors raised. - raise NetworkTimeout(msg) from error + raise NetworkTimeout(msg, errors={"errorLabels": labels}) from error else: - raise AutoReconnect(msg) from error + raise AutoReconnect(msg, errors={"errorLabels": labels}) from error def _get_timeout_details(options: PoolOptions) -> dict[str, float]: @@ -1414,9 +1418,9 @@ def _raise_if_not_ready(self, checkout_started_time: float, emit_event: bool) -> ) details = _get_timeout_details(self.opts) - _raise_connection_failure( - self.address, AutoReconnect("connection pool paused"), timeout_details=details - ) + error = AutoReconnect("connection pool paused") + error._add_error_label("TransientTransactionError") + _raise_connection_failure(self.address, error, timeout_details=details) def _get_conn( self, checkout_started_time: float, handler: Optional[_MongoClientErrorHandler] = None diff --git a/test/asynchronous/test_pooling.py b/test/asynchronous/test_pooling.py index 8213c794fe..8bd12076c0 100644 --- a/test/asynchronous/test_pooling.py +++ b/test/asynchronous/test_pooling.py @@ -36,7 +36,7 @@ from test.asynchronous.helpers import ConcurrentRunner from test.utils_shared import delay -from pymongo.asynchronous.pool import Pool, PoolOptions +from pymongo.asynchronous.pool import Pool, PoolOptions, PoolState from pymongo.socket_checker import SocketChecker _IS_SYNC = False @@ -608,6 +608,21 @@ async def test_max_pool_size_with_connection_failure(self): # seems error-prone, so check the message too. self.assertNotIn("waiting for socket from pool", str(context.exception)) + async def test_pool_cleared_error_labelled_transient(self): + test_pool = Pool( + ("localhost", 27017), + PoolOptions(max_pool_size=1), + ) + # Pause the pool, causing it to fail connection checkout. + test_pool.state = PoolState.PAUSED + + with self.assertRaises(AutoReconnect) as context: + async with test_pool.checkout(): + pass + + # Verify that the TransientTransactionError label is present in the error. + self.assertTrue(context.exception.has_error_label("TransientTransactionError")) + if __name__ == "__main__": unittest.main() diff --git a/test/test_pooling.py b/test/test_pooling.py index 44e8c4afe5..9329680fc9 100644 --- a/test/test_pooling.py +++ b/test/test_pooling.py @@ -37,7 +37,7 @@ from test.utils_shared import delay from pymongo.socket_checker import SocketChecker -from pymongo.synchronous.pool import Pool, PoolOptions +from pymongo.synchronous.pool import Pool, PoolOptions, PoolState _IS_SYNC = True @@ -606,6 +606,21 @@ def test_max_pool_size_with_connection_failure(self): # seems error-prone, so check the message too. self.assertNotIn("waiting for socket from pool", str(context.exception)) + def test_pool_cleared_error_labelled_transient(self): + test_pool = Pool( + ("localhost", 27017), + PoolOptions(max_pool_size=1), + ) + # Pause the pool, causing it to fail connection checkout. + test_pool.state = PoolState.PAUSED + + with self.assertRaises(AutoReconnect) as context: + with test_pool.checkout(): + pass + + # Verify that the TransientTransactionError label is present in the error. + self.assertTrue(context.exception.has_error_label("TransientTransactionError")) + if __name__ == "__main__": unittest.main() From bd02e75f9190ae35565428309bc09f2c98930f12 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Mon, 7 Apr 2025 17:10:36 -0400 Subject: [PATCH 2/3] Add test, revert pymongo changes --- pymongo/asynchronous/pool.py | 16 ++++++---------- pymongo/synchronous/pool.py | 16 ++++++---------- test/asynchronous/test_pooling.py | 15 --------------- test/asynchronous/test_transactions.py | 19 +++++++++++++++++++ test/test_pooling.py | 15 --------------- test/test_transactions.py | 19 +++++++++++++++++++ 6 files changed, 50 insertions(+), 50 deletions(-) diff --git a/pymongo/asynchronous/pool.py b/pymongo/asynchronous/pool.py index 1476934c33..d06c528e78 100644 --- a/pymongo/asynchronous/pool.py +++ b/pymongo/asynchronous/pool.py @@ -190,10 +190,6 @@ def _raise_connection_failure( ) -> NoReturn: """Convert a socket.error to ConnectionFailure and raise it.""" host, port = address - if isinstance(error, PyMongoError) and error._error_labels: - labels = error._error_labels - else: - labels = None # If connecting to a Unix socket, port will be None. if port is not None: msg = "%s:%d: %s" % (host, port, error) @@ -204,15 +200,15 @@ def _raise_connection_failure( if "configured timeouts" not in msg: msg += format_timeout_details(timeout_details) if isinstance(error, socket.timeout): - raise NetworkTimeout(msg, errors={"errorLabels": labels}) from error + raise NetworkTimeout(msg) from error elif isinstance(error, SSLError) and "timed out" in str(error): # Eventlet does not distinguish TLS network timeouts from other # SSLErrors (https://github.com/eventlet/eventlet/issues/692). # Luckily, we can work around this limitation because the phrase # 'timed out' appears in all the timeout related SSLErrors raised. - raise NetworkTimeout(msg, errors={"errorLabels": labels}) from error + raise NetworkTimeout(msg) from error else: - raise AutoReconnect(msg, errors={"errorLabels": labels}) from error + raise AutoReconnect(msg) from error def _get_timeout_details(options: PoolOptions) -> dict[str, float]: @@ -1424,9 +1420,9 @@ def _raise_if_not_ready(self, checkout_started_time: float, emit_event: bool) -> ) details = _get_timeout_details(self.opts) - error = AutoReconnect("connection pool paused") - error._add_error_label("TransientTransactionError") - _raise_connection_failure(self.address, error, timeout_details=details) + _raise_connection_failure( + self.address, AutoReconnect("connection pool paused"), timeout_details=details + ) async def _get_conn( self, checkout_started_time: float, handler: Optional[_MongoClientErrorHandler] = None diff --git a/pymongo/synchronous/pool.py b/pymongo/synchronous/pool.py index f7a7547305..cd78e26fea 100644 --- a/pymongo/synchronous/pool.py +++ b/pymongo/synchronous/pool.py @@ -190,10 +190,6 @@ def _raise_connection_failure( ) -> NoReturn: """Convert a socket.error to ConnectionFailure and raise it.""" host, port = address - if isinstance(error, PyMongoError) and error._error_labels: - labels = error._error_labels - else: - labels = None # If connecting to a Unix socket, port will be None. if port is not None: msg = "%s:%d: %s" % (host, port, error) @@ -204,15 +200,15 @@ def _raise_connection_failure( if "configured timeouts" not in msg: msg += format_timeout_details(timeout_details) if isinstance(error, socket.timeout): - raise NetworkTimeout(msg, errors={"errorLabels": labels}) from error + raise NetworkTimeout(msg) from error elif isinstance(error, SSLError) and "timed out" in str(error): # Eventlet does not distinguish TLS network timeouts from other # SSLErrors (https://github.com/eventlet/eventlet/issues/692). # Luckily, we can work around this limitation because the phrase # 'timed out' appears in all the timeout related SSLErrors raised. - raise NetworkTimeout(msg, errors={"errorLabels": labels}) from error + raise NetworkTimeout(msg) from error else: - raise AutoReconnect(msg, errors={"errorLabels": labels}) from error + raise AutoReconnect(msg) from error def _get_timeout_details(options: PoolOptions) -> dict[str, float]: @@ -1418,9 +1414,9 @@ def _raise_if_not_ready(self, checkout_started_time: float, emit_event: bool) -> ) details = _get_timeout_details(self.opts) - error = AutoReconnect("connection pool paused") - error._add_error_label("TransientTransactionError") - _raise_connection_failure(self.address, error, timeout_details=details) + _raise_connection_failure( + self.address, AutoReconnect("connection pool paused"), timeout_details=details + ) def _get_conn( self, checkout_started_time: float, handler: Optional[_MongoClientErrorHandler] = None diff --git a/test/asynchronous/test_pooling.py b/test/asynchronous/test_pooling.py index 8bd12076c0..5649a02ffe 100644 --- a/test/asynchronous/test_pooling.py +++ b/test/asynchronous/test_pooling.py @@ -608,21 +608,6 @@ async def test_max_pool_size_with_connection_failure(self): # seems error-prone, so check the message too. self.assertNotIn("waiting for socket from pool", str(context.exception)) - async def test_pool_cleared_error_labelled_transient(self): - test_pool = Pool( - ("localhost", 27017), - PoolOptions(max_pool_size=1), - ) - # Pause the pool, causing it to fail connection checkout. - test_pool.state = PoolState.PAUSED - - with self.assertRaises(AutoReconnect) as context: - async with test_pool.checkout(): - pass - - # Verify that the TransientTransactionError label is present in the error. - self.assertTrue(context.exception.has_error_label("TransientTransactionError")) - if __name__ == "__main__": unittest.main() diff --git a/test/asynchronous/test_transactions.py b/test/asynchronous/test_transactions.py index 884110cd45..d2eed40bac 100644 --- a/test/asynchronous/test_transactions.py +++ b/test/asynchronous/test_transactions.py @@ -20,6 +20,8 @@ from test.asynchronous.utils_spec_runner import AsyncSpecRunner from gridfs.asynchronous.grid_file import AsyncGridFS, AsyncGridFSBucket +from pymongo.asynchronous.pool import PoolState +from pymongo.server_selectors import writable_server_selector sys.path[0:0] = [""] @@ -39,6 +41,7 @@ from pymongo.asynchronous.cursor import AsyncCursor from pymongo.asynchronous.helpers import anext from pymongo.errors import ( + AutoReconnect, CollectionInvalid, ConfigurationError, ConnectionFailure, @@ -386,6 +389,22 @@ async def find_raw_batches(*args, **kwargs): if isinstance(res, (AsyncCommandCursor, AsyncCursor)): await res.to_list() + @async_client_context.require_transactions + async def test_transaction_pool_cleared_error_labelled_transient(self): + c = await self.async_single_client() + + with self.assertRaises(AutoReconnect) as context: + async with c.start_session() as session: + async with await session.start_transaction(): + server = await c._select_server(writable_server_selector, session, "test") + # Pause the server's pool, causing it to fail connection checkout. + server.pool.state = PoolState.PAUSED + async with c._checkout(server, session): + pass + + # Verify that the TransientTransactionError label is present in the error. + self.assertTrue(context.exception.has_error_label("TransientTransactionError")) + class PatchSessionTimeout: """Patches the client_session's with_transaction timeout for testing.""" diff --git a/test/test_pooling.py b/test/test_pooling.py index 9329680fc9..d780ba796e 100644 --- a/test/test_pooling.py +++ b/test/test_pooling.py @@ -606,21 +606,6 @@ def test_max_pool_size_with_connection_failure(self): # seems error-prone, so check the message too. self.assertNotIn("waiting for socket from pool", str(context.exception)) - def test_pool_cleared_error_labelled_transient(self): - test_pool = Pool( - ("localhost", 27017), - PoolOptions(max_pool_size=1), - ) - # Pause the pool, causing it to fail connection checkout. - test_pool.state = PoolState.PAUSED - - with self.assertRaises(AutoReconnect) as context: - with test_pool.checkout(): - pass - - # Verify that the TransientTransactionError label is present in the error. - self.assertTrue(context.exception.has_error_label("TransientTransactionError")) - if __name__ == "__main__": unittest.main() diff --git a/test/test_transactions.py b/test/test_transactions.py index 80b3e3765e..b883e88efc 100644 --- a/test/test_transactions.py +++ b/test/test_transactions.py @@ -20,6 +20,8 @@ from test.utils_spec_runner import SpecRunner from gridfs.synchronous.grid_file import GridFS, GridFSBucket +from pymongo.server_selectors import writable_server_selector +from pymongo.synchronous.pool import PoolState sys.path[0:0] = [""] @@ -34,6 +36,7 @@ from bson.raw_bson import RawBSONDocument from pymongo import WriteConcern from pymongo.errors import ( + AutoReconnect, CollectionInvalid, ConfigurationError, ConnectionFailure, @@ -378,6 +381,22 @@ def find_raw_batches(*args, **kwargs): if isinstance(res, (CommandCursor, Cursor)): res.to_list() + @client_context.require_transactions + def test_transaction_pool_cleared_error_labelled_transient(self): + c = self.single_client() + + with self.assertRaises(AutoReconnect) as context: + with c.start_session() as session: + with session.start_transaction(): + server = c._select_server(writable_server_selector, session, "test") + # Pause the server's pool, causing it to fail connection checkout. + server.pool.state = PoolState.PAUSED + with c._checkout(server, session): + pass + + # Verify that the TransientTransactionError label is present in the error. + self.assertTrue(context.exception.has_error_label("TransientTransactionError")) + class PatchSessionTimeout: """Patches the client_session's with_transaction timeout for testing.""" From c5339b58d0afee0f7fe07bf5a20f096e918695b0 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Mon, 7 Apr 2025 17:12:46 -0400 Subject: [PATCH 3/3] Fix test_pooling import --- test/asynchronous/test_pooling.py | 2 +- test/test_pooling.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/asynchronous/test_pooling.py b/test/asynchronous/test_pooling.py index 5649a02ffe..8213c794fe 100644 --- a/test/asynchronous/test_pooling.py +++ b/test/asynchronous/test_pooling.py @@ -36,7 +36,7 @@ from test.asynchronous.helpers import ConcurrentRunner from test.utils_shared import delay -from pymongo.asynchronous.pool import Pool, PoolOptions, PoolState +from pymongo.asynchronous.pool import Pool, PoolOptions from pymongo.socket_checker import SocketChecker _IS_SYNC = False diff --git a/test/test_pooling.py b/test/test_pooling.py index d780ba796e..44e8c4afe5 100644 --- a/test/test_pooling.py +++ b/test/test_pooling.py @@ -37,7 +37,7 @@ from test.utils_shared import delay from pymongo.socket_checker import SocketChecker -from pymongo.synchronous.pool import Pool, PoolOptions, PoolState +from pymongo.synchronous.pool import Pool, PoolOptions _IS_SYNC = True