[client-app] Correctly handle db malformed errors in main process

Summary: This commit addressess issue T8135, which prevents the app from starting. This was happening because when 3111c16 landed, we added database access from the main (backend) electron process to be able to read the identity now stored in the database: nylas@3111c166#diff-1efa26fa0ae1603366b2c0033d971028R44 However, we omitted to add any error handling, so if the database failed to open due to a database malformed error (which it does: https://sentry.io/nylas/nylas-mail/?query=is%3Aunresolved+release%3A2.0.14+malformed&statsPeriod=14d), the app will just fail to start, given that this happens during the initialization of the main process. Additionally, the fact that we had no error handling increased the error reports for malformed errors given that we would never handle them, so every-time we opened the app we would report the same error This commit adds the same error handling we have in the DatabaseStore and moves the code around so it's available both in the main and renderer processes. After this commit, if the database fails to open during main process initialization, due to malformed errors or others, we will correctly inform the user that the database is corrupted, rebuild it, and restart the app. Test Plan: manually throw errors during setup, verify that we handle them correctly Reviewers: mark, spang, evan, halla Reviewed By: evan, halla Differential Revision: https://phab.nylas.com/D4431
jorgenio · Apr 17, 2017 · 26868f6 · 26868f6
1 parent 5b99c56
commit 26868f6
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 45 deletions.
diff --git a/packages/client-app/src/browser/application.es6 b/packages/client-app/src/browser/application.es6
@@ -48,7 +48,15 @@ export default class Application extends EventEmitter {
     this.nylasProtocolHandler = new NylasProtocolHandler(this.resourcePath, this.safeMode);
 
     this.databaseReader = new DatabaseReader({configDirPath, specMode});
-    await this.databaseReader.open();
+    try {
+      await this.databaseReader.open();
+    } catch (err) {
+      this._deleteDatabase(() => {
+        app.relaunch()
+        app.quit()
+      })
+      return
+    }
 
     const Config = require('../config');
     const config = new Config();

diff --git a/packages/client-app/src/browser/database-reader.es6 b/packages/client-app/src/browser/database-reader.es6
@@ -1,12 +1,12 @@
-import {setupDatabase, databasePath} from '../database-helpers'
+import {openDatabase, databasePath} from '../database-helpers'
 
 export default class DatabaseReader {
   constructor({configDirPath, specMode}) {
     this.databasePath = databasePath(configDirPath, specMode)
   }
 
   async open() {
-    this.database = await setupDatabase(this.databasePath)
+    this.database = await openDatabase(this.databasePath)
   }
 
   getJSONBlob(key) {

diff --git a/packages/client-app/src/database-helpers.es6 b/packages/client-app/src/database-helpers.es6
@@ -1,28 +1,63 @@
 import path from 'path';
 import Sqlite3 from 'better-sqlite3';
 
-export function setupDatabase(dbPath) {
-  return new Promise((resolve, reject) => {
-    const db = new Sqlite3(dbPath, {});
-    db.on('close', reject)
-    db.on('open', () => {
-      // https://www.sqlite.org/wal.html
-      // WAL provides more concurrency as readers do not block writers and a writer
-      // does not block readers. Reading and writing can proceed concurrently.
-      db.pragma(`journal_mode = WAL`);
-
-      // Note: These are properties of the connection, so they must be set regardless
-      // of whether the database setup queries are run.
-
-      // https://www.sqlite.org/intern-v-extern-blob.html
-      // A database page size of 8192 or 16384 gives the best performance for large BLOB I/O.
-      db.pragma(`main.page_size = 8192`);
-      db.pragma(`main.cache_size = 20000`);
-      db.pragma(`main.synchronous = NORMAL`);
-
-      resolve(db);
+let app;
+let dialog;
+let errorLogger;
+
+if (process.type === 'renderer') {
+  const remote = require('electron').remote // eslint-disable-line
+  app = remote.getGlobal('application')
+  dialog = remote.dialog
+  errorLogger = NylasEnv.errorLogger;
+} else {
+  app = global.application
+  errorLogger = global.errorLogger;
+  dialog = require('electron').dialog // eslint-disable-line
+}
+
+export function handleUnrecoverableDatabaseError(err = (new Error(`Manually called handleUnrecoverableDatabaseError`))) {
+  errorLogger.reportError(err, {}, {noWindows: true});
+  if (!app) {
+    throw new Error('handleUnrecoverableDatabaseError: `app` is not ready!')
+  }
+  app.rebuildDatabase();
+}
+
+export async function openDatabase(dbPath) {
+  try {
+    const database = await new Promise((resolve, reject) => {
+      const db = new Sqlite3(dbPath, {});
+      db.on('close', reject)
+      db.on('open', () => {
+        // https://www.sqlite.org/wal.html
+        // WAL provides more concurrency as readers do not block writers and a writer
+        // does not block readers. Reading and writing can proceed concurrently.
+        db.pragma(`journal_mode = WAL`);
+
+        // Note: These are properties of the connection, so they must be set regardless
+        // of whether the database setup queries are run.
+
+        // https://www.sqlite.org/intern-v-extern-blob.html
+        // A database page size of 8192 or 16384 gives the best performance for large BLOB I/O.
+        db.pragma(`main.page_size = 8192`);
+        db.pragma(`main.cache_size = 20000`);
+        db.pragma(`main.synchronous = NORMAL`);
+
+        resolve(db);
+      });
+    })
+    return database
+  } catch (err) {
+    dialog.showMessageBox({
+      type: 'warning',
+      buttons: ['Okay'],
+      message: `Unable to open SQLite database at ${dbPath}.\n\nDatbase will be rebuilt`,
+      detail: err.toString(),
     });
-  })
+    handleUnrecoverableDatabaseError(err);
+    return null
+  }
 }
 
 export function databasePath(configDirPath, specMode = false) {

diff --git a/packages/client-app/src/flux/stores/database-store.es6 b/packages/client-app/src/flux/stores/database-store.es6
@@ -1,6 +1,5 @@
 /* eslint global-require: 0 */
 import path from 'path';
-import fs from 'fs';
 import createDebug from 'debug'
 import childProcess from 'child_process';
 import PromiseQueue from 'promise-queue';
@@ -15,7 +14,7 @@ import Actions from '../actions'
 import DatabaseChangeRecord from './database-change-record';
 import DatabaseWriter from './database-writer';
 import DatabaseSetupQueryBuilder from './database-setup-query-builder';
-import {setupDatabase, databasePath} from '../../database-helpers'
+import {openDatabase, handleUnrecoverableDatabaseError, databasePath} from '../../database-helpers'
 
 const debug = createDebug('app:RxDB')
 const debugVerbose = createDebug('app:RxDB:all')
@@ -178,23 +177,15 @@ class DatabaseStore extends NylasStore {
 
   async _openDatabase() {
     if (this._db) return
-    try {
-      this._db = await setupDatabase(this._databasePath)
-    } catch (err) {
-      NylasEnv.showErrorDialog({
-        title: `Unable to open SQLite database at ${this._databasePath}`,
-        message: err.toString(),
-      });
-      this._handleSetupError(err);
-    }
+    this._db = await openDatabase(this._databasePath)
   }
 
   _checkDatabaseVersion({allowUnset} = {}, ready) {
     const result = this._db.pragma('user_version', true);
     const isUnsetVersion = (result === '0');
     const isWrongVersion = (result !== DatabaseVersion);
     if (isWrongVersion && !(isUnsetVersion && allowUnset)) {
-      return this._handleSetupError(new Error(`Incorrect database schema version: ${result} not ${DatabaseVersion}`));
+      return handleUnrecoverableDatabaseError(new Error(`Incorrect database schema version: ${result} not ${DatabaseVersion}`));
     }
     return ready();
   }
@@ -208,20 +199,13 @@ class DatabaseStore extends NylasStore {
         this._db.prepare(query).run();
       }
     } catch (err) {
-      return this._handleSetupError(err);
+      return handleUnrecoverableDatabaseError(err);
     }
 
     this._db.pragma(`user_version=${DatabaseVersion}`);
     return ready();
   }
 
-  _handleSetupError(err = (new Error(`Manually called _handleSetupError`))) {
-    NylasEnv.reportError(err, {}, {noWindows: true});
-
-    const app = remote.getGlobal('application');
-    app.rebuildDatabase();
-  }
-
   _prettyConsoleLog(qa) {
     let q = qa.replace(/%/g, '%%');
     q = `color:black |||%c ${q}`;

diff --git a/packages/client-app/src/services/delta-streaming-connection.es6 b/packages/client-app/src/services/delta-streaming-connection.es6
@@ -151,7 +151,8 @@ class DeltaStreamingConnection {
       const error = new Error('DeltaStreamingConnection: Cursor is invalid. Need to blow away local cache.');
       NylasEnv.reportError(error)
       this._setCursor(0)
-      DatabaseStore._handleSetupError(error)
+      const app = require('electron').remote.getGlobal('application') // eslint-disable-line
+      app.rebuildDatabase()
       return
     }