Skip to content

Commit 0145f62

Browse files
msrathore-dbclaude
andcommitted
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 - Keep base Error fallback with precise "closed connection"/"closed cursor" matching for older connector versions (v4.0.0-v4.0.5) that raise Error instead of InterfaceError - Expand tests to cover all connector error messages with exact code paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6936842 commit 0145f62

File tree

2 files changed

+137
-18
lines changed

2 files changed

+137
-18
lines changed

src/databricks/sqlalchemy/base.py

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -357,22 +357,35 @@ def is_disconnect(self, e, connection, cursor):
357357
Returns:
358358
True if the error indicates a disconnect, False otherwise
359359
"""
360-
from databricks.sql.exc import InterfaceError, DatabaseError, Error
360+
from databricks.sql.exc import (
361+
Error,
362+
InterfaceError,
363+
DatabaseError,
364+
RequestError,
365+
)
366+
367+
error_msg = str(e).lower()
361368

362-
# InterfaceError: Client-side errors (e.g., connection already closed)
369+
# InterfaceError: closed connection/cursor errors from client.py
370+
# All raised when self.open is False:
363371
if isinstance(e, InterfaceError):
372+
return "closed" in error_msg
373+
374+
# RequestError (subclass of DatabaseError via OperationalError):
375+
# transport/network-level errors indicating connection is unusable.
376+
# Check before DatabaseError since RequestError is a subclass.
377+
if isinstance(e, RequestError):
364378
return True
365379

366-
# DatabaseError: Server-side errors with invalid handle indicate session expired
380+
# DatabaseError: server-side errors indicating session/operation gone
367381
if isinstance(e, DatabaseError):
368-
error_msg = str(e).lower()
369-
return "invalid" in error_msg and "handle" in error_msg
382+
return ("invalid" in error_msg and "handle" in error_msg) or (
383+
"unexpectedly closed server side" in error_msg
384+
)
370385

371-
# Base Error class: Check for closed connection errors
372-
# This catches errors like "Cannot create cursor from closed connection"
386+
# Base Error class: older connector versions raise Error (not InterfaceError)
373387
if isinstance(e, Error):
374-
error_msg = str(e).lower()
375-
return "closed" in error_msg or "cannot create cursor" in error_msg
388+
return "closed connection" in error_msg or "closed cursor" in error_msg
376389

377390
return False
378391

Lines changed: 115 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,73 @@
11
"""Tests for DatabricksDialect.is_disconnect() method."""
22
import pytest
33
from databricks.sqlalchemy import DatabricksDialect
4-
from databricks.sql.exc import InterfaceError, DatabaseError, OperationalError
4+
from databricks.sql.exc import (
5+
Error,
6+
InterfaceError,
7+
DatabaseError,
8+
OperationalError,
9+
RequestError,
10+
SessionAlreadyClosedError,
11+
CursorAlreadyClosedError,
12+
MaxRetryDurationError,
13+
NonRecoverableNetworkError,
14+
UnsafeToRetryError,
15+
)
516

617

718
class TestIsDisconnect:
819
@pytest.fixture
920
def dialect(self):
1021
return DatabricksDialect()
1122

12-
def test_interface_error_is_disconnect(self, dialect):
13-
"""InterfaceError (client-side) is always a disconnect."""
14-
error = InterfaceError("Cannot create cursor from closed connection")
15-
assert dialect.is_disconnect(error, None, None) is True
23+
# --- InterfaceError: closed connection/cursor (client.py) ---
24+
25+
def test_interface_error_closed_connection(self, dialect):
26+
"""All InterfaceError messages with 'closed' are disconnects."""
27+
test_cases = [
28+
InterfaceError("Cannot create cursor from closed connection"),
29+
InterfaceError("Cannot get autocommit on closed connection"),
30+
InterfaceError("Cannot set autocommit on closed connection"),
31+
InterfaceError("Cannot commit on closed connection"),
32+
InterfaceError("Cannot rollback on closed connection"),
33+
InterfaceError("Cannot get transaction isolation on closed connection"),
34+
InterfaceError("Cannot set transaction isolation on closed connection"),
35+
InterfaceError("Attempting operation on closed cursor"),
36+
]
37+
for error in test_cases:
38+
assert dialect.is_disconnect(error, None, None) is True
39+
40+
def test_interface_error_without_closed_not_disconnect(self, dialect):
41+
"""InterfaceError without 'closed' is not a disconnect."""
42+
error = InterfaceError("Some other interface error")
43+
assert dialect.is_disconnect(error, None, None) is False
44+
45+
# --- RequestError: transport/network-level errors ---
46+
47+
def test_request_error_is_disconnect(self, dialect):
48+
"""All RequestError instances are disconnects."""
49+
test_cases = [
50+
RequestError("HTTP client is closing or has been closed"),
51+
RequestError("Connection pool not initialized"),
52+
RequestError("HTTP request failed: max retries exceeded"),
53+
RequestError("HTTP request error: connection reset"),
54+
]
55+
for error in test_cases:
56+
assert dialect.is_disconnect(error, None, None) is True
57+
58+
def test_request_error_subclasses_are_disconnect(self, dialect):
59+
"""RequestError subclasses are all disconnects."""
60+
test_cases = [
61+
SessionAlreadyClosedError("Session already closed"),
62+
CursorAlreadyClosedError("Cursor already closed"),
63+
MaxRetryDurationError("Retry duration exceeded"),
64+
NonRecoverableNetworkError("HTTP 501"),
65+
UnsafeToRetryError("Unexpected HTTP error"),
66+
]
67+
for error in test_cases:
68+
assert dialect.is_disconnect(error, None, None) is True
69+
70+
# --- DatabaseError: server-side session/operation errors ---
1671

1772
def test_database_error_with_invalid_handle(self, dialect):
1873
"""DatabaseError with 'invalid handle' is a disconnect."""
@@ -25,21 +80,72 @@ def test_database_error_with_invalid_handle(self, dialect):
2580
for error in test_cases:
2681
assert dialect.is_disconnect(error, None, None) is True
2782

28-
def test_database_error_without_invalid_handle(self, dialect):
29-
"""DatabaseError without 'invalid handle' is not a disconnect."""
83+
def test_database_error_unexpectedly_closed_server_side(self, dialect):
84+
"""DatabaseError for operations closed server-side is a disconnect."""
85+
test_cases = [
86+
DatabaseError("Command abc123 unexpectedly closed server side"),
87+
DatabaseError("Command None unexpectedly closed server side"),
88+
]
89+
for error in test_cases:
90+
assert dialect.is_disconnect(error, None, None) is True
91+
92+
def test_database_error_without_disconnect_indicators(self, dialect):
93+
"""DatabaseError without disconnect indicators is not a disconnect."""
3094
test_cases = [
3195
DatabaseError("Syntax error in SQL"),
3296
DatabaseError("Table not found"),
3397
DatabaseError("Permission denied"),
98+
DatabaseError("Catalog name is required for get_schemas"),
99+
DatabaseError("Catalog name is required for get_columns"),
34100
]
35101
for error in test_cases:
36102
assert dialect.is_disconnect(error, None, None) is False
37103

38-
def test_other_errors_not_disconnect(self, dialect):
39-
"""Other exception types are not disconnects."""
104+
# --- OperationalError (non-RequestError) ---
105+
106+
def test_operational_error_not_disconnect(self, dialect):
107+
"""OperationalError without disconnect indicators is not a disconnect."""
40108
test_cases = [
41109
OperationalError("Timeout waiting for query"),
110+
OperationalError("Empty TColumn instance"),
111+
OperationalError("Unsupported TRowSet instance"),
112+
]
113+
for error in test_cases:
114+
assert dialect.is_disconnect(error, None, None) is False
115+
116+
# --- Base Error class: older connector versions (client.py:385) ---
117+
118+
def test_base_error_closed_connection_is_disconnect(self, dialect):
119+
"""Base Error with 'closed connection/cursor' is a disconnect.
120+
121+
Older released versions of databricks-sql-connector raise Error
122+
(not InterfaceError) for closed connection messages.
123+
"""
124+
test_cases = [
125+
Error("Cannot create cursor from closed connection"),
126+
Error("Cannot get autocommit on closed connection"),
127+
Error("Attempting operation on closed cursor"),
128+
]
129+
for error in test_cases:
130+
assert dialect.is_disconnect(error, None, None) is True
131+
132+
def test_base_error_without_closed_not_disconnect(self, dialect):
133+
"""Base Error without 'closed connection/cursor' is not a disconnect."""
134+
test_cases = [
135+
Error("Some other error"),
136+
Error("Connection timeout"),
137+
]
138+
for error in test_cases:
139+
assert dialect.is_disconnect(error, None, None) is False
140+
141+
# --- Other exceptions ---
142+
143+
def test_other_errors_not_disconnect(self, dialect):
144+
"""Non-connector exception types are not disconnects."""
145+
test_cases = [
42146
Exception("Some random error"),
147+
ValueError("Bad value"),
148+
RuntimeError("Runtime failure"),
43149
]
44150
for error in test_cases:
45151
assert dialect.is_disconnect(error, None, None) is False

0 commit comments

Comments
 (0)