diff --git a/CHANGELOG.md b/CHANGELOG.md index a70bf7529..2c7cd93c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Cancelling a pending connection no longer lets the abandoned attempt overwrite or drop a later successful connection to the same database (#1358) - Importing connections from DBeaver now brings over the username (#1355) ## [0.43.1] - 2026-05-20 diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift index 8b102ef50..730a6baad 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift @@ -153,7 +153,7 @@ final class LibPQPluginConnection: @unchecked Sendable { // MARK: - Connection Management func connect() async throws { - try await pluginDispatchAsync(on: queue) { [self] in + try await pluginDispatchAsyncCancellable(on: queue) { [self] in func escapeConnParam(_ value: String) -> String { value.replacingOccurrences(of: "\\", with: "\\\\") .replacingOccurrences(of: "'", with: "\\'") diff --git a/TablePro/Core/Database/DatabaseManager+Sessions.swift b/TablePro/Core/Database/DatabaseManager+Sessions.swift index ce54d5b6c..3bb0ace24 100644 --- a/TablePro/Core/Database/DatabaseManager+Sessions.swift +++ b/TablePro/Core/Database/DatabaseManager+Sessions.swift @@ -40,8 +40,7 @@ extension DatabaseManager { do { effectiveConnection = try await buildEffectiveConnection(for: resolvedConnection) } catch { - removeSessionEntry(for: connection.id) - currentSessionId = nil + finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled) throw error } @@ -51,8 +50,7 @@ extension DatabaseManager { do { try await PreConnectHookRunner.run(script: script) } catch { - removeSessionEntry(for: connection.id) - currentSessionId = nil + finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled) throw error } } @@ -68,8 +66,7 @@ extension DatabaseManager { isAPIToken: isApiOnly, window: NSApp.keyWindow ) else { - removeSessionEntry(for: connection.id) - currentSessionId = nil + finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled) throw CancellationError() } passwordOverride = prompted @@ -84,7 +81,7 @@ extension DatabaseManager { awaitPlugins: true ) } catch { - if connection.resolvedSSHConfig.enabled { + if !Task.isCancelled, connection.resolvedSSHConfig.enabled { Task { do { try await SSHTunnelManager.shared.closeTunnel(connectionId: connection.id) @@ -93,13 +90,13 @@ extension DatabaseManager { } } } - removeSessionEntry(for: connection.id) - currentSessionId = nil + finalizeConnectionFailure(for: connection.id, cancelled: Task.isCancelled) throw error } do { try await driver.connect() + try Task.checkCancellation() let timeoutSeconds = AppSettingsManager.shared.general.queryTimeoutSeconds do { @@ -122,6 +119,8 @@ extension DatabaseManager { for: connection, resolvedConnection: resolvedConnection, driver: driver ) + try Task.checkCancellation() + // Batch all session mutations into a single write to fire objectWillChange once. if var session = activeSessions[connection.id] { session.driver = driver @@ -144,7 +143,10 @@ extension DatabaseManager { await startHealthMonitor(for: connection.id) } } catch { - if connection.resolvedSSHConfig.enabled { + let cancelled = Task.isCancelled + if cancelled { + driver.disconnect() + } else if connection.resolvedSSHConfig.enabled { Task { do { try await SSHTunnelManager.shared.closeTunnel(connectionId: connection.id) @@ -154,21 +156,19 @@ extension DatabaseManager { } } - // Remove failed session completely so UI returns to Welcome window. - removeSessionEntry(for: connection.id) - - if currentSessionId == connection.id { - if let nextSessionId = activeSessions.keys.first { - currentSessionId = nextSessionId - } else { - currentSessionId = nil - } - } - + finalizeConnectionFailure(for: connection.id, cancelled: cancelled) throw error } } + internal func finalizeConnectionFailure(for connectionId: UUID, cancelled: Bool) { + guard !cancelled else { return } + removeSessionEntry(for: connectionId) + if currentSessionId == connectionId { + currentSessionId = activeSessions.keys.first + } + } + private func executePostConnectActions( for connection: DatabaseConnection, resolvedConnection: DatabaseConnection, diff --git a/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift b/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift new file mode 100644 index 000000000..ee8d1f886 --- /dev/null +++ b/TableProTests/Core/Database/CancelledConnectionCleanupTests.swift @@ -0,0 +1,105 @@ +// +// CancelledConnectionCleanupTests.swift +// TableProTests +// +// Pins the fix for #1358: a cancelled connection attempt must not tear down the +// shared session entry, which after a cancel + retry belongs to the new attempt. +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +@Suite("Cancelled connection cleanup", .serialized) +@MainActor +struct CancelledConnectionCleanupTests { + @Test("Cancelled attempt leaves the session entry intact") + func cancelledLeavesSessionIntact() { + let id = UUID() + DatabaseManager.shared.injectSession( + ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Retry")), + for: id + ) + defer { DatabaseManager.shared.removeSession(for: id) } + + DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: true) + + #expect(DatabaseManager.shared.activeSessions[id] != nil) + } + + @Test("Genuine failure removes the session entry") + func genuineFailureRemovesSession() { + let id = UUID() + DatabaseManager.shared.injectSession( + ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Failed")), + for: id + ) + defer { DatabaseManager.shared.removeSession(for: id) } + + DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: false) + + #expect(DatabaseManager.shared.activeSessions[id] == nil) + } + + @Test("Cancelled finalize keeps currentSessionId untouched") + func cancelledKeepsCurrentSessionId() { + let id = UUID() + DatabaseManager.shared.injectSession( + ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Retry")), + for: id + ) + DatabaseManager.shared.currentSessionId = id + defer { + DatabaseManager.shared.removeSession(for: id) + DatabaseManager.shared.currentSessionId = nil + } + + DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: true) + + #expect(DatabaseManager.shared.currentSessionId == id) + } + + @Test("Genuine failure clears currentSessionId when no other session remains") + func genuineFailureClearsCurrentSessionId() { + let id = UUID() + DatabaseManager.shared.injectSession( + ConnectionSession(connection: TestFixtures.makeConnection(id: id, name: "Failed")), + for: id + ) + DatabaseManager.shared.currentSessionId = id + defer { + DatabaseManager.shared.removeSession(for: id) + DatabaseManager.shared.currentSessionId = nil + } + + DatabaseManager.shared.finalizeConnectionFailure(for: id, cancelled: false) + + #expect(DatabaseManager.shared.currentSessionId != id) + } + + @Test("Genuine failure moves currentSessionId to a remaining session") + func genuineFailureSwitchesToRemainingSession() { + let failedId = UUID() + let otherId = UUID() + DatabaseManager.shared.injectSession( + ConnectionSession(connection: TestFixtures.makeConnection(id: failedId, name: "Failed")), + for: failedId + ) + DatabaseManager.shared.injectSession( + ConnectionSession(connection: TestFixtures.makeConnection(id: otherId, name: "Other")), + for: otherId + ) + DatabaseManager.shared.currentSessionId = failedId + defer { + DatabaseManager.shared.removeSession(for: failedId) + DatabaseManager.shared.removeSession(for: otherId) + DatabaseManager.shared.currentSessionId = nil + } + + DatabaseManager.shared.finalizeConnectionFailure(for: failedId, cancelled: false) + + #expect(DatabaseManager.shared.currentSessionId == otherId) + #expect(DatabaseManager.shared.activeSessions[otherId] != nil) + } +}