From b3448e92d269dbe25d2bfd9609f33e37af9241af Mon Sep 17 00:00:00 2001 From: Robsdedude Date: Fri, 16 May 2025 14:49:03 +0200 Subject: [PATCH] Fail hard on using closed driver --- CHANGELOG.md | 3 ++ docs/source/api.rst | 5 +++ docs/source/async_api.rst | 5 +++ src/neo4j/_async/driver.py | 64 +++++++++++++++++++++++--------- src/neo4j/_sync/driver.py | 64 +++++++++++++++++++++++--------- tests/unit/async_/test_driver.py | 25 +++++-------- tests/unit/sync/test_driver.py | 25 +++++-------- 7 files changed, 125 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 381d7def1..d7fcd6b3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,9 @@ See also https://github.com/neo4j/neo4j-python-driver/wiki for a full changelog. If you were calling it directly, please use `Record.__getitem__(slice(...))` or simply `record[...]` instead. - Remove deprecated class `neo4j.Bookmark` in favor of `neo4j.Bookmarks`. - Remove deprecated class `session.last_bookmark()` in favor of `last_bookmarks()`. +- Change behavior of closed drivers: + - Raise `DriverError` on using the closed driver. + - Calling `driver.close()` again is now a no-op. ## Version 5.28 diff --git a/docs/source/api.rst b/docs/source/api.rst index 2db712fdc..177776948 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -370,6 +370,8 @@ Closing a driver will immediately shut down all connections in the pool. :returns: the result of the ``result_transformer_`` :rtype: T + :raises DriverError: if the driver has been closed. + .. versionadded:: 5.5 .. versionchanged:: 5.8 @@ -384,6 +386,9 @@ Closing a driver will immediately shut down all connections in the pool. The ``query_`` parameter now also accepts a :class:`.Query` object instead of only :class:`str`. + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. + .. _driver-configuration-ref: diff --git a/docs/source/async_api.rst b/docs/source/async_api.rst index 418247813..b3d9a3379 100644 --- a/docs/source/async_api.rst +++ b/docs/source/async_api.rst @@ -352,6 +352,8 @@ Closing a driver will immediately shut down all connections in the pool. :returns: the result of the ``result_transformer_`` :rtype: T + :raises DriverError: if the driver has been closed. + .. versionadded:: 5.5 .. versionchanged:: 5.8 @@ -366,6 +368,9 @@ Closing a driver will immediately shut down all connections in the pool. The ``query_`` parameter now also accepts a :class:`.Query` object instead of only :class:`str`. + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. + .. _async-driver-configuration-ref: diff --git a/src/neo4j/_async/driver.py b/src/neo4j/_async/driver.py index a202fc590..d135b852c 100644 --- a/src/neo4j/_async/driver.py +++ b/src/neo4j/_async/driver.py @@ -85,7 +85,10 @@ AsyncClientCertificateProvider, ClientCertificate, ) -from ..exceptions import Neo4jError +from ..exceptions import ( + DriverError, + Neo4jError, +) from .auth_management import _AsyncStaticClientCertificateProvider from .bookmark_manager import ( AsyncNeo4jBookmarkManager, @@ -556,13 +559,7 @@ def __del__( def _check_state(self): if self._closed: - # TODO: 6.0 - raise the error - # raise DriverError("Driver closed") - deprecation_warn( - "Using a driver after it has been closed is deprecated. " - "Future versions of the driver will raise an error.", - stack_level=3, - ) + raise DriverError("Driver closed") @property def encrypted(self) -> bool: @@ -614,6 +611,11 @@ def session(self, **config) -> AsyncSession: key-word arguments. :returns: new :class:`neo4j.AsyncSession` object + + :raises DriverError: if the driver has been closed. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ if "warn_notification_severity" in config: # Would work just fine, but we don't want to introduce yet @@ -651,9 +653,8 @@ async def close(self) -> None: spawned from it (such as sessions or transactions) while calling this method. Failing to do so results in unspecified behavior. """ - # TODO: 6.0 - NOOP if already closed - # if self._closed: - # return + if self._closed: + return try: await self._pool.close() except asyncio.CancelledError: @@ -917,6 +918,8 @@ async def example(driver: neo4j.AsyncDriver) -> neo4j.Record:: :returns: the result of the ``result_transformer_`` :rtype: T + :raises DriverError: if the driver has been closed. + .. versionadded:: 5.5 .. versionchanged:: 5.8 @@ -930,6 +933,9 @@ async def example(driver: neo4j.AsyncDriver) -> neo4j.Record:: .. versionchanged:: 5.15 The ``query_`` parameter now also accepts a :class:`.Query` object instead of only :class:`str`. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. ''' # noqa: E501 example code isn't too long self._check_state() invalid_kwargs = [ @@ -1073,11 +1079,15 @@ async def verify_connectivity(self, **config) -> None: :raises Exception: if the driver cannot connect to the remote. Use the exception to further understand the cause of the connectivity problem. + :raises DriverError: if the driver has been closed. .. versionchanged:: 5.0 The undocumented return value has been removed. If you need information about the remote server, use :meth:`get_server_info` instead. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() if config: @@ -1152,8 +1162,12 @@ async def get_server_info(self, **config) -> ServerInfo: :raises Exception: if the driver cannot connect to the remote. Use the exception to further understand the cause of the connectivity problem. + :raises DriverError: if the driver has been closed. .. versionadded:: 5.0 + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() if config: @@ -1170,15 +1184,20 @@ async def supports_multi_db(self) -> bool: """ Check if the server or cluster supports multi-databases. - :returns: Returns true if the server or cluster the driver connects to - supports multi-databases, otherwise false. - .. note:: Feature support query based solely on the Bolt protocol version. The feature might still be disabled on the server side even if this function return :data:`True`. It just guarantees that the driver won't throw a :exc:`.ConfigurationError` when trying to use this driver feature. + + :returns: Returns true if the server or cluster the driver connects to + supports multi-databases, otherwise false. + + :raises DriverError: if the driver has been closed. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() session_config = self._read_session_config({}) @@ -1246,10 +1265,14 @@ async def verify_authentication( :raises Exception: if the driver cannot connect to the remote. Use the exception to further understand the cause of the connectivity problem. + :raises DriverError: if the driver has been closed. .. versionadded:: 5.8 .. versionchanged:: 5.14 Stabilized from experimental. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() if config: @@ -1281,10 +1304,6 @@ async def supports_session_auth(self) -> bool: """ Check if the remote supports connection re-authentication. - :returns: Returns true if the server or cluster the driver connects to - supports re-authentication of existing connections, otherwise - false. - .. note:: Feature support query based solely on the Bolt protocol version. The feature might still be disabled on the server side even if this @@ -1292,7 +1311,16 @@ async def supports_session_auth(self) -> bool: won't throw a :exc:`.ConfigurationError` when trying to use this driver feature. + :returns: Returns true if the server or cluster the driver connects to + supports re-authentication of existing connections, otherwise + false. + + :raises DriverError: if the driver has been closed. + .. versionadded:: 5.8 + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() session_config = self._read_session_config({}) diff --git a/src/neo4j/_sync/driver.py b/src/neo4j/_sync/driver.py index 24a61b5c8..185a49a49 100644 --- a/src/neo4j/_sync/driver.py +++ b/src/neo4j/_sync/driver.py @@ -84,7 +84,10 @@ ClientCertificate, ClientCertificateProvider, ) -from ..exceptions import Neo4jError +from ..exceptions import ( + DriverError, + Neo4jError, +) from .auth_management import _StaticClientCertificateProvider from .bookmark_manager import ( Neo4jBookmarkManager, @@ -555,13 +558,7 @@ def __del__( def _check_state(self): if self._closed: - # TODO: 6.0 - raise the error - # raise DriverError("Driver closed") - deprecation_warn( - "Using a driver after it has been closed is deprecated. " - "Future versions of the driver will raise an error.", - stack_level=3, - ) + raise DriverError("Driver closed") @property def encrypted(self) -> bool: @@ -613,6 +610,11 @@ def session(self, **config) -> Session: key-word arguments. :returns: new :class:`neo4j.Session` object + + :raises DriverError: if the driver has been closed. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ if "warn_notification_severity" in config: # Would work just fine, but we don't want to introduce yet @@ -650,9 +652,8 @@ def close(self) -> None: spawned from it (such as sessions or transactions) while calling this method. Failing to do so results in unspecified behavior. """ - # TODO: 6.0 - NOOP if already closed - # if self._closed: - # return + if self._closed: + return try: self._pool.close() except asyncio.CancelledError: @@ -916,6 +917,8 @@ def example(driver: neo4j.Driver) -> neo4j.Record:: :returns: the result of the ``result_transformer_`` :rtype: T + :raises DriverError: if the driver has been closed. + .. versionadded:: 5.5 .. versionchanged:: 5.8 @@ -929,6 +932,9 @@ def example(driver: neo4j.Driver) -> neo4j.Record:: .. versionchanged:: 5.15 The ``query_`` parameter now also accepts a :class:`.Query` object instead of only :class:`str`. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. ''' # noqa: E501 example code isn't too long self._check_state() invalid_kwargs = [ @@ -1072,11 +1078,15 @@ def verify_connectivity(self, **config) -> None: :raises Exception: if the driver cannot connect to the remote. Use the exception to further understand the cause of the connectivity problem. + :raises DriverError: if the driver has been closed. .. versionchanged:: 5.0 The undocumented return value has been removed. If you need information about the remote server, use :meth:`get_server_info` instead. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() if config: @@ -1151,8 +1161,12 @@ def get_server_info(self, **config) -> ServerInfo: :raises Exception: if the driver cannot connect to the remote. Use the exception to further understand the cause of the connectivity problem. + :raises DriverError: if the driver has been closed. .. versionadded:: 5.0 + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() if config: @@ -1169,15 +1183,20 @@ def supports_multi_db(self) -> bool: """ Check if the server or cluster supports multi-databases. - :returns: Returns true if the server or cluster the driver connects to - supports multi-databases, otherwise false. - .. note:: Feature support query based solely on the Bolt protocol version. The feature might still be disabled on the server side even if this function return :data:`True`. It just guarantees that the driver won't throw a :exc:`.ConfigurationError` when trying to use this driver feature. + + :returns: Returns true if the server or cluster the driver connects to + supports multi-databases, otherwise false. + + :raises DriverError: if the driver has been closed. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() session_config = self._read_session_config({}) @@ -1245,10 +1264,14 @@ def verify_authentication( :raises Exception: if the driver cannot connect to the remote. Use the exception to further understand the cause of the connectivity problem. + :raises DriverError: if the driver has been closed. .. versionadded:: 5.8 .. versionchanged:: 5.14 Stabilized from experimental. + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() if config: @@ -1280,10 +1303,6 @@ def supports_session_auth(self) -> bool: """ Check if the remote supports connection re-authentication. - :returns: Returns true if the server or cluster the driver connects to - supports re-authentication of existing connections, otherwise - false. - .. note:: Feature support query based solely on the Bolt protocol version. The feature might still be disabled on the server side even if this @@ -1291,7 +1310,16 @@ def supports_session_auth(self) -> bool: won't throw a :exc:`.ConfigurationError` when trying to use this driver feature. + :returns: Returns true if the server or cluster the driver connects to + supports re-authentication of existing connections, otherwise + false. + + :raises DriverError: if the driver has been closed. + .. versionadded:: 5.8 + + .. versionchanged:: 6.0 + Raise :exc:`DriverError` if the driver has been closed. """ self._check_state() session_config = self._read_session_config({}) diff --git a/tests/unit/async_/test_driver.py b/tests/unit/async_/test_driver.py index 8a87a1e49..d21d88b69 100644 --- a/tests/unit/async_/test_driver.py +++ b/tests/unit/async_/test_driver.py @@ -52,6 +52,7 @@ AsyncBoltPool, AsyncNeo4jPool, ) +from neo4j._async_compat.util import AsyncUtil from neo4j._debug import ENABLED as DEBUG_ENABLED from neo4j.api import ( AsyncBookmarkManager, @@ -63,7 +64,10 @@ AsyncClientCertificateProvider, ClientCertificate, ) -from neo4j.exceptions import ConfigurationError +from neo4j.exceptions import ( + ConfigurationError, + DriverError, +) from ..._async_compat import ( AsyncTestDecorators, @@ -1324,28 +1328,22 @@ async def test_supports_session_auth(session_cls_mock) -> None: ), ) @mark_async_test -async def test_using_closed_driver_where_deprecated( +async def test_using_closed_driver_where_forbidden( method_name, args, kwargs, session_cls_mock ) -> None: driver = AsyncGraphDatabase.driver("bolt://localhost") await driver.close() method = getattr(driver, method_name) - with pytest.warns( - DeprecationWarning, - match="Using a driver after it has been closed is deprecated.", - ): - if inspect.iscoroutinefunction(method): - await method(*args, **kwargs) - else: - method(*args, **kwargs) + with pytest.raises(DriverError, match="closed"): + await AsyncUtil.callback(method, *args, **kwargs) @pytest.mark.parametrize( ("method_name", "args", "kwargs"), (("close", (), {}),) ) @mark_async_test -async def test_using_closed_driver_where_not_deprecated( +async def test_using_closed_driver_where_no_op( method_name, args, kwargs, session_cls_mock ) -> None: driver = AsyncGraphDatabase.driver("bolt://localhost") @@ -1354,7 +1352,4 @@ async def test_using_closed_driver_where_not_deprecated( method = getattr(driver, method_name) with warnings.catch_warnings(): warnings.simplefilter("error") - if inspect.iscoroutinefunction(method): - await method(*args, **kwargs) - else: - method(*args, **kwargs) + await AsyncUtil.callback(method, *args, **kwargs) diff --git a/tests/unit/sync/test_driver.py b/tests/unit/sync/test_driver.py index 2d22b41af..9552ff635 100644 --- a/tests/unit/sync/test_driver.py +++ b/tests/unit/sync/test_driver.py @@ -45,6 +45,7 @@ TrustSystemCAs, ) from neo4j._api import TelemetryAPI +from neo4j._async_compat.util import Util from neo4j._debug import ENABLED as DEBUG_ENABLED from neo4j._sync.auth_management import _StaticClientCertificateProvider from neo4j._sync.config import PoolConfig @@ -62,7 +63,10 @@ ClientCertificate, ClientCertificateProvider, ) -from neo4j.exceptions import ConfigurationError +from neo4j.exceptions import ( + ConfigurationError, + DriverError, +) from ..._async_compat import ( mark_sync_test, @@ -1323,28 +1327,22 @@ def test_supports_session_auth(session_cls_mock) -> None: ), ) @mark_sync_test -def test_using_closed_driver_where_deprecated( +def test_using_closed_driver_where_forbidden( method_name, args, kwargs, session_cls_mock ) -> None: driver = GraphDatabase.driver("bolt://localhost") driver.close() method = getattr(driver, method_name) - with pytest.warns( - DeprecationWarning, - match="Using a driver after it has been closed is deprecated.", - ): - if inspect.iscoroutinefunction(method): - method(*args, **kwargs) - else: - method(*args, **kwargs) + with pytest.raises(DriverError, match="closed"): + Util.callback(method, *args, **kwargs) @pytest.mark.parametrize( ("method_name", "args", "kwargs"), (("close", (), {}),) ) @mark_sync_test -def test_using_closed_driver_where_not_deprecated( +def test_using_closed_driver_where_no_op( method_name, args, kwargs, session_cls_mock ) -> None: driver = GraphDatabase.driver("bolt://localhost") @@ -1353,7 +1351,4 @@ def test_using_closed_driver_where_not_deprecated( method = getattr(driver, method_name) with warnings.catch_warnings(): warnings.simplefilter("error") - if inspect.iscoroutinefunction(method): - method(*args, **kwargs) - else: - method(*args, **kwargs) + Util.callback(method, *args, **kwargs)