From 5bdb69b435f6b6f8e91b253373c5951c19880496 Mon Sep 17 00:00:00 2001 From: julius Date: Wed, 6 Oct 2021 13:37:10 -0700 Subject: [PATCH 01/10] catch InvalidOperation --- pymongo/mongo_client.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index c0bcb9575b..b6c376fa5b 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -738,7 +738,13 @@ def target(): client = self_ref() if client is None: return False # Stop the executor. - MongoClient._process_periodic_tasks(client) + try: + MongoClient._process_periodic_tasks(client) + except InvalidOperation as e: + if client._get_topology._closed: + return False + else: + raise e return True executor = periodic_executor.PeriodicExecutor( From 8dfd430de0aab4aacdf912a983498f092429a412 Mon Sep 17 00:00:00 2001 From: julius Date: Thu, 7 Oct 2021 12:37:18 -0700 Subject: [PATCH 02/10] add testing, raise InvalidOperation so it can be caught in _process_periodic_tasks --- pymongo/mongo_client.py | 6 +++++- test/test_client.py | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index b6c376fa5b..57506a514a 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -1589,6 +1589,8 @@ def _process_kill_cursors(self): try: self._cleanup_cursor(True, cursor_id, address, sock_mgr, None, False) + except InvalidOperation as e: + raise e except Exception: helpers._handle_exception() @@ -1606,9 +1608,11 @@ def _process_kill_cursors(self): def _process_periodic_tasks(self): """Process any pending kill cursors requests and maintain connection pool parameters.""" - self._process_kill_cursors() try: + self._process_kill_cursors() self._topology.update_pool(self.__all_credentials) + except InvalidOperation: + pass except Exception: helpers._handle_exception() diff --git a/test/test_client.py b/test/test_client.py index 4398fa08be..59830be162 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -1591,6 +1591,25 @@ def test_network_error_message(self): with self.assertRaisesRegex(AutoReconnect, expected): client.pymongo_test.test.find_one({}) + def test_process_period_tasks(self): + client = MongoClient("mongodb://user:password@localhost/") + coll = client.db.collection + coll.insert_many([{} for _ in range(5)]) + cursor = coll.find(batch_size=2) + cursor.next() + c_id = cursor.cursor_id + assert c_id + client.close() + # Add cursor to kill cursors queue + del cursor + wait_until(lambda: c_id in [c for _, c, _ in + client._MongoClient__kill_cursors_queue], + "waited for cursor to be added to queue") + try: + client._process_periodic_tasks() # This must not raise or print any exceptions + except Exception: + self.fail("client._process_periodic_tasks() raised an exception") + class TestExhaustCursor(IntegrationTest): """Test that clients properly handle errors from exhaust cursors.""" From 877d90af77aea16908f8077b4cc45f3444c2b24a Mon Sep 17 00:00:00 2001 From: julius Date: Thu, 7 Oct 2021 12:40:04 -0700 Subject: [PATCH 03/10] remove error code that doesn't need to be there --- pymongo/mongo_client.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index 57506a514a..c55c4eb560 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -738,13 +738,7 @@ def target(): client = self_ref() if client is None: return False # Stop the executor. - try: - MongoClient._process_periodic_tasks(client) - except InvalidOperation as e: - if client._get_topology._closed: - return False - else: - raise e + MongoClient._process_periodic_tasks(client) return True executor = periodic_executor.PeriodicExecutor( From 995269a956932328d1e247ed8355c4384651f840 Mon Sep 17 00:00:00 2001 From: julius Date: Thu, 7 Oct 2021 14:34:23 -0700 Subject: [PATCH 04/10] use rs_or_single_client --- test/test_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_client.py b/test/test_client.py index 59830be162..4ed557195b 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -1591,8 +1591,8 @@ def test_network_error_message(self): with self.assertRaisesRegex(AutoReconnect, expected): client.pymongo_test.test.find_one({}) - def test_process_period_tasks(self): - client = MongoClient("mongodb://user:password@localhost/") + def test_process_periodic_tasks(self): + client = rs_or_single_client() coll = client.db.collection coll.insert_many([{} for _ in range(5)]) cursor = coll.find(batch_size=2) From acd5cf3b8aa82fe8ae23d46cd141d4bd170116a1 Mon Sep 17 00:00:00 2001 From: julius Date: Thu, 7 Oct 2021 18:48:33 -0700 Subject: [PATCH 05/10] add more test, shane fixes --- pymongo/mongo_client.py | 10 ++++++++-- test/test_client.py | 13 ++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index c55c4eb560..96c516a3d3 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -1584,7 +1584,10 @@ def _process_kill_cursors(self): self._cleanup_cursor(True, cursor_id, address, sock_mgr, None, False) except InvalidOperation as e: - raise e + if self._topology._closed: + raise e + else: + helpers._handle_exception() except Exception: helpers._handle_exception() @@ -1606,7 +1609,10 @@ def _process_periodic_tasks(self): self._process_kill_cursors() self._topology.update_pool(self.__all_credentials) except InvalidOperation: - pass + if self._topology._closed: + pass + else: + helpers._handle_exception() except Exception: helpers._handle_exception() diff --git a/test/test_client.py b/test/test_client.py index 4ed557195b..c2ea9adc56 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -1591,6 +1591,7 @@ def test_network_error_message(self): with self.assertRaisesRegex(AutoReconnect, expected): client.pymongo_test.test.find_one({}) + @unittest.skipIf('PyPy' in sys.version, 'PYTHON-2938 could fail on PyPy') def test_process_periodic_tasks(self): client = rs_or_single_client() coll = client.db.collection @@ -1598,17 +1599,15 @@ def test_process_periodic_tasks(self): cursor = coll.find(batch_size=2) cursor.next() c_id = cursor.cursor_id - assert c_id + self.assertIsNotNone(c_id) client.close() # Add cursor to kill cursors queue del cursor - wait_until(lambda: c_id in [c for _, c, _ in - client._MongoClient__kill_cursors_queue], + wait_until(lambda: client._MongoClient__kill_cursors_queuee, "waited for cursor to be added to queue") - try: - client._process_periodic_tasks() # This must not raise or print any exceptions - except Exception: - self.fail("client._process_periodic_tasks() raised an exception") + client._process_periodic_tasks() # This must not raise or print any exceptions + with self.assertRaises(InvalidOperation): + coll.find({}) class TestExhaustCursor(IntegrationTest): From 20c43b044bbc33d317f9a8d4a8fe3bd7a4601fd0 Mon Sep 17 00:00:00 2001 From: julius Date: Fri, 8 Oct 2021 11:16:32 -0700 Subject: [PATCH 06/10] fix typo --- test/test_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_client.py b/test/test_client.py index c2ea9adc56..7a9d70ce9d 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -1603,7 +1603,7 @@ def test_process_periodic_tasks(self): client.close() # Add cursor to kill cursors queue del cursor - wait_until(lambda: client._MongoClient__kill_cursors_queuee, + wait_until(lambda: client._MongoClient__kill_cursors_queue, "waited for cursor to be added to queue") client._process_periodic_tasks() # This must not raise or print any exceptions with self.assertRaises(InvalidOperation): From 4a090c73549a18964f27611cc14cf612abca773b Mon Sep 17 00:00:00 2001 From: julius Date: Fri, 8 Oct 2021 12:02:35 -0700 Subject: [PATCH 07/10] shane fixes --- pymongo/mongo_client.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index 96c516a3d3..f2a3c5c072 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -1583,13 +1583,11 @@ def _process_kill_cursors(self): try: self._cleanup_cursor(True, cursor_id, address, sock_mgr, None, False) - except InvalidOperation as e: - if self._topology._closed: - raise e + except Exception as e: + if isinstance(e, InvalidOperation) and self._topology._closed: + raise else: helpers._handle_exception() - except Exception: - helpers._handle_exception() # Don't re-open topology if it's closed and there's no pending cursors. if address_to_cursor_ids: @@ -1599,7 +1597,11 @@ def _process_kill_cursors(self): self._kill_cursors( cursor_ids, address, topology, session=None) except Exception: - helpers._handle_exception() + if (isinstance(e,InvalidOperation) and + self._topology._closed): + raise + else: + helpers._handle_exception() # This method is run periodically by a background thread. def _process_periodic_tasks(self): @@ -1608,13 +1610,11 @@ def _process_periodic_tasks(self): try: self._process_kill_cursors() self._topology.update_pool(self.__all_credentials) - except InvalidOperation: - if self._topology._closed: - pass + except Exception as e: + if isinstance(e, InvalidOperation) and self._topology._closed: + return else: helpers._handle_exception() - except Exception: - helpers._handle_exception() def __start_session(self, implicit, **kwargs): # Raises ConfigurationError if sessions are not supported. From 13560f857b989f9613555212ca1575d8f8db758d Mon Sep 17 00:00:00 2001 From: julius Date: Fri, 8 Oct 2021 12:08:10 -0700 Subject: [PATCH 08/10] add comment --- pymongo/mongo_client.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index f2a3c5c072..07c84a687e 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -1585,6 +1585,8 @@ def _process_kill_cursors(self): None, False) except Exception as e: if isinstance(e, InvalidOperation) and self._topology._closed: + # Raise the exception when client is closed so that it + # can be caught in _process_periodic_tasks raise else: helpers._handle_exception() From 803198813b52029444fc14dc62a5f1a9075dc2ea Mon Sep 17 00:00:00 2001 From: julius Date: Fri, 8 Oct 2021 12:18:45 -0700 Subject: [PATCH 09/10] change test so it works --- test/test_client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_client.py b/test/test_client.py index 7a9d70ce9d..41f655bf49 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -1607,7 +1607,8 @@ def test_process_periodic_tasks(self): "waited for cursor to be added to queue") client._process_periodic_tasks() # This must not raise or print any exceptions with self.assertRaises(InvalidOperation): - coll.find({}) + coll.insert_many([{} for _ in range(5)]) + class TestExhaustCursor(IntegrationTest): From 8a59524c13ec20f03bfb0e7c37a8faf4e518c769 Mon Sep 17 00:00:00 2001 From: julius Date: Fri, 8 Oct 2021 13:37:20 -0700 Subject: [PATCH 10/10] shane fixes --- pymongo/mongo_client.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index f6b2d1c091..279eae5587 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -1596,8 +1596,9 @@ def _process_kill_cursors(self): try: self._cleanup_cursor(True, cursor_id, address, sock_mgr, None, False) - except Exception as e: - if isinstance(e, InvalidOperation) and self._topology._closed: + except Exception as exc: + if (isinstance(exc, InvalidOperation) + and self._topology._closed): # Raise the exception when client is closed so that it # can be caught in _process_periodic_tasks raise @@ -1611,8 +1612,8 @@ def _process_kill_cursors(self): try: self._kill_cursors( cursor_ids, address, topology, session=None) - except Exception: - if (isinstance(e,InvalidOperation) and + except Exception as exc: + if (isinstance(exc, InvalidOperation) and self._topology._closed): raise else: @@ -1625,8 +1626,8 @@ def _process_periodic_tasks(self): try: self._process_kill_cursors() self._topology.update_pool(self.__all_credentials) - except Exception as e: - if isinstance(e, InvalidOperation) and self._topology._closed: + except Exception as exc: + if isinstance(exc, InvalidOperation) and self._topology._closed: return else: helpers._handle_exception()