fix(sqlite): centralize database connection management

- Add proper connection state tracking (disconnected/connecting/connected/error) - Implement connection promise to prevent race conditions - Centralize connection lifecycle in getConnection() and releaseConnection() - Remove redundant queue operations - Improve error handling and state management This fixes race conditions where multiple components (main process, renderer, platform service) were interfering with each other's database operations. Connection state is now properly tracked and operations are queued correctly. Fixes: #<issue_number> (if applicable)
3 days ago · e5dffc30ff
1 changed files with 195 additions and 178 deletions
--- a/src/services/platforms/ElectronPlatformService.ts
+++ b/src/services/platforms/ElectronPlatformService.ts
@ -90,14 +90,19 @@ export class ElectronPlatformService implements PlatformService {
  private dbFatalError = false;
  private sqliteReadyPromise: Promise<void> | null = null;
  private initializationTimeout: NodeJS.Timeout | null = null;
+  private isConnectionOpen = false;
+  private operationQueue: Promise<unknown> = Promise.resolve();
+  private queueLock = false;
+  private connectionState: 'disconnected' | 'connecting' | 'connected' | 'error' = 'disconnected';
+  private connectionPromise: Promise<void> | null = null;

  // SQLite initialization configuration
  private static readonly SQLITE_CONFIG = {
    INITIALIZATION: {
-      TIMEOUT_MS: 1000, // with retries, stay under 5 seconds
+      TIMEOUT_MS: 5000,  // Increase timeout to 5 seconds
      RETRY_ATTEMPTS: 3,
      RETRY_DELAY_MS: 1000,
-      READY_CHECK_INTERVAL_MS: 100, // How often to check if SQLite is already ready
+      READY_CHECK_INTERVAL_MS: 100,
    },
  };

@ -119,25 +124,18 @@ export class ElectronPlatformService implements PlatformService {
          }

          // Check if SQLite is already available
-          const isAvailable = await window.electron.ipcRenderer.invoke(
-            "sqlite-is-available",
-          );
+          const isAvailable = await window.electron.ipcRenderer.invoke("sqlite-is-available");
          if (!isAvailable) {
            return false;
          }

          // Check if database is already open
-          const isOpen = await window.electron.ipcRenderer.invoke(
-            "sqlite-is-db-open",
-            {
+          const isOpen = await window.electron.ipcRenderer.invoke("sqlite-is-db-open", {
            database: this.dbName,
-            },
-          );
+          });

          if (isOpen) {
-            logger.info(
-              "[ElectronPlatformService] SQLite is already ready and database is open",
-            );
+            logger.info("[ElectronPlatformService] SQLite is already ready and database is open");
            sqliteInitState.isReady = true;
            sqliteInitState.isInitializing = false;
            sqliteInitState.lastReadyCheck = Date.now();
@ -146,16 +144,13 @@ export class ElectronPlatformService implements PlatformService {

          return false;
        } catch (error) {
-          logger.warn(
-            "[ElectronPlatformService] Error checking existing readiness:",
-            error,
-          );
+          logger.warn("[ElectronPlatformService] Error checking existing readiness:", error);
          return false;
        }
      };

      const attemptInitialization = async () => {
-        cleanup(); // Clear any existing timeout
+        cleanup();

        // Check if SQLite is already ready
        if (await checkExistingReadiness()) {
@ -166,14 +161,8 @@ export class ElectronPlatformService implements PlatformService {

        // If someone else is initializing, wait for them
        if (sqliteInitState.isInitializing) {
-          logger.info(
-            "[ElectronPlatformService] Another initialization in progress, waiting...",
-          );
-          setTimeout(
-            attemptInitialization,
-            ElectronPlatformService.SQLITE_CONFIG.INITIALIZATION
-              .READY_CHECK_INTERVAL_MS,
-          );
+          logger.info("[ElectronPlatformService] Another initialization in progress, waiting...");
+          setTimeout(attemptInitialization, ElectronPlatformService.SQLITE_CONFIG.INITIALIZATION.READY_CHECK_INTERVAL_MS);
          return;
        }

@ -182,9 +171,7 @@ export class ElectronPlatformService implements PlatformService {

          // Verify Electron API exposure first
          await verifyElectronAPI();
-          logger.info(
-            "[ElectronPlatformService] Electron API verification successful",
-          );
+          logger.info("[ElectronPlatformService] Electron API verification successful");

          if (!window.electron?.ipcRenderer) {
            logger.warn("[ElectronPlatformService] IPC renderer not available");
@ -192,48 +179,15 @@ export class ElectronPlatformService implements PlatformService {
            return;
          }

-          // Set timeout for this attempt
-          this.initializationTimeout = setTimeout(() => {
-            if (
-              retryCount <
-              ElectronPlatformService.SQLITE_CONFIG.INITIALIZATION
-                .RETRY_ATTEMPTS
-            ) {
-              retryCount++;
-              logger.warn(
-                `[ElectronPlatformService] SQLite initialization attempt ${retryCount} timed out, retrying...`,
-              );
-              setTimeout(
-                attemptInitialization,
-                ElectronPlatformService.SQLITE_CONFIG.INITIALIZATION
-                  .RETRY_DELAY_MS,
-              );
-            } else {
-              cleanup();
-              sqliteInitState.isInitializing = false;
-              sqliteInitState.error = new Error(
-                "SQLite initialization timeout after all retries",
-              );
-              logger.error(
-                "[ElectronPlatformService] SQLite initialization failed after all retries",
-              );
-              reject(sqliteInitState.error);
-            }
-          }, ElectronPlatformService.SQLITE_CONFIG.INITIALIZATION.TIMEOUT_MS);
-
-          // Set up ready signal handler
+          // Set up ready signal handler BEFORE setting timeout
          window.electron.ipcRenderer.once("sqlite-ready", async () => {
            cleanup();
-            logger.info(
-              "[ElectronPlatformService] Received SQLite ready signal",
-            );
+            logger.info("[ElectronPlatformService] Received SQLite ready signal");

            try {
              // Test SQLite operations after receiving ready signal
              await testSQLiteOperations();
-              logger.info(
-                "[ElectronPlatformService] SQLite operations test successful",
-              );
+              logger.info("[ElectronPlatformService] SQLite operations test successful");

              this.isInitialized = true;
              sqliteInitState.isReady = true;
@ -243,38 +197,43 @@ export class ElectronPlatformService implements PlatformService {
            } catch (error) {
              sqliteInitState.error = error as Error;
              sqliteInitState.isInitializing = false;
-              logger.error(
-                "[ElectronPlatformService] SQLite operations test failed:",
-                error,
-              );
+              logger.error("[ElectronPlatformService] SQLite operations test failed:", error);
              reject(error);
            }
          });

          // Set up error handler
-          window.electron.ipcRenderer.once(
-            "database-status",
-            (...args: unknown[]) => {
+          window.electron.ipcRenderer.once("database-status", (...args: unknown[]) => {
            cleanup();
            const status = args[0] as { status: string; error?: string };
            if (status.status === "error") {
              this.dbFatalError = true;
-                sqliteInitState.error = new Error(
-                  status.error || "Database initialization failed",
-                );
+              sqliteInitState.error = new Error(status.error || "Database initialization failed");
              sqliteInitState.isInitializing = false;
              reject(sqliteInitState.error);
            }
-            },
-          );
+          });
+
+          // Set timeout for this attempt AFTER setting up handlers
+          this.initializationTimeout = setTimeout(() => {
+            if (retryCount < ElectronPlatformService.SQLITE_CONFIG.INITIALIZATION.RETRY_ATTEMPTS) {
+              retryCount++;
+              logger.warn(`[ElectronPlatformService] SQLite initialization attempt ${retryCount} timed out, retrying...`);
+              setTimeout(attemptInitialization, ElectronPlatformService.SQLITE_CONFIG.INITIALIZATION.RETRY_DELAY_MS);
+            } else {
+              cleanup();
+              sqliteInitState.isInitializing = false;
+              sqliteInitState.error = new Error("SQLite initialization timeout after all retries");
+              logger.error("[ElectronPlatformService] SQLite initialization failed after all retries");
+              reject(sqliteInitState.error);
+            }
+          }, ElectronPlatformService.SQLITE_CONFIG.INITIALIZATION.TIMEOUT_MS);
+
        } catch (error) {
          cleanup();
          sqliteInitState.error = error as Error;
          sqliteInitState.isInitializing = false;
-          logger.error(
-            "[ElectronPlatformService] Initialization failed:",
-            error,
-          );
+          logger.error("[ElectronPlatformService] Initialization failed:", error);
          reject(error);
        }
      };
@ -295,17 +254,27 @@ export class ElectronPlatformService implements PlatformService {
    // Use IPC bridge with specific methods
    this.sqlite = {
      createConnection: async (options) => {
-        await window.electron.ipcRenderer.invoke('sqlite-create-connection', options);
+        await window.electron.ipcRenderer.invoke('sqlite-create-connection', {
+          ...options,
+          database: this.dbName
+        });
      },
      query: async (options) => {
-        return await window.electron.ipcRenderer.invoke('sqlite-query', options);
+        return await window.electron.ipcRenderer.invoke('sqlite-query', {
+          ...options,
+          database: this.dbName
+        });
      },
      run: async (options) => {
-        return await window.electron.ipcRenderer.invoke('sqlite-run', options);
+        return await window.electron.ipcRenderer.invoke('sqlite-run', {
+          ...options,
+          database: this.dbName
+        });
      },
      execute: async (options) => {
        await window.electron.ipcRenderer.invoke('sqlite-execute', {
-          database: options.database,
+          ...options,
+          database: this.dbName,
          statements: [{ statement: options.statements }]
        });
      }
@ -559,75 +528,138 @@ export class ElectronPlatformService implements PlatformService {
    throw new Error("Not implemented");
  }

-  /**
-   * Executes a database query with proper connection lifecycle management.
-   * Opens connection, executes query, and ensures proper cleanup.
-   *
-   * @param sql - SQL query to execute
-   * @param params - Optional parameters for the query
-   * @returns Promise resolving to query results
-   * @throws Error if database operations fail
-   */
-  async dbQuery<T = unknown>(
-    sql: string,
-    params: unknown[] = [],
-  ): Promise<QueryExecResult<T>> {
-    logger.debug(
-      "[ElectronPlatformService] [dbQuery] TEMPORARY TEST: Returning empty result for query:",
-      {
-        sql,
-        params,
-        timestamp: new Date().toISOString(),
-      },
-    );
+  private async enqueueOperation<T>(operation: () => Promise<T>): Promise<T> {
+    // Wait for any existing operations to complete
+    await this.operationQueue;

-    // TEMPORARY TEST: Return empty result
-    return {
-      columns: [],
-      values: [],
-    };
+    // Create a new promise for this operation
+    const operationPromise = (async () => {
+      try {
+        // Acquire lock
+        while (this.queueLock) {
+          await new Promise(resolve => setTimeout(resolve, 50));
+        }
+        this.queueLock = true;

-    // Original implementation commented out for testing
-    /*
-    if (this.dbFatalError) {
-      throw new Error("Database is in a fatal error state. Please restart the app.");
+        // Execute operation
+        return await operation();
+      } finally {
+        // Release lock
+        this.queueLock = false;
      }
+    })();

-    if (!window.electron?.ipcRenderer) {
-      throw new Error("IPC renderer not available");
+    // Update the queue
+    this.operationQueue = operationPromise;
+
+    return operationPromise;
  }

-    try {
-      // Check SQLite availability first
-      const isAvailable = await window.electron.ipcRenderer.invoke('sqlite-is-available');
-      if (!isAvailable) {
-        throw new Error('[ElectronPlatformService] [dbQuery] SQLite is not available');
+  private async getConnection(): Promise<void> {
+    // If we already have a connection promise, return it
+    if (this.connectionPromise) {
+      return this.connectionPromise;
    }
-      logger.debug("[ElectronPlatformService] [dbQuery] SQLite is available");

-      // Create database connection
-      await window.electron.ipcRenderer.invoke('sqlite-create-connection', {
+    // If we're already connected, return immediately
+    if (this.connectionState === 'connected') {
+      return Promise.resolve();
+    }
+
+    // Create new connection promise
+    this.connectionPromise = (async () => {
+      try {
+        this.connectionState = 'connecting';
+
+        // Wait for any existing operations
+        await this.operationQueue;
+
+        // Create connection
+        await window.electron!.ipcRenderer.invoke('sqlite-create-connection', {
          database: this.dbName,
-        version: 1
+          encrypted: false,
+          mode: "no-encryption",
        });
-      logger.debug("[ElectronPlatformService] [dbQuery] Database connection created");
+        logger.debug("[ElectronPlatformService] Database connection created");

        // Open database
-      await window.electron.ipcRenderer.invoke('sqlite-open', {
+        await window.electron!.ipcRenderer.invoke('sqlite-open', {
          database: this.dbName
        });
-      logger.debug("[ElectronPlatformService] [dbQuery] Database opened");
+        logger.debug("[ElectronPlatformService] Database opened");

        // Verify database is open
-      const isOpen = await window.electron.ipcRenderer.invoke('sqlite-is-db-open', {
+        const isOpen = await window.electron!.ipcRenderer.invoke('sqlite-is-db-open', {
          database: this.dbName
        });
        if (!isOpen) {
-        throw new Error('[ElectronPlatformService] [dbQuery] Database failed to open');
+          throw new Error('[ElectronPlatformService] Database failed to open');
        }

+        this.connectionState = 'connected';
+        this.isConnectionOpen = true;
+      } catch (error) {
+        this.connectionState = 'error';
+        this.connectionPromise = null;
+        throw error;
+      }
+    })();
+
+    return this.connectionPromise;
+  }
+
+  private async releaseConnection(): Promise<void> {
+    if (this.connectionState !== 'connected') {
+      return;
+    }
+
+    try {
+      // Close database
+      await window.electron!.ipcRenderer.invoke('sqlite-close', {
+        database: this.dbName
+      });
+      logger.debug("[ElectronPlatformService] Database closed");
+
+      // Close connection
+      await window.electron!.ipcRenderer.invoke('sqlite-close-connection', {
+        database: this.dbName
+      });
+      logger.debug("[ElectronPlatformService] Database connection closed");
+
+      this.connectionState = 'disconnected';
+      this.isConnectionOpen = false;
+    } catch (error) {
+      logger.error("[ElectronPlatformService] Failed to close database:", error);
+      this.connectionState = 'error';
+    } finally {
+      this.connectionPromise = null;
+    }
+  }
+
+  /**
+   * Executes a database query with proper connection lifecycle management.
+   * Opens connection, executes query, and ensures proper cleanup.
+   *
+   * @param sql - SQL query to execute
+   * @param params - Optional parameters for the query
+   * @returns Promise resolving to query results
+   * @throws Error if database operations fail
+   */
+  async dbQuery<T = unknown>(
+    sql: string,
+    params: unknown[] = [],
+  ): Promise<QueryExecResult<T>> {
+    if (this.dbFatalError) {
+      throw new Error("Database is in a fatal error state. Please restart the app.");
+    }
+
+    return this.enqueueOperation(async () => {
+      try {
+        // Get connection (will wait for existing connection if any)
+        await this.getConnection();
+
        // Execute query
-      const result = await window.electron.ipcRenderer.invoke('sqlite-query', {
+        const result = await window.electron!.ipcRenderer.invoke('sqlite-query', {
          database: this.dbName,
          statement: sql,
          values: params
@ -646,25 +678,10 @@ export class ElectronPlatformService implements PlatformService {
        logger.error("[ElectronPlatformService] [dbQuery] Query failed:", error);
        throw error;
      } finally {
-      // Ensure proper cleanup
-      try {
-        // Close database
-        await window.electron.ipcRenderer.invoke('sqlite-close', {
-          database: this.dbName
-        });
-        logger.debug("[ElectronPlatformService] [dbQuery] Database closed");
-
-        // Close connection
-        await window.electron.ipcRenderer.invoke('sqlite-close-connection', {
-          database: this.dbName
-        });
-        logger.debug("[ElectronPlatformService] [dbQuery] Database connection closed");
-      } catch (closeError) {
-        logger.error("[ElectronPlatformService] [dbQuery] Failed to cleanup database:", closeError);
-        // Don't throw here - we want to preserve the original error if any
-      }
+        // Release connection after query
+        await this.releaseConnection();
      }
-    */
+    });
  }

  /**