Merge pull request #947 from QueryaHub/issue/916-lazy-secrets-keychain-error-handling

perf(storage): resolve connection secrets lazily and handle Keychain errors gracefully (#916)

Author: ZhuchkaTriplesix

lib/core/storage/connection_secrets_store.dart

@@ -56,10 +56,14 @@ class ConnectionSecretsStore {
       readForConnection(
     int connectionId,
   ) async {
-    final password = await backend.read(_passwordKey(connectionId));
-    final connectionString =
-        await backend.read(_connectionStringKey(connectionId));
-    return (password: password, connectionString: connectionString);
+    try {
+      final password = await backend.read(_passwordKey(connectionId));
+      final connectionString =
+          await backend.read(_connectionStringKey(connectionId));
+      return (password: password, connectionString: connectionString);
+    } catch (_) {
+      return (password: null, connectionString: null);
+    }
   }
 
   static Future<void> deleteForConnection(int connectionId) async {

lib/core/storage/local_db.dart

@@ -1,5 +1,6 @@
 import 'dart:io';
 
+import 'package:flutter/foundation.dart';
 import 'package:path/path.dart' as p;
 import 'package:querya_desktop/core/storage/app_data_root.dart';
 import 'package:querya_desktop/core/storage/connection_secrets_store.dart';
@@ -423,34 +424,50 @@ class LocalDb {
     return _sqliteInt(rows.first['id']);
   }
 
-  Future<List<ConnectionRow>> getConnections() async {
+  /// Returns all saved connections.
+  ///
+  /// By default, passwords and connection strings are NOT eagerly hydrated from
+  /// the platform secure store to avoid IPC bottlenecks, Keychain lockups, and
+  /// D-Bus timeouts during sidebar/startup population. Secrets are resolved
+  /// on-demand when a connection is initiated.
+  Future<List<ConnectionRow>> getConnections({bool hydrateSecrets = false}) async {
     final db = await _open();
     final rows =
         await db.query('connections', orderBy: 'sort_order ASC, name ASC');
+    if (!hydrateSecrets) {
+      return rows.map((m) => ConnectionRow.fromMap(m)).toList();
+    }
     final futures =
         rows.map((m) => _hydrateConnection(ConnectionRow.fromMap(m)));
     return Future.wait(futures);
   }
 
   static Future<ConnectionRow> _hydrateConnection(ConnectionRow row) async {
     if (row.id == null) return row;
-    final secrets = await ConnectionSecretsStore.readForConnection(row.id!);
-    return ConnectionRow(
-      id: row.id,
-      type: row.type,
-      name: row.name,
-      host: row.host,
-      port: row.port,
-      username: row.username,
-      password: secrets.password ?? row.password,
-      databaseName: row.databaseName,
-      authSource: row.authSource,
-      useSSL: row.useSSL,
-      connectionString: secrets.connectionString ?? row.connectionString,
-      folderId: row.folderId,
-      sortOrder: row.sortOrder,
-      createdAt: row.createdAt,
-    );
+    try {
+      final secrets = await ConnectionSecretsStore.readForConnection(row.id!);
+      return row.copyWith(
+        password: secrets.password ?? row.password,
+        connectionString: secrets.connectionString ?? row.connectionString,
+      );
+    } catch (e) {
+      debugPrint('LocalDb._hydrateConnection failed for connection ${row.id}: $e');
+      return row;
+    }
+  }
+
+  /// Hydrates secrets for a single [ConnectionRow] on demand.
+  Future<ConnectionRow> hydrateConnection(ConnectionRow row) =>
+      _hydrateConnection(row);
+
+  /// Retrieves a single connection by [id], optionally hydrating secrets.
+  Future<ConnectionRow?> getConnectionById(int id, {bool hydrateSecrets = false}) async {
+    final db = await _open();
+    final rows = await db.query('connections', where: 'id = ?', whereArgs: [id]);
+    if (rows.isEmpty) return null;
+    final row = ConnectionRow.fromMap(rows.first);
+    if (!hydrateSecrets) return row;
+    return _hydrateConnection(row);
   }
 
   /// Inserts a row and returns the SQLite row id.

test/core/storage/local_db_secrets_test.dart

@@ -98,7 +98,15 @@ void main() {
 
       final list = await LocalDb.instance.getConnections();
       final loaded = list.singleWhere((c) => c.id == id);
-      expect(loaded.password, 'redis-secret');
+      // Secrets are resolved lazily on demand, not eagerly in getConnections()
+      expect(loaded.password, isNull);
+
+      final hydrated = await LocalDb.instance.hydrateConnection(loaded);
+      expect(hydrated.password, 'redis-secret');
+
+      final eagerlyHydrated = (await LocalDb.instance.getConnections(hydrateSecrets: true))
+          .singleWhere((c) => c.id == id);
+      expect(eagerlyHydrated.password, 'redis-secret');
     });
 
     test('removeConnection deletes secure-store entries', () async {
@@ -146,7 +154,7 @@ void main() {
       );
       await LocalDb.instance.updateConnection(updatedRow);
 
-      final list = await LocalDb.instance.getConnections();
+      final list = await LocalDb.instance.getConnections(hydrateSecrets: true);
       final loaded = list.singleWhere((c) => c.id == id);
       expect(loaded.name, 'PG_Updated');
       expect(loaded.host, 'db.example.com');
@@ -234,7 +242,7 @@ void main() {
         throwsA(isA<StateError>()),
       );
 
-      final list = await LocalDb.instance.getConnections();
+      final list = await LocalDb.instance.getConnections(hydrateSecrets: true);
       final loaded = list.singleWhere((c) => c.id == id);
       expect(loaded.name, 'PG_Before');
       expect(loaded.host, 'localhost');
@@ -273,10 +281,65 @@ void main() {
       expect(merged.host, 'db.example.com');
 
       await LocalDb.instance.updateConnection(merged);
-      final loaded = (await LocalDb.instance.getConnections())
+      final loaded = (await LocalDb.instance.getConnections(hydrateSecrets: true))
           .singleWhere((c) => c.id == id);
       expect(loaded.password, 'keep-me');
       expect(loaded.name, 'PG Renamed');
     });
+
+    test('getConnections does not read secure store by default', () async {
+      const row = ConnectionRow(
+        type: 'mysql',
+        name: 'MySQL_Lazy',
+        host: 'localhost',
+        port: 3306,
+        username: 'root',
+        password: 'super-secret-pw',
+        createdAt: '2026-01-01T00:00:00Z',
+      );
+      final id = await LocalDb.instance.addConnection(row);
+
+      // failNextRead should NOT be triggered because getConnections() does not read secrets!
+      testMemorySecrets.failNextRead = StateError('should not be called');
+
+      final list = await LocalDb.instance.getConnections();
+      final conn = list.singleWhere((c) => c.id == id);
+      expect(conn.name, 'MySQL_Lazy');
+      expect(conn.password, isNull);
+
+      // failNextRead is still set because read was never called
+      expect(testMemorySecrets.failNextRead, isNotNull);
+      testMemorySecrets.failNextRead = null;
+    });
+
+    test(
+        'readForConnection and _hydrateConnection handle Keychain error gracefully without throwing',
+        () async {
+      const row = ConnectionRow(
+        type: 'mysql',
+        name: 'MySQL_Faulty',
+        host: 'localhost',
+        port: 3306,
+        username: 'root',
+        password: 'secret-password',
+        createdAt: '2026-01-01T00:00:00Z',
+      );
+      final id = await LocalDb.instance.addConnection(row);
+
+      testMemorySecrets.failNextRead = StateError('org.freedesktop.DBus.Error.NoReply');
+
+      // readForConnection should return nulls instead of throwing
+      final secrets = await ConnectionSecretsStore.readForConnection(id);
+      expect(secrets.password, isNull);
+      expect(secrets.connectionString, isNull);
+
+      testMemorySecrets.failNextRead = StateError('Keychain locked');
+
+      // getConnections(hydrateSecrets: true) should still succeed and return the connection row
+      final list = await LocalDb.instance.getConnections(hydrateSecrets: true);
+      final conn = list.singleWhere((c) => c.id == id);
+      expect(conn.name, 'MySQL_Faulty');
+      expect(conn.password, isNull);
+    });
   });
 }

test/memory_secrets_backend.dart

@@ -8,14 +8,24 @@ final MemorySecretsStorageBackend testMemorySecrets =
 class MemorySecretsStorageBackend implements SecretsStorageBackend {
   final Map<String, String> _values = {};
 
+  /// When non-null, the next [read] throws this error (then clears the flag).
+  Object? failNextRead;
+
   /// When non-null, the next [write] throws this error (then clears the flag).
   Object? failNextWrite;
 
   /// When non-null, the next [delete] throws this error (then clears the flag).
   Object? failNextDelete;
 
   @override
-  Future<String?> read(String key) async => _values[key];
+  Future<String?> read(String key) async {
+    final fail = failNextRead;
+    if (fail != null) {
+      failNextRead = null;
+      throw fail;
+    }
+    return _values[key];
+  }
 
   @override
   Future<void> write(String key, String? value) async {
@@ -43,6 +53,7 @@ class MemorySecretsStorageBackend implements SecretsStorageBackend {
 
   void clear() {
     _values.clear();
+    failNextRead = null;
     failNextWrite = null;
     failNextDelete = null;
   }