From 5b86b2e4c2a6f190b32c9969f54540cbb5951348 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Fri, 10 Jul 2026 11:59:09 +0300 Subject: [PATCH] fix(connections): improve PostgreSQL connection UX and error typing (#255, #256, #259) - Extract host and port from the connection URI so URI-only PostgreSQL rows show meaningful metadata in the sidebar. - Throw PostgresConnectionException from connect() with original cause and stack trace preserved. - Wrap unexpected factory failures in PostgresConnectionPool.acquire and rethrow typed PostgreSQL exceptions unchanged. - Update tests for exception fields, typed connect errors, pool wrapping, and URI-derived display metadata. --- lib/core/database/postgres_connection.dart | 23 ++++++++-- .../database/postgres_connection_pool.dart | 21 +++++++-- .../postgresql_connection_form.dart | 19 ++++++-- .../postgres_connection_pool_test.dart | 46 +++++++++++++++++++ .../database/postgres_connection_test.dart | 33 +++++++++++++ .../postgresql_connection_form_test.dart | 39 ++++++++++++++++ 6 files changed, 171 insertions(+), 10 deletions(-) diff --git a/lib/core/database/postgres_connection.dart b/lib/core/database/postgres_connection.dart index fe76ee13..8aae9434 100644 --- a/lib/core/database/postgres_connection.dart +++ b/lib/core/database/postgres_connection.dart @@ -137,10 +137,17 @@ class PostgresConnection { ); } _isConnected = true; - } catch (e) { + } catch (e, st) { _isConnected = false; _conn = null; - rethrow; + Error.throwWithStackTrace( + PostgresConnectionException( + 'Failed to connect to PostgreSQL${name.isNotEmpty ? ' ($name)' : ''}: $e', + cause: e, + stackTrace: st, + ), + st, + ); } } @@ -183,6 +190,8 @@ class PostgresConnection { return (ok: true, error: null); } return (ok: false, error: 'Connection could not be established.'); + } on PostgresConnectionException catch (e) { + return (ok: false, error: e.message); } catch (e) { return (ok: false, error: e.toString()); } finally { @@ -770,8 +779,16 @@ class PostgresSequenceDetails { } class PostgresConnectionException implements Exception { - PostgresConnectionException(this.message); + PostgresConnectionException( + this.message, { + this.cause, + this.stackTrace, + }); + final String message; + final Object? cause; + final StackTrace? stackTrace; + @override String toString() => message; } diff --git a/lib/core/database/postgres_connection_pool.dart b/lib/core/database/postgres_connection_pool.dart index 73c71433..e4fa225d 100644 --- a/lib/core/database/postgres_connection_pool.dart +++ b/lib/core/database/postgres_connection_pool.dart @@ -87,10 +87,23 @@ class PostgresConnectionPool { _evictIfNeededBeforeNewSlot(); - final conn = await createAndConnect(row, database: database, mode: mode); - entry = _PoolEntry(conn)..refs = 1; - _pool[k] = entry; - return PgLease._(this, k, conn); + try { + final conn = await createAndConnect(row, database: database, mode: mode); + entry = _PoolEntry(conn)..refs = 1; + _pool[k] = entry; + return PgLease._(this, k, conn); + } on PostgresConnectionException { + rethrow; + } catch (e, st) { + Error.throwWithStackTrace( + PostgresConnectionException( + 'Failed to acquire PostgreSQL connection for database "$database": $e', + cause: e, + stackTrace: st, + ), + st, + ); + } } /// Drops idle LRU slots until there is room for one more key. diff --git a/lib/features/postgresql/postgresql_connection_form.dart b/lib/features/postgresql/postgresql_connection_form.dart index 8cdd2303..2628e0ad 100644 --- a/lib/features/postgresql/postgresql_connection_form.dart +++ b/lib/features/postgresql/postgresql_connection_form.dart @@ -145,16 +145,29 @@ class _PostgresConnectionFormContentState final port = int.tryParse(_portController.text.trim()) ?? 5432; final database = _databaseController.text.trim(); final uri = _connectionStringController.text.trim(); + + String? uriHost; + int? uriPort; + if (uri.isNotEmpty) { + final parsedUri = Uri.tryParse(uri); + if (parsedUri != null && parsedUri.host.isNotEmpty) { + uriHost = parsedUri.host; + uriPort = parsedUri.hasPort ? parsedUri.port : null; + } + } + + final effectiveHost = uriHost ?? host; + final effectivePort = uriPort ?? port; final displayName = name.isNotEmpty ? name : (uri.isNotEmpty - ? 'PostgreSQL (URI)' + ? 'PostgreSQL: $effectiveHost:$effectivePort' : 'PostgreSQL $host:$port/$database'); final row = ConnectionRow( type: 'postgresql', name: displayName, - host: uri.isNotEmpty ? null : host, - port: uri.isNotEmpty ? null : port, + host: uriHost ?? (uri.isEmpty ? host : null), + port: uriPort ?? (uri.isEmpty ? port : null), username: _usernameController.text.trim().isEmpty ? null : _usernameController.text.trim(), diff --git a/test/core/database/postgres_connection_pool_test.dart b/test/core/database/postgres_connection_pool_test.dart index 0542de50..dff4f3fd 100644 --- a/test/core/database/postgres_connection_pool_test.dart +++ b/test/core/database/postgres_connection_pool_test.dart @@ -339,4 +339,50 @@ void main() { expect(fake.disconnectCount, 1); }); }); + + group('PostgresConnectionPool error wrapping', () { + test('wraps unexpected factory errors in PostgresConnectionException', () async { + Future factory( + ConnectionRow row, { + required String database, + required PgSessionMode mode, + }) async { + throw const FormatException('bad connection string'); + } + + final pool = PostgresConnectionPool(createAndConnect: factory); + await expectLater( + pool.acquire(_row(), database: 'postgres'), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Failed to acquire PostgreSQL connection'), + ), + ), + ); + }); + + test('rethrows PostgresConnectionException from factory', () async { + Future factory( + ConnectionRow row, { + required String database, + required PgSessionMode mode, + }) async { + throw PostgresConnectionException('driver refused'); + } + + final pool = PostgresConnectionPool(createAndConnect: factory); + await expectLater( + pool.acquire(_row(), database: 'postgres'), + throwsA( + isA().having( + (e) => e.message, + 'message', + equals('driver refused'), + ), + ), + ); + }); + }); } diff --git a/test/core/database/postgres_connection_test.dart b/test/core/database/postgres_connection_test.dart index 4d2ea716..673a87bb 100644 --- a/test/core/database/postgres_connection_test.dart +++ b/test/core/database/postgres_connection_test.dart @@ -433,6 +433,18 @@ void main() { expect(ex.message, 'connection refused'); expect(ex.toString(), 'connection refused'); }); + + test('can store cause and stack trace', () { + final cause = StateError('root'); + final trace = StackTrace.current; + final ex = PostgresConnectionException( + 'connection refused', + cause: cause, + stackTrace: trace, + ); + expect(ex.cause, cause); + expect(ex.stackTrace, trace); + }); }); group('replaceDatabaseInConnectionString', () { @@ -470,4 +482,25 @@ void main() { expect(result.error, contains('sslmode')); }); }); + + group('PostgresConnection.connect', () { + test('throws PostgresConnectionException on invalid sslmode', () async { + final conn = PostgresConnection( + id: 1, + name: 'test', + host: 'localhost', + connectionString: 'postgresql://localhost/db?sslmode=invalid', + ); + expect( + conn.connect, + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('sslmode'), + ), + ), + ); + }); + }); } diff --git a/test/features/postgresql/postgresql_connection_form_test.dart b/test/features/postgresql/postgresql_connection_form_test.dart index c1589417..18b76694 100644 --- a/test/features/postgresql/postgresql_connection_form_test.dart +++ b/test/features/postgresql/postgresql_connection_form_test.dart @@ -1,5 +1,6 @@ import 'package:flutter/material.dart' as material; import 'package:flutter_test/flutter_test.dart'; +import 'package:querya_desktop/core/storage/local_db.dart'; import 'package:querya_desktop/core/theme/app_theme.dart'; import 'package:querya_desktop/features/postgresql/postgresql_connection_form.dart'; import 'package:shadcn_flutter/shadcn_flutter.dart'; @@ -88,5 +89,43 @@ void main() { expect(result, isNull); }); + + testWidgets('Save from URI extracts host and port for display', (tester) async { + await tester.binding.setSurfaceSize(const Size(800, 700)); + ConnectionRow? result; + await tester.pumpWidget( + ShadcnApp( + theme: AppTheme.dark, + darkTheme: AppTheme.dark, + themeMode: ThemeMode.dark, + home: material.Builder( + builder: (context) => material.ElevatedButton( + onPressed: () async { + result = await showPostgresConnectionForm(context); + }, + child: const material.Text('Open'), + ), + ), + ), + ); + + await tester.tap(find.text('Open')); + await tester.pumpAndSettle(); + + await tester.enterText( + find.byWidgetPredicate( + (w) => w is TextField && w.placeholder is Text && (w.placeholder as Text).data == 'postgresql://user:pass@host:5432/dbname?sslmode=require', + ), + 'postgresql://u:p@remote.example.com:5433/db', + ); + await tester.tap(find.text('Save')); + await tester.pumpAndSettle(); + + expect(result, isNotNull); + expect(result!.host, 'remote.example.com'); + expect(result!.port, 5433); + expect(result!.name, 'PostgreSQL: remote.example.com:5433'); + expect(result!.connectionString, 'postgresql://u:p@remote.example.com:5433/db'); + }); }); }