make sqlite storage more resilient to sporadic errors

- catch SQLite exceptions and report them
- failed statements are ignored, we're really just caching here, so if it fails we're handling it gracefully elsewhere
- handle cases where the database file goes away after we opened it
- handle cases where the schema wasn't created after the database file was opened successfully
- add tests

2 files changed, 141 insertions, 100 deletions

diff --git a/platform/default/sqlite3.cpp b/platform/default/sqlite3.cpp
index 19ecaba0bf..b2ae4e65bc 100644
--- a/platform/default/sqlite3.cpp
+++ b/platform/default/sqlite3.cpp
@@ -76,11 +76,16 @@ Statement::Statement(sqlite3 *db, const char *sql) {
     }
 }
 
-#define CHECK_SQLITE_OK(err) \
+#define CHECK_SQLITE_OK_STMT(err, stmt) \
     if (err != SQLITE_OK) { \
         throw Exception { err, sqlite3_errmsg(sqlite3_db_handle(stmt)) }; \
     }
 
+#define CHECK_SQLITE_OK(err) \
+   if (err != SQLITE_OK) { \
+       throw Exception { err, sqlite3_errstr(err) }; \
+   }
+
 Statement::Statement(Statement &&other) {
     *this = std::move(other);
 }
@@ -92,8 +97,7 @@ Statement &Statement::operator=(Statement &&other) {
 
 Statement::~Statement() {
     if (stmt) {
-        const int err = sqlite3_finalize(stmt);
-        CHECK_SQLITE_OK(err)
+        sqlite3_finalize(stmt);
     }
 }
 
@@ -101,38 +105,38 @@ Statement::operator bool() const {
     return stmt != nullptr;
 }
 
-#define BIND_3(type, value) \
+#define BIND_3(type, value, stmt) \
     assert(stmt); \
     const int err = sqlite3_bind_##type(stmt, offset, value); \
-    CHECK_SQLITE_OK(err)
+    CHECK_SQLITE_OK_STMT(err, stmt)
 
-#define BIND_5(type, value, length, param) \
+#define BIND_5(type, value, length, param, stmt) \
     assert(stmt); \
     const int err = sqlite3_bind_##type(stmt, offset, value, length, param); \
-    CHECK_SQLITE_OK(err)
+    CHECK_SQLITE_OK_STMT(err, stmt)
 
 template <> void Statement::bind(int offset, int value) {
-    BIND_3(int, value)
+    BIND_3(int, value, stmt)
 }
 
 template <> void Statement::bind(int offset, int64_t value) {
-    BIND_3(int64, value)
+    BIND_3(int64, value, stmt)
 }
 
 template <> void Statement::bind(int offset, double value) {
-    BIND_3(double, value)
+    BIND_3(double, value, stmt)
 }
 
 template <> void Statement::bind(int offset, bool value) {
-    BIND_3(int, value)
+    BIND_3(int, value, stmt)
 }
 
 template <> void Statement::bind(int offset, const char *value) {
-    BIND_5(text, value, -1, nullptr)
+    BIND_5(text, value, -1, nullptr, stmt)
 }
 
 void Statement::bind(int offset, const std::string &value, bool retain) {
-    BIND_5(blob, value.data(), int(value.size()), retain ? SQLITE_TRANSIENT : SQLITE_STATIC)
+    BIND_5(blob, value.data(), int(value.size()), retain ? SQLITE_TRANSIENT : SQLITE_STATIC, stmt)
 }
 
 bool Statement::run() {
@@ -143,7 +147,8 @@ bool Statement::run() {
     } else if (err == SQLITE_ROW) {
         return true;
     } else {
-        throw std::runtime_error("failed to run statement");
+        CHECK_SQLITE_OK_STMT(err, stmt)
+        return false;
     }
 }
 
diff --git a/platform/default/sqlite_cache.cpp b/platform/default/sqlite_cache.cpp
index b8d47159ce..3f95db7c2d 100644
--- a/platform/default/sqlite_cache.cpp
+++ b/platform/default/sqlite_cache.cpp
@@ -9,6 +9,7 @@
 #include <mbgl/platform/log.hpp>
 
 #include "sqlite3.hpp"
+#include <sqlite3.h>
 
 #include <uv.h>
 
@@ -88,10 +89,10 @@ struct SQLiteCache::ActionDispatcher {
     template <typename T> void operator()(T &t) { cache.process(t); }
 };
 
-SQLiteCache::SQLiteCache(const std::string &path_)
+SQLiteCache::SQLiteCache(const std::string& path_)
     : path(path_),
       loop(uv_loop_new()),
-      queue(new Queue(loop, [this](Action &action) {
+      queue(new Queue(loop, [this](Action& action) {
           mapbox::util::apply_visitor(ActionDispatcher{ *this }, action);
       })),
       thread([this]() {
@@ -99,8 +100,16 @@ SQLiteCache::SQLiteCache(const std::string &path_)
           pthread_setname_np("SQLite Cache");
 #endif
           uv_run(loop, UV_RUN_DEFAULT);
-      })
-{
+
+          try {
+              getStmt.reset();
+              putStmt.reset();
+              refreshStmt.reset();
+              db.reset();
+          } catch (mapbox::sqlite::Exception& ex) {
+              Log::Error(Event::Database, ex.code, ex.what());
+          }
+      }) {
 }
 
 SQLiteCache::~SQLiteCache() {
@@ -140,6 +149,10 @@ void SQLiteCache::put(const Resource &resource, std::shared_ptr<const Response>
 void SQLiteCache::createDatabase() {
     db = util::make_unique<Database>(path.c_str(), ReadWrite | Create);
 
+    createSchema();
+}
+
+void SQLiteCache::createSchema() {
     constexpr const char *const sql = ""
         "CREATE TABLE IF NOT EXISTS `http_cache` ("
         "    `url` TEXT PRIMARY KEY NOT NULL,"
@@ -155,111 +168,134 @@ void SQLiteCache::createDatabase() {
 
     try {
         db->exec(sql);
-    } catch(mapbox::sqlite::Exception &) {
+        schema = true;
+    } catch(mapbox::sqlite::Exception &ex) {
+        Log::Error(Event::Database, ex.code, ex.what());
+
         // Creating the database table + index failed. That means there may already be one, likely
         // with different columsn. Drop it and try to create a new one.
-        try {
-            db->exec("DROP TABLE IF EXISTS `http_cache`");
-            db->exec(sql);
-        } catch (mapbox::sqlite::Exception &ex) {
-            Log::Error(Event::Database, "Failed to create database: %s", ex.what());
-            db.reset();
-        }
+        db->exec("DROP TABLE IF EXISTS `http_cache`");
+        db->exec(sql);
     }
 }
 
 void SQLiteCache::process(GetAction &action) {
-    // This is called in the SQLite event loop.
-    if (!db) {
-        createDatabase();
-    }
+    try {
+        // This is called in the SQLite event loop.
+        if (!db) {
+            createDatabase();
+        }
 
-    if (!getStmt) {
-        // Initialize the statement                                   0         1
-        getStmt = util::make_unique<Statement>(db->prepare("SELECT `status`, `modified`, "
-        //     2         3        4          5                                       1
-            "`etag`, `expires`, `data`, `compressed` FROM `http_cache` WHERE `url` = ?"));
-    } else {
-        getStmt->reset();
-    }
+        if (!schema) {
+            createSchema();
+        }
 
-    const std::string unifiedURL = unifyMapboxURLs(action.resource.url);
-    getStmt->bind(1, unifiedURL.c_str());
-    if (getStmt->run()) {
-        // There is data.
-        auto response = util::make_unique<Response>();
-        response->status = Response::Status(getStmt->get<int>(0));
-        response->modified = getStmt->get<int64_t>(1);
-        response->etag = getStmt->get<std::string>(2);
-        response->expires = getStmt->get<int64_t>(3);
-        response->data = getStmt->get<std::string>(4);
-        if (getStmt->get<int>(5)) { // == compressed
-            response->data = util::decompress(response->data);
+        if (!getStmt) {
+            // Initialize the statement                                   0         1
+            getStmt = util::make_unique<Statement>(db->prepare("SELECT `status`, `modified`, "
+            //     2         3        4          5                                       1
+                "`etag`, `expires`, `data`, `compressed` FROM `http_cache` WHERE `url` = ?"));
+        } else {
+            getStmt->reset();
         }
-        action.callback(std::move(response));
-    } else {
-        // There is no data.
+
+        const std::string unifiedURL = unifyMapboxURLs(action.resource.url);
+        getStmt->bind(1, unifiedURL.c_str());
+        if (getStmt->run()) {
+            // There is data.
+            auto response = util::make_unique<Response>();
+            response->status = Response::Status(getStmt->get<int>(0));
+            response->modified = getStmt->get<int64_t>(1);
+            response->etag = getStmt->get<std::string>(2);
+            response->expires = getStmt->get<int64_t>(3);
+            response->data = getStmt->get<std::string>(4);
+            if (getStmt->get<int>(5)) { // == compressed
+                response->data = util::decompress(response->data);
+            }
+            action.callback(std::move(response));
+        } else {
+            // There is no data.
+            action.callback(nullptr);
+        }
+    } catch (mapbox::sqlite::Exception& ex) {
+        Log::Error(Event::Database, ex.code, ex.what());
         action.callback(nullptr);
     }
 }
 
 void SQLiteCache::process(PutAction &action) {
-    if (!db) {
-        createDatabase();
-    }
+    try {
+        if (!db) {
+            createDatabase();
+        }
 
-    if (!putStmt) {
-        putStmt = util::make_unique<Statement>(db->prepare("REPLACE INTO `http_cache` ("
-        //     1       2       3         4         5         6        7          8
-            "`url`, `status`, `kind`, `modified`, `etag`, `expires`, `data`, `compressed`"
-            ") VALUES(?, ?, ?, ?, ?, ?, ?, ?)"));
-    } else {
-        putStmt->reset();
-    }
+        if (!schema) {
+            createSchema();
+        }
 
-    const std::string unifiedURL = unifyMapboxURLs(action.resource.url);
-    putStmt->bind(1 /* url */, unifiedURL.c_str());
-    putStmt->bind(2 /* status */, int(action.response->status));
-    putStmt->bind(3 /* kind */, int(action.resource.kind));
-    putStmt->bind(4 /* modified */, action.response->modified);
-    putStmt->bind(5 /* etag */, action.response->etag.c_str());
-    putStmt->bind(6 /* expires */, action.response->expires);
-
-    std::string data;
-    if (action.resource.kind != Resource::Image) {
-        // Do not compress images, since they are typically compressed already.
-        data = util::compress(action.response->data);
-    }
+        if (!putStmt) {
+            putStmt = util::make_unique<Statement>(db->prepare("REPLACE INTO `http_cache` ("
+            //     1       2       3         4         5         6        7          8
+                "`url`, `status`, `kind`, `modified`, `etag`, `expires`, `data`, `compressed`"
+                ") VALUES(?, ?, ?, ?, ?, ?, ?, ?)"));
+        } else {
+            putStmt->reset();
+        }
 
-    if (!data.empty() && data.size() < action.response->data.size()) {
-        // Store the compressed data when it is smaller than the original
-        // uncompressed data.
-        putStmt->bind(7 /* data */, data, false); // do not retain the string internally.
-        putStmt->bind(8 /* compressed */, true);
-    } else {
-        putStmt->bind(7 /* data */, action.response->data, false); // do not retain the string internally.
-        putStmt->bind(8 /* compressed */, false);
-    }
+        const std::string unifiedURL = unifyMapboxURLs(action.resource.url);
+        putStmt->bind(1 /* url */, unifiedURL.c_str());
+        putStmt->bind(2 /* status */, int(action.response->status));
+        putStmt->bind(3 /* kind */, int(action.resource.kind));
+        putStmt->bind(4 /* modified */, action.response->modified);
+        putStmt->bind(5 /* etag */, action.response->etag.c_str());
+        putStmt->bind(6 /* expires */, action.response->expires);
+
+        std::string data;
+        if (action.resource.kind != Resource::Image) {
+            // Do not compress images, since they are typically compressed already.
+            data = util::compress(action.response->data);
+        }
 
-    putStmt->run();
+        if (!data.empty() && data.size() < action.response->data.size()) {
+            // Store the compressed data when it is smaller than the original
+            // uncompressed data.
+            putStmt->bind(7 /* data */, data, false); // do not retain the string internally.
+            putStmt->bind(8 /* compressed */, true);
+        } else {
+            putStmt->bind(7 /* data */, action.response->data, false); // do not retain the string internally.
+            putStmt->bind(8 /* compressed */, false);
+        }
+
+        putStmt->run();
+    } catch (mapbox::sqlite::Exception& ex) {
+        Log::Error(Event::Database, ex.code, ex.what());
+    }
 }
 
 void SQLiteCache::process(RefreshAction &action) {
-    if (!db) {
-        createDatabase();
-    }
+    try {
+        if (!db) {
+            createDatabase();
+        }
 
-    if (!refreshStmt) {
-        refreshStmt = util::make_unique<Statement>( //       1               2
-            db->prepare("UPDATE `http_cache` SET `expires` = ? WHERE `url` = ?"));
-    } else {
-        refreshStmt->reset();
-    }
+        if (!schema) {
+            createSchema();
+        }
+
+        if (!refreshStmt) {
+            refreshStmt = util::make_unique<Statement>( //       1               2
+                db->prepare("UPDATE `http_cache` SET `expires` = ? WHERE `url` = ?"));
+        } else {
+            refreshStmt->reset();
+        }
 
-    const std::string unifiedURL = unifyMapboxURLs(action.resource.url);
-    refreshStmt->bind(1, int64_t(action.expires));
-    refreshStmt->bind(2, unifiedURL.c_str());
-    refreshStmt->run();
+        const std::string unifiedURL = unifyMapboxURLs(action.resource.url);
+        refreshStmt->bind(1, int64_t(action.expires));
+        refreshStmt->bind(2, unifiedURL.c_str());
+        refreshStmt->run();
+    } catch (mapbox::sqlite::Exception& ex) {
+        Log::Error(Event::Database, ex.code, ex.what());
+    }
 }
 
 void SQLiteCache::process(StopAction &) {