Improve is_disconnect error handling precision

- InterfaceError: check for "closed" in message instead of blanket True
- Add RequestError handling for transport/network-level disconnect errors
- Add "unexpectedly closed server side" to DatabaseError disconnect checks
- Remove overly broad Error base class catch with loose string matching
- Expand tests to cover all connector error messages with exact code paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: msrathore-db

src/databricks/sqlalchemy/base.py

@@ -357,22 +357,26 @@ def is_disconnect(self, e, connection, cursor):
         Returns:
             True if the error indicates a disconnect, False otherwise
         """
-        from databricks.sql.exc import InterfaceError, DatabaseError, Error
+        from databricks.sql.exc import InterfaceError, DatabaseError, RequestError
 
-        # InterfaceError: Client-side errors (e.g., connection already closed)
+        error_msg = str(e).lower()
+
+        # InterfaceError: closed connection/cursor errors from client.py
+        # All raised when self.open is False:
         if isinstance(e, InterfaceError):
+            return "closed" in error_msg
+
+        # RequestError (subclass of DatabaseError via OperationalError):
+        # transport/network-level errors indicating connection is unusable.
+        # Check before DatabaseError since RequestError is a subclass.
+        if isinstance(e, RequestError):
             return True
 
-        # DatabaseError: Server-side errors with invalid handle indicate session expired
+        # DatabaseError: server-side errors indicating session/operation gone
         if isinstance(e, DatabaseError):
-            error_msg = str(e).lower()
-            return "invalid" in error_msg and "handle" in error_msg
-
-        # Base Error class: Check for closed connection errors
-        # This catches errors like "Cannot create cursor from closed connection"
-        if isinstance(e, Error):
-            error_msg = str(e).lower()
-            return "closed" in error_msg or "cannot create cursor" in error_msg
+            return ("invalid" in error_msg and "handle" in error_msg) or (
+                "unexpectedly closed server side" in error_msg
+            )
 
         return False
 

tests/test_local/test_is_disconnect.py

@@ -1,18 +1,72 @@
 """Tests for DatabricksDialect.is_disconnect() method."""
 import pytest
 from databricks.sqlalchemy import DatabricksDialect
-from databricks.sql.exc import InterfaceError, DatabaseError, OperationalError
+from databricks.sql.exc import (
+    InterfaceError,
+    DatabaseError,
+    OperationalError,
+    RequestError,
+    SessionAlreadyClosedError,
+    CursorAlreadyClosedError,
+    MaxRetryDurationError,
+    NonRecoverableNetworkError,
+    UnsafeToRetryError,
+)
 
 
 class TestIsDisconnect:
     @pytest.fixture
     def dialect(self):
         return DatabricksDialect()
 
-    def test_interface_error_is_disconnect(self, dialect):
-        """InterfaceError (client-side) is always a disconnect."""
-        error = InterfaceError("Cannot create cursor from closed connection")
-        assert dialect.is_disconnect(error, None, None) is True
+    # --- InterfaceError: closed connection/cursor (client.py) ---
+
+    def test_interface_error_closed_connection(self, dialect):
+        """All InterfaceError messages with 'closed' are disconnects."""
+        test_cases = [
+            InterfaceError("Cannot create cursor from closed connection"),
+            InterfaceError("Cannot get autocommit on closed connection"),
+            InterfaceError("Cannot set autocommit on closed connection"),
+            InterfaceError("Cannot commit on closed connection"),
+            InterfaceError("Cannot rollback on closed connection"),
+            InterfaceError("Cannot get transaction isolation on closed connection"),
+            InterfaceError("Cannot set transaction isolation on closed connection"),
+            InterfaceError("Attempting operation on closed cursor"),
+        ]
+        for error in test_cases:
+            assert dialect.is_disconnect(error, None, None) is True
+
+    def test_interface_error_without_closed_not_disconnect(self, dialect):
+        """InterfaceError without 'closed' is not a disconnect."""
+        error = InterfaceError("Some other interface error")
+        assert dialect.is_disconnect(error, None, None) is False
+
+    # --- RequestError: transport/network-level errors ---
+
+    def test_request_error_is_disconnect(self, dialect):
+        """All RequestError instances are disconnects."""
+        test_cases = [
+            RequestError("HTTP client is closing or has been closed"),
+            RequestError("Connection pool not initialized"),
+            RequestError("HTTP request failed: max retries exceeded"),
+            RequestError("HTTP request error: connection reset"),
+        ]
+        for error in test_cases:
+            assert dialect.is_disconnect(error, None, None) is True
+
+    def test_request_error_subclasses_are_disconnect(self, dialect):
+        """RequestError subclasses are all disconnects."""
+        test_cases = [
+            SessionAlreadyClosedError("Session already closed"),
+            CursorAlreadyClosedError("Cursor already closed"),
+            MaxRetryDurationError("Retry duration exceeded"),
+            NonRecoverableNetworkError("HTTP 501"),
+            UnsafeToRetryError("Unexpected HTTP error"),
+        ]
+        for error in test_cases:
+            assert dialect.is_disconnect(error, None, None) is True
+
+    # --- DatabaseError: server-side session/operation errors ---
 
     def test_database_error_with_invalid_handle(self, dialect):
         """DatabaseError with 'invalid handle' is a disconnect."""
@@ -25,21 +79,47 @@ def test_database_error_with_invalid_handle(self, dialect):
         for error in test_cases:
             assert dialect.is_disconnect(error, None, None) is True
 
-    def test_database_error_without_invalid_handle(self, dialect):
-        """DatabaseError without 'invalid handle' is not a disconnect."""
+    def test_database_error_unexpectedly_closed_server_side(self, dialect):
+        """DatabaseError for operations closed server-side is a disconnect."""
+        test_cases = [
+            DatabaseError("Command abc123 unexpectedly closed server side"),
+            DatabaseError("Command None unexpectedly closed server side"),
+        ]
+        for error in test_cases:
+            assert dialect.is_disconnect(error, None, None) is True
+
+    def test_database_error_without_disconnect_indicators(self, dialect):
+        """DatabaseError without disconnect indicators is not a disconnect."""
         test_cases = [
             DatabaseError("Syntax error in SQL"),
             DatabaseError("Table not found"),
             DatabaseError("Permission denied"),
+            DatabaseError("Catalog name is required for get_schemas"),
+            DatabaseError("Catalog name is required for get_columns"),
         ]
         for error in test_cases:
             assert dialect.is_disconnect(error, None, None) is False
 
-    def test_other_errors_not_disconnect(self, dialect):
-        """Other exception types are not disconnects."""
+    # --- OperationalError (non-RequestError) ---
+
+    def test_operational_error_not_disconnect(self, dialect):
+        """OperationalError without disconnect indicators is not a disconnect."""
         test_cases = [
             OperationalError("Timeout waiting for query"),
+            OperationalError("Empty TColumn instance"),
+            OperationalError("Unsupported TRowSet instance"),
+        ]
+        for error in test_cases:
+            assert dialect.is_disconnect(error, None, None) is False
+
+    # --- Other exceptions ---
+
+    def test_other_errors_not_disconnect(self, dialect):
+        """Non-connector exception types are not disconnects."""
+        test_cases = [
             Exception("Some random error"),
+            ValueError("Bad value"),
+            RuntimeError("Runtime failure"),
         ]
         for error in test_cases:
             assert dialect.is_disconnect(error, None, None) is False