Merge pull request #1019 from ncloudioj/gh1002-reconnect-db

Handle exceptions if the addon couldn't connect to Metadatastore

lib/MetadataStore.js

@@ -7,7 +7,6 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Sqlite.jsm");
 Cu.import("resource://gre/modules/NetUtil.jsm");
 Cu.import("resource://gre/modules/Timer.jsm");
-const fileIO = require("sdk/io/file");
 
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
@@ -113,6 +112,9 @@ MetadataStore.prototype = {
   }),
 
   get transactionInProgress() {
+    if (!this._conn) {
+      return false;
+    }
     return this._conn.transactionInProgress;
   },
 
@@ -216,6 +218,10 @@ MetadataStore.prototype = {
    * Returns a promise that is resolved upon success, or rejected if an exception occurs
    */
   asyncInsert: Task.async(function*(metaObjects) {
+    if (!this._conn) {
+      throw new Error("MetadataStore is not yet connected");
+    }
+
     const principal = Services.scriptSecurityManager.getSystemPrincipal();
     for (let metaObject of metaObjects) {
       yield this._conn.executeTransaction(function*() {
@@ -320,10 +326,9 @@ MetadataStore.prototype = {
   asyncTearDown: Task.async(function*() {
     try {
       yield this.asyncClose();
-    } finally {
-      if (fileIO.exists(this._path) && fileIO.isFile(this._path)) {
-        fileIO.remove(this._path);
-      }
+      yield OS.File.remove(this._path, {ignoreAbsent: true});
+    } catch (e) {
+      Cu.reportError("MetadataStore failed to tear down the database: " + e.message);
     }
   }),
 
@@ -347,6 +352,10 @@ MetadataStore.prototype = {
     let items = [];
     let queryError = null;
 
+    if (!this._conn) {
+      throw new Error("MetadataStore is not yet connected");
+    }
+
     yield this._conn.executeCached(aSql, params, aRow => {
       try {
         // check if caller wants to handle query raws
@@ -385,7 +394,8 @@ MetadataStore.prototype = {
   /**
    * Get page metadata (including images) for the given cache_keys.
    * For the missing cache_keys, it simply ignores them and will not
-   * raise any exception
+   * raise any exception. Note that if this function gets called on
+   * a closed or unestablished connection, it returns an empty array
    *
    * @param {Array} cacheKeys an cache key array
    *
@@ -393,25 +403,38 @@ MetadataStore.prototype = {
    */
   asyncGetMetadataByCacheKey: Task.async(function*(cacheKeys) {
     const quoted = cacheKeys.map(key => {return `'${key}'`;}).join(",");
+    let metaObjects;
+    try {
+      metaObjects = yield this.asyncExecuteQuery(
+        `SELECT * FROM page_metadata WHERE cache_key IN (${quoted})`,
+        {columns: ["id", "cache_key", "places_url", "title", "type", "description", "media_url"]}
+      );
+    }
+    catch (e) {
+      // return immediately if there is any exception gets thrown
+      return [];
+    }
 
-    let metaObjects = yield this.asyncExecuteQuery(
-      `SELECT * FROM page_metadata WHERE cache_key IN (${quoted})`,
-      {columns: ["id", "cache_key", "places_url", "title", "type", "description", "media_url"]}
-    );
-
-    // fetch favicons and images
+    // fetch the favicons and images for each metadata object
     for (let metaObject of metaObjects) {
+      let images;
       metaObject.images = [];
       metaObject.favicons = [];
 
-      let images = yield this.asyncExecuteQuery(
-        `SELECT pi.*
-         FROM page_metadata AS pm
-            JOIN page_metadata_images AS pmi ON pm.id = pmi.metadata_id
-            JOIN page_images AS pi ON pi.id = pmi.image_id
-         WHERE pm.id = ${metaObject.id}`,
-         {columns: ["url", "type", "height", "width", "color"]}
-      );
+      try {
+        images = yield this.asyncExecuteQuery(
+          `SELECT pi.*
+           FROM page_metadata AS pm
+              JOIN page_metadata_images AS pmi ON pm.id = pmi.metadata_id
+              JOIN page_images AS pi ON pi.id = pmi.image_id
+           WHERE pm.id = ${metaObject.id}`,
+           {columns: ["url", "type", "height", "width", "color"]}
+        );
+      } catch (e) {
+        // return immediately if there is any exception gets thrown, we do not
+        // want to return the partially fetched metadata entries
+        return [];
+      }
       for (let image of images) {
         switch (image.type) {
           case IMAGE_TYPES.favicon:
@@ -434,6 +457,7 @@ MetadataStore.prototype = {
         }
       }
     }
+
     return metaObjects;
   }),
 
@@ -447,15 +471,15 @@ MetadataStore.prototype = {
   *        an time interval in millisecond for this cron job
   */
   enableDataExpiryJob(interval) {
-    if (!this._conn) {
-      throw new Error("The database connection is not open yet");
-    }
-
     if (this._dataExpiryJob) {
       return;
     }
 
     this._dataExpiryJob = setInterval(() => {
+      if (!this._conn) {
+        return;  // ignore the callback if the connection is invalid
+      }
+
       this._conn.execute(SQL_DELETE_EXPIRED).catch(error => {
         // The delete might fail if a table dropping is being processed at
         // the same time

lib/main.js

@@ -4,13 +4,20 @@ const {PlacesProvider} = require("lib/PlacesProvider");
 const {SearchProvider} = require("lib/SearchProvider");
 const {MetadataStore} = require("lib/MetadataStore");
 const {ActivityStreams} = require("lib/ActivityStreams");
+const {setTimeout, clearTimeout} = require("sdk/timers");
 const {Cu} = require("chrome");
 
 Cu.import("resource://gre/modules/ClientID.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 
+// The constant to set the limit of MetadataStore reconnection
+// The addon will try reconnecting to the database in the next minute periodically,
+// if it fails to establish the connection in the addon initialization
+const kMaxConnectRetry = 120;
+
 let app = null;
 let metadataStore = null;
+let connectRetried = 0;
 
 Object.assign(exports, {
   main(options) {
@@ -23,21 +30,46 @@ Object.assign(exports, {
     Task.spawn(function*() {
       options.clientID = yield ClientID.getClientID();
       metadataStore = new MetadataStore();
-      yield metadataStore.asyncConnect();
+      try {
+        yield metadataStore.asyncConnect();
+      } catch (e) {
+        this.reconnectMetadataStore();
+      }
       app = new ActivityStreams(metadataStore, options);
     }.bind(this));
   },
 
+  reconnectMetadataStore() {
+    if (connectRetried > kMaxConnectRetry) {
+      throw new Error("Metadata store reconnecting has reached the maximum limit");
+    }
+
+    this.reconnectTimeoutID = setTimeout(function() {
+      metadataStore.asyncConnect().then(() => {connectRetried = 0;})
+      .catch(error => {
+        // increment the connect counter to avoid the endless retry
+        connectRetried++;
+        this.reconnectMetadataStore();
+      });
+    }.bind(this), 500);
+  },
+
   onUnload(reason) {
     if (app) {
       app.unload(reason);
       app = null;
     }
 
-    if (metadataStore && (reason === "uninstall" || reason === "disable")) {
-      metadataStore.asyncTearDown();
-    } else {
-      metadataStore.asyncClose();
+    if (this.reconnectTimeoutID) {
+      clearTimeout(this.reconnectTimeoutID);
+    }
+
+    if (metadataStore) {
+      if (reason === "uninstall" || reason === "disable") {
+        metadataStore.asyncTearDown();
+      } else {
+        metadataStore.asyncClose();
+      }
     }
 
     PlacesProvider.links.uninit();

test/test-MetadataStore.js

@@ -194,6 +194,30 @@ exports.test_async_get_by_cache_key_in_special_cases = function*(assert) {
   assert.ok(error, "It should raise exception on the invalid image type");
 };
 
+exports.test_on_an_invalid_connection = function*(assert) {
+  yield gMetadataStore.asyncClose();
+
+  let error = false;
+  try {
+    yield gMetadataStore.asyncExecuteQuery("SELECT * FROM page_metadata");
+  } catch (e) {
+    error = true;
+  }
+  assert.ok(error, "It should raise exception if the connection is closed or not established");
+
+  error = false;
+  try {
+    yield gMetadataStore.asycnInsert(metadataFixture);
+  } catch (e) {
+    error = true;
+  }
+  assert.ok(error, "It should raise exception if the connection is closed or not established");
+
+  let cacheKeys = metadataFixture.map(fixture => {return fixture.cache_key;});
+  let metaObjects = yield gMetadataStore.asyncGetMetadataByCacheKey(cacheKeys);
+  assert.equal(metaObjects.length, 0, "It should return an empty array if the connection is closed or not established");
+},
+
 exports.test_color_conversions = function*(assert) {
   const white = [0, 0, 0];
   const black = [255, 255, 255];