Skip to content

Commit 07da2bf

Browse files
datlechindatlechin
authored
fix(connections): cancelled connect no longer aborts a later successful connection (#1358) (#1367)
* fix(connections): cancelled connect no longer aborts a later successful connection (#1358) * refactor(connections): explicit ACL on finalizeConnectionFailure, fix changelog spelling, add session-switch test --------- Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
1 parent 1f5f8cb commit 07da2bf

4 files changed

Lines changed: 128 additions & 22 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- Cancelling a pending connection no longer lets the abandoned attempt overwrite or drop a later successful connection to the same database (#1358)
1213
- Importing connections from DBeaver now brings over the username (#1355)
1314

1415
## [0.43.1] - 2026-05-20

‎Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ final class LibPQPluginConnection: @unchecked Sendable {
153153
// MARK: - Connection Management
154154

155155
func connect() async throws {
156-
try await pluginDispatchAsync(on: queue) { [self] in
156+
try await pluginDispatchAsyncCancellable(on: queue) { [self] in
157157
func escapeConnParam(_ value: String) -> String {
158158
value.replacingOccurrences(of: "\\", with: "\\\\")
159159
.replacingOccurrences(of: "'", with: "\\'")

‎TablePro/Core/Database/DatabaseManager+Sessions.swift‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ extension DatabaseManager {
4040
do {
4141
effectiveConnection = try await buildEffectiveConnection(for: resolvedConnection)
4242
} catch {
43-
removeSessionEntry(for: connection.id)
44-
currentSessionId = nil
43+
finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled)
4544
throw error
4645
}
4746

@@ -51,8 +50,7 @@ extension DatabaseManager {
5150
do {
5251
try await PreConnectHookRunner.run(script: script)
5352
} catch {
54-
removeSessionEntry(for: connection.id)
55-
currentSessionId = nil
53+
finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled)
5654
throw error
5755
}
5856
}
@@ -68,8 +66,7 @@ extension DatabaseManager {
6866
isAPIToken: isApiOnly,
6967
window: NSApp.keyWindow
7068
) else {
71-
removeSessionEntry(for: connection.id)
72-
currentSessionId = nil
69+
finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled)
7370
throw CancellationError()
7471
}
7572
passwordOverride = prompted
@@ -84,7 +81,7 @@ extension DatabaseManager {
8481
awaitPlugins: true
8582
)
8683
} catch {
87-
if connection.resolvedSSHConfig.enabled {
84+
if !Task.isCancelled, connection.resolvedSSHConfig.enabled {
8885
Task {
8986
do {
9087
try await SSHTunnelManager.shared.closeTunnel(connectionId: connection.id)
@@ -93,13 +90,13 @@ extension DatabaseManager {
9390
}
9491
}
9592
}
96-
removeSessionEntry(for: connection.id)
97-
currentSessionId = nil
93+
finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled)
9894
throw error
9995
}
10096

10197
do {
10298
try await driver.connect()
99+
try Task.checkCancellation()
103100

104101
let timeoutSeconds = AppSettingsManager.shared.general.queryTimeoutSeconds
105102
do {
@@ -122,6 +119,8 @@ extension DatabaseManager {
122119
for: connection, resolvedConnection: resolvedConnection, driver: driver
123120
)
124121

122+
try Task.checkCancellation()
123+
125124
// Batch all session mutations into a single write to fire objectWillChange once.
126125
if var session = activeSessions[connection.id] {
127126
session.driver = driver
@@ -144,7 +143,10 @@ extension DatabaseManager {
144143
await startHealthMonitor(for: connection.id)
145144
}
146145
} catch {
147-
if connection.resolvedSSHConfig.enabled {
146+
let cancelled = Task.isCancelled
147+
if cancelled {
148+
driver.disconnect()
149+
} else if connection.resolvedSSHConfig.enabled {
148150
Task {
149151
do {
150152
try await SSHTunnelManager.shared.closeTunnel(connectionId: connection.id)
@@ -154,21 +156,19 @@ extension DatabaseManager {
154156
}
155157
}
156158

157-
// Remove failed session completely so UI returns to Welcome window.
158-
removeSessionEntry(for: connection.id)
159-
160-
if currentSessionId == connection.id {
161-
if let nextSessionId = activeSessions.keys.first {
162-
currentSessionId = nextSessionId
163-
} else {
164-
currentSessionId = nil
165-
}
166-
}
167-
159+
finalizeConnectionFailure(for: connection.id, cancelled: cancelled)
168160
throw error
169161
}
170162
}
171163

164+
internal func finalizeConnectionFailure(for connectionId: UUID, cancelled: Bool) {
165+
guard !cancelled else { return }
166+
removeSessionEntry(for: connectionId)
167+
if currentSessionId == connectionId {
168+
currentSessionId = activeSessions.keys.first
169+
}
170+
}
171+
172172
private func executePostConnectActions(
173173
for connection: DatabaseConnection,
174174
resolvedConnection: DatabaseConnection,

‎TableProTests/Core/Database/CancelledConnectionCleanupTests.swift‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
//
2+
// CancelledConnectionCleanupTests.swift
3+
// TableProTests
4+
//
5+
// Pins the fix for #1358: a cancelled connection attempt must not tear down the
6+
// shared session entry, which after a cancel + retry belongs to the new attempt.
7+
//
8+
9+
import Foundation
10+
@testable import TablePro
11+
import TableProPluginKit
12+
import Testing
13+
14+
@Suite("Cancelled connection cleanup", .serialized)
15+
@MainActor
16+
struct CancelledConnectionCleanupTests {
17+
@Test("Cancelled attempt leaves the session entry intact")
18+
func cancelledLeavesSessionIntact() {
19+
let id = UUID()
20+
DatabaseManager.shared.injectSession(
21+
ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Retry")),
22+
for: id
23+
)
24+
defer { DatabaseManager.shared.removeSession(for: id) }
25+
26+
DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: true)
27+
28+
#expect(DatabaseManager.shared.activeSessions[id] != nil)
29+
}
30+
31+
@Test("Genuine failure removes the session entry")
32+
func genuineFailureRemovesSession() {
33+
let id = UUID()
34+
DatabaseManager.shared.injectSession(
35+
ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Failed")),
36+
for: id
37+
)
38+
defer { DatabaseManager.shared.removeSession(for: id) }
39+
40+
DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: false)
41+
42+
#expect(DatabaseManager.shared.activeSessions[id] == nil)
43+
}
44+
45+
@Test("Cancelled finalize keeps currentSessionId untouched")
46+
func cancelledKeepsCurrentSessionId() {
47+
let id = UUID()
48+
DatabaseManager.shared.injectSession(
49+
ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Retry")),
50+
for: id
51+
)
52+
DatabaseManager.shared.currentSessionId = id
53+
defer {
54+
DatabaseManager.shared.removeSession(for: id)
55+
DatabaseManager.shared.currentSessionId = nil
56+
}
57+
58+
DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: true)
59+
60+
#expect(DatabaseManager.shared.currentSessionId == id)
61+
}
62+
63+
@Test("Genuine failure clears currentSessionId when no other session remains")
64+
func genuineFailureClearsCurrentSessionId() {
65+
let id = UUID()
66+
DatabaseManager.shared.injectSession(
67+
ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Failed")),
68+
for: id
69+
)
70+
DatabaseManager.shared.currentSessionId = id
71+
defer {
72+
DatabaseManager.shared.removeSession(for: id)
73+
DatabaseManager.shared.currentSessionId = nil
74+
}
75+
76+
DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: false)
77+
78+
#expect(DatabaseManager.shared.currentSessionId != id)
79+
}
80+
81+
@Test("Genuine failure moves currentSessionId to a remaining session")
82+
func genuineFailureSwitchesToRemainingSession() {
83+
let failedId = UUID()
84+
let otherId = UUID()
85+
DatabaseManager.shared.injectSession(
86+
ConnectionSession(connection: TestFixtures.makeConnection(id: failedId, name: "Failed")),
87+
for: failedId
88+
)
89+
DatabaseManager.shared.injectSession(
90+
ConnectionSession(connection: TestFixtures.makeConnection(id: otherId, name: "Other")),
91+
for: otherId
92+
)
93+
DatabaseManager.shared.currentSessionId = failedId
94+
defer {
95+
DatabaseManager.shared.removeSession(for: failedId)
96+
DatabaseManager.shared.removeSession(for: otherId)
97+
DatabaseManager.shared.currentSessionId = nil
98+
}
99+
100+
DatabaseManager.shared.finalizeConnectionFailure(for: failedId, cancelled: false)
101+
102+
#expect(DatabaseManager.shared.currentSessionId == otherId)
103+
#expect(DatabaseManager.shared.activeSessions[otherId] != nil)
104+
}
105+
}

0 commit comments

Comments
 (0)