Skip to content

Commit a9ad295

Browse files
committed
Address review: consolidate ping(), handle timeout, dedup reconnect dialogs
- Merge connection_ping() into the existing ping(): adds the transaction_status guard and close-on-failure recovery, returns bool. Removes the redundant abstract method and updates callers. - Treat axios ECONNABORTED (client-side timeout, no error.response) as a connection-lost signal so the reconnect dialog still triggers when the server hangs on a recently-dead socket. - Add a per-server _reconnectPending guard so concurrent failed child-load requests do not stack multiple reconnect dialogs; the flag is cleared on both OK and Cancel.
1 parent cef5b02 commit a9ad295

4 files changed

Lines changed: 82 additions & 71 deletions

File tree

web/pgadmin/browser/utils.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -433,12 +433,12 @@ def children(self, **kwargs):
433433

434434
try:
435435
conn = manager.connection(did=did)
436-
# Use connection_ping() instead of connected() to detect
437-
# stale / half-open TCP connections that were silently
438-
# dropped while pgAdmin was idle. connected() only checks
439-
# local state and would miss these, causing the subsequent
440-
# SQL queries to hang indefinitely.
441-
if not conn.connection_ping():
436+
# Use ping() instead of connected() to detect stale /
437+
# half-open TCP connections that were silently dropped while
438+
# pgAdmin was idle. connected() only checks local state and
439+
# would miss these, causing the subsequent SQL queries to
440+
# hang indefinitely.
441+
if not conn.ping():
442442
status, msg = conn.connect()
443443
if not status:
444444
return service_unavailable(

web/pgadmin/static/js/tree/tree_nodes.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,15 @@ export class ManageTreeNodes {
126126
} catch (error) {
127127
/* react-aspen does not handle reject case */
128128
console.error(error);
129-
if (error.response?.status === 503 &&
130-
error.response?.data?.info === 'CONNECTION_LOST') {
129+
const isConnectionLost =
130+
(error.response?.status === 503 &&
131+
error.response?.data?.info === 'CONNECTION_LOST') ||
132+
// Axios client-side timeout has no error.response — treat it
133+
// as a likely connection loss so the reconnect dialog still
134+
// triggers when the server hangs on a recently-dead socket
135+
// (before TCP keepalives detect it).
136+
error.code === 'ECONNABORTED';
137+
if (isConnectionLost) {
131138
// Connection dropped while idle. Walk up to the server node
132139
// and mark it disconnected, then show a reconnect prompt so
133140
// the user can re-establish instead of seeing a silent
@@ -138,24 +145,38 @@ export class ManageTreeNodes {
138145
if (d?._type === 'server') break;
139146
serverNode = serverNode.parentNode ?? null;
140147
}
148+
const sData = serverNode
149+
? (serverNode.metadata?.data ?? serverNode.data)
150+
: null;
151+
// When a server has multiple expanded children, every in-flight
152+
// child-load request will fail independently. Guard with a
153+
// per-server flag so we only show one reconnect dialog at a
154+
// time instead of stacking them.
155+
if (sData?._reconnectPending) return [];
141156
if (serverNode) {
142-
const sData = serverNode.metadata?.data ?? serverNode.data;
143-
if (sData) sData.connected = false;
157+
if (sData) {
158+
sData.connected = false;
159+
sData._reconnectPending = true;
160+
}
144161
pgAdmin.Browser.tree?.addIcon(serverNode, {icon: 'icon-server-not-connected'});
145162
pgAdmin.Browser.tree?.close(serverNode);
146163
}
164+
const clearPending = () => {
165+
if (sData) sData._reconnectPending = false;
166+
};
147167
pgAdmin.Browser.notifier.confirm(
148168
gettext('Connection lost'),
149169
gettext('The connection to the server has been lost. Would you like to reconnect?'),
150170
function() {
171+
clearPending();
151172
// Re-open (connect) the server node in the tree which
152173
// will trigger the standard connect-to-server flow
153174
// including any password prompts.
154175
if (serverNode && pgAdmin.Browser.tree) {
155176
pgAdmin.Browser.tree.toggle(serverNode);
156177
}
157178
},
158-
function() { /* cancelled */ }
179+
clearPending,
159180
);
160181
} else {
161182
pgAdmin.Browser.notifier.error(parseApiError(error)||'Node Load Error...');

web/pgadmin/utils/driver/abstract.py

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,8 @@ class BaseConnection()
116116
- Implement this method to get the status of the connection. It should
117117
return True for connected, otherwise False. This is a local check
118118
only (e.g. inspecting driver-level state) and may not detect
119-
server-side disconnects. Use connection_ping() when a network-level
120-
check is required.
121-
122-
* connection_ping()
123-
- Implement this method to verify the connection is alive by sending a
124-
lightweight query (e.g. SELECT 1) to the server. Returns True if the
125-
server responds, False otherwise. Unlike connected(), this detects
126-
stale or half-open TCP connections that were silently dropped.
119+
server-side disconnects. Use ping() when a network-level check is
120+
required.
127121
128122
* reset()
129123
- Implement this method to reconnect the database server (if possible)
@@ -133,9 +127,13 @@ class BaseConnection()
133127
connection. Range of return values different for each driver type.
134128
135129
* ping()
136-
- Implement this method to ping the server. There are times, a connection
137-
has been lost, but - the connection driver does not know about it. This
138-
can be helpful to figure out the actual reason for query failure.
130+
- Implement this method to verify the connection is alive by sending a
131+
lightweight query (e.g. SELECT 1) to the server. Returns True if the
132+
server responds, False otherwise. Unlike connected(), this detects
133+
stale or half-open TCP connections that were silently dropped. When
134+
a query is already in progress or the connection is inside a
135+
transaction block, the probe is skipped and True is returned (the
136+
connection is evidently alive).
139137
140138
* _release()
141139
- Implement this method to release the connection object. This should not
@@ -216,14 +214,6 @@ def async_fetchmany_2darray(self, records=-1,
216214
def connected(self):
217215
pass
218216

219-
@abstractmethod
220-
def connection_ping(self):
221-
"""
222-
Check if the connection is actually alive by sending a lightweight
223-
query to the server. Returns True if alive, False otherwise.
224-
"""
225-
pass
226-
227217
@abstractmethod
228218
def reset(self):
229219
pass
@@ -234,6 +224,10 @@ def transaction_status(self):
234224

235225
@abstractmethod
236226
def ping(self):
227+
"""
228+
Check if the connection is actually alive by sending a lightweight
229+
query to the server. Returns True if alive, False otherwise.
230+
"""
237231
pass
238232

239233
@abstractmethod

web/pgadmin/utils/driver/psycopg3/connection.py

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,46 +1413,6 @@ def connected(self):
14131413
self.conn = None
14141414
return False
14151415

1416-
def connection_ping(self):
1417-
"""
1418-
Check if the connection is actually alive by executing a lightweight
1419-
query. Unlike connected(), which only inspects local state, this
1420-
sends traffic to the server and will detect stale / half-open TCP
1421-
connections that were silently dropped by firewalls or the OS while
1422-
pgAdmin was idle.
1423-
1424-
Returns True if alive, False otherwise.
1425-
"""
1426-
if not self.connected():
1427-
return False
1428-
1429-
try:
1430-
# Check the transaction status before executing the ping
1431-
# query. If a query is already in progress (ACTIVE) or we
1432-
# are inside a transaction block (INTRANS / INERROR), running
1433-
# SELECT 1 would fail or disrupt the ongoing operation. In
1434-
# those states the connection is evidently alive, so just
1435-
# return True.
1436-
txn_status = self.conn.info.transaction_status
1437-
if txn_status != 0:
1438-
# 0 = IDLE — safe to send a query
1439-
# 1 = ACTIVE — command in progress, connection is alive
1440-
# 2 = INTRANS — in transaction block, connection is alive
1441-
# 3 = INERROR — in failed transaction, connection is alive
1442-
return True
1443-
1444-
cur = self.conn.cursor()
1445-
cur.execute("SELECT 1")
1446-
cur.close()
1447-
return True
1448-
except Exception:
1449-
try:
1450-
self.conn.close()
1451-
except Exception:
1452-
pass
1453-
self.conn = None
1454-
return False
1455-
14561416
def _decrypt_password(self, manager):
14571417
"""
14581418
Decrypt password
@@ -1526,7 +1486,43 @@ def async_query_error(self):
15261486
return self.__async_query_error
15271487

15281488
def ping(self):
1529-
return self.execute_scalar('SELECT 1')
1489+
"""
1490+
Check if the connection is actually alive by executing a lightweight
1491+
query. Unlike connected(), which only inspects local state, this
1492+
sends traffic to the server and will detect stale / half-open TCP
1493+
connections that were silently dropped by firewalls or the OS while
1494+
pgAdmin was idle.
1495+
1496+
Returns True if alive, False otherwise.
1497+
"""
1498+
if not self.connected():
1499+
return False
1500+
1501+
try:
1502+
# Check the transaction status before executing the ping
1503+
# query. If a query is already in progress (ACTIVE) or we
1504+
# are inside a transaction block (INTRANS / INERROR), running
1505+
# SELECT 1 would fail or disrupt the ongoing operation. In
1506+
# those states the connection is evidently alive, so just
1507+
# return True.
1508+
# 0 = IDLE — safe to send a query
1509+
# 1 = ACTIVE — command in progress, connection is alive
1510+
# 2 = INTRANS — in transaction block, connection is alive
1511+
# 3 = INERROR — in failed transaction, connection is alive
1512+
if self.conn.info.transaction_status != 0:
1513+
return True
1514+
1515+
cur = self.conn.cursor()
1516+
cur.execute("SELECT 1")
1517+
cur.close()
1518+
return True
1519+
except Exception:
1520+
try:
1521+
self.conn.close()
1522+
except Exception:
1523+
pass
1524+
self.conn = None
1525+
return False
15301526

15311527
def _release(self):
15321528
if self.wasConnected:

0 commit comments

Comments
 (0)