From: Daniele Varrazzo <daniele.varrazzo@gmail.com>
Date: Wed, 10 Apr 2024 20:16:47 +0000 (+0200)
Subject: refactor: drop _try_cancel internal method.
X-Git-Tag: 3.2.0~44^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=46f661dbbc30808d11a71c0ae7bbfc8245b6fd42;p=thirdparty%2Fpsycopg.git

refactor: drop _try_cancel internal method.

This method was useful before introducing cancel_safe, which is now the
function of choice for internal cancelling.

Also refactor the exception handling to account for possible errors in
`PGcancel.cancel()`, not only in `PGconn.get_cancel()`, to make sure to
not clobber an exception bubbling up with ours, whatever happens to the
underlying connection.
---

diff --git a/docs/api/connections.rst b/docs/api/connections.rst
index a6d95d20b..0f9462ce2 100644
--- a/docs/api/connections.rst
+++ b/docs/api/connections.rst
@@ -303,7 +303,9 @@ The `!Connection` class
 
         .. warning::
 
-            The `!cancel()` method has a few shortcomings:
+            The `!cancel()` method is implemented using the :pq:`PQcancel`
+            function, which is deprecated since PostgreSQL 17, and has a few
+            shortcomings:
 
             - it is blocking even on async connections,
             - it `might use an insecure connection`__ even if the original
diff --git a/psycopg/psycopg/_connection_base.py b/psycopg/psycopg/_connection_base.py
index 4fdda67b5..4944f1d31 100644
--- a/psycopg/psycopg/_connection_base.py
+++ b/psycopg/psycopg/_connection_base.py
@@ -293,8 +293,16 @@ class BaseConnection(Generic[Row]):
 
     def cancel(self) -> None:
         """Cancel the current operation on the connection."""
-        if self._should_cancel():
-            self._try_cancel(self.pgconn)
+        if not self._should_cancel():
+            return
+
+        # Don't fail cancelling (which might happen on connection closing) to
+        # avoid clobbering eventual exceptions with ours, which is less important.
+        try:
+            c = self.pgconn.get_cancel()
+            c.cancel()
+        except Exception as ex:
+            logger.warning("couldn't try to cancel query: %s", str(ex))
 
     def _should_cancel(self) -> bool:
         """Check whether the current command should actually be cancelled when
@@ -311,19 +319,6 @@ class BaseConnection(Generic[Row]):
             )
         return True
 
-    @classmethod
-    def _try_cancel(cls, pgconn: "PGconn") -> None:
-        """Try to cancel the current command using a PGcancel object,
-        which is deprecated since libpq 17.
-        """
-        try:
-            # Can fail if the connection is closed
-            c = pgconn.get_cancel()
-        except Exception as ex:
-            logger.warning("couldn't try to cancel query: %s", ex)
-        else:
-            c.cancel()
-
     def _cancel_gen(self) -> PQGenConn[None]:
         try:
             cancel_conn = self.pgconn.cancel_conn()