[core] Improve OfflineDatabase error handling

- fixes missing exception handlers (e.g. runtim error from the `initialize()` method)
- introduces generic exception handling mechanism to reduce the repeated code

2 files changed, 62 insertions, 55 deletions

diff --git a/platform/default/include/mbgl/storage/offline_database.hpp b/platform/default/include/mbgl/storage/offline_database.hpp
index e599094a6d..f097cd5440 100644
--- a/platform/default/include/mbgl/storage/offline_database.hpp
+++ b/platform/default/include/mbgl/storage/offline_database.hpp
@@ -99,6 +99,7 @@ private:
     void handleError(const mapbox::sqlite::Exception&, const char* action);
     void handleError(const util::IOException&, const char* action);
     void handleError(const std::runtime_error& ex, const char* action);
+    void handleError(const char* action);
 
     void removeExisting();
     void removeOldCacheTable();
diff --git a/platform/default/src/mbgl/storage/offline_database.cpp b/platform/default/src/mbgl/storage/offline_database.cpp
index 5aa5738f41..4f813e906d 100644
--- a/platform/default/src/mbgl/storage/offline_database.cpp
+++ b/platform/default/src/mbgl/storage/offline_database.cpp
@@ -17,10 +17,8 @@ OfflineDatabase::OfflineDatabase(std::string path_)
     : path(std::move(path_)) {
     try {
         initialize();
-    } catch (const util::IOException& ex) {
-        handleError(ex, "open database");
-    } catch (const mapbox::sqlite::Exception& ex) {
-        handleError(ex, "open database");
+    } catch (...) {
+        handleError("open database");
     }
     // Assume that we can't open the database right now and work with an empty database object.
 }
@@ -80,10 +78,8 @@ void OfflineDatabase::cleanup() {
     try {
         statements.clear();
         db.reset();
-    } catch (const util::IOException& ex) {
-        handleError(ex, "close database");
-    } catch (const mapbox::sqlite::Exception& ex) {
-        handleError(ex, "close database");
+    } catch (...) {
+        handleError("close database");
     }
 }
 
@@ -125,7 +121,25 @@ void OfflineDatabase::handleError(const util::IOException& ex, const char* actio
 }
 
 void OfflineDatabase::handleError(const std::runtime_error& ex, const char* action) {
-    Log::Error(Event::Database, -1, "Can't %s: %s", action, ex.what());
+    Log::Error(Event::Database, "Can't %s: %s", action, ex.what());
+}
+
+void OfflineDatabase::handleError(const char* action) {
+    // Note: mbgl-defined exceptions must be handled first.
+    try {
+        throw;
+    } catch (const mapbox::sqlite::Exception& ex) {
+        handleError(ex, action);
+    } catch (const util::IOException& ex) {
+        handleError(ex, action);
+    } catch (const MapboxTileLimitExceededException& ex) {
+        throw; // This is ours and must be handled on client side.
+    } catch (const std::runtime_error& ex) {
+        handleError(ex, action);
+    } catch (...) {
+        assert(false);
+        Log::Error(Event::Database, "Can't %s", action);
+    }
 }
 
 void OfflineDatabase::removeExisting() {
@@ -209,11 +223,8 @@ optional<Response> OfflineDatabase::get(const Resource& resource) try {
 
     auto result = getInternal(resource);
     return result ? optional<Response>{ result->first } : nullopt;
-} catch (const util::IOException& ex) {
-    handleError(ex, "read resource");
-    return nullopt;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "read resource");
+} catch (...) {
+    handleError("read resource");
     return nullopt;
 }
 
@@ -248,9 +259,9 @@ std::pair<bool, uint64_t> OfflineDatabase::put(const Resource& resource, const R
     auto result = putInternal(resource, response, true);
     transaction.commit();
     return result;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "write resource");
-    return { false, 0 };
+} catch (...) {
+    handleError("write resource");
+    return {false, 0};
 }
 
 std::pair<bool, uint64_t> OfflineDatabase::putInternal(const Resource& resource, const Response& response, bool evict_) {
@@ -665,8 +676,8 @@ std::exception_ptr OfflineDatabase::invalidateAmbientCache() try {
     resourceQuery.run();
 
     return nullptr;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "invalidate ambient cache");
+} catch (...) {
+    handleError("invalidate ambient cache");
     return std::current_exception();
 }
 
@@ -696,8 +707,8 @@ std::exception_ptr OfflineDatabase::clearAmbientCache() try {
     vacuum();
 
     return nullptr;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "clear ambient cache");
+} catch (...) {
+    handleError("clear ambient cache");
     return std::current_exception();
 }
 
@@ -732,8 +743,8 @@ std::exception_ptr OfflineDatabase::invalidateRegion(int64_t regionID) try {
 
     assert(db);
     return nullptr;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "invalidate region");
+} catch (...) {
+    handleError("invalidate region");
     return std::current_exception();
 }
 
@@ -756,8 +767,8 @@ expected<OfflineRegions, std::exception_ptr> OfflineDatabase::listRegions() try
     }
     // Explicit move to avoid triggering the copy constructor.
     return { std::move(result) };
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "list regions");
+} catch (...) {
+    handleError("list regions");
     return unexpected<std::exception_ptr>(std::current_exception());
 }
 
@@ -774,8 +785,8 @@ OfflineDatabase::createRegion(const OfflineRegionDefinition& definition,
     query.bindBlob(2, metadata);
     query.run();
     return OfflineRegion(query.lastInsertRowId(), definition, metadata);
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "create region");
+} catch (...) {
+    handleError("create region");
     return unexpected<std::exception_ptr>(std::current_exception());
 }
 
@@ -867,8 +878,8 @@ OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetad
     query.run();
 
     return metadata;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "update region metadata");
+} catch (...) {
+    handleError("update region metadata");
     return unexpected<std::exception_ptr>(std::current_exception());
 }
 
@@ -886,22 +897,22 @@ std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region) try {
     // Ensure that the cached offlineTileCount value is recalculated.
     offlineMapboxTileCount = {};
     return nullptr;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "delete region");
+} catch (...) {
+    handleError("delete region");
     return std::current_exception();
 }
 
 optional<std::pair<Response, uint64_t>> OfflineDatabase::getRegionResource(const Resource& resource) try {
     return getInternal(resource);
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "read region resource");
+} catch (...) {
+    handleError("read region resource");
     return nullopt;
 }
 
 optional<int64_t> OfflineDatabase::hasRegionResource(const Resource& resource) try {
     return hasInternal(resource);
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "query region resource");
+} catch (...) {
+    handleError("query region resource");
     return nullopt;
 }
 
@@ -915,8 +926,8 @@ uint64_t OfflineDatabase::putRegionResource(int64_t regionID,
     auto size = putRegionResourceInternal(regionID, resource, response);
     transaction.commit();
     return size;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "write region resource");
+} catch (...) {
+    handleError("write region resource");
     return 0;
 }
 
@@ -961,8 +972,8 @@ void OfflineDatabase::putRegionResources(int64_t regionID,
     status.completedResourceSize += completedResourceSize;
     status.completedTileCount += completedTileCount;
     status.completedTileSize += completedTileSize;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "write region resources");
+} catch (...) {
+    handleError("write region resources");
 }
 
 uint64_t OfflineDatabase::putRegionResourceInternal(int64_t regionID, const Resource& resource, const Response& response) {
@@ -1070,13 +1081,8 @@ expected<OfflineRegionDefinition, std::exception_ptr> OfflineDatabase::getRegion
     query.run();
 
     return decodeOfflineRegionDefinition(query.get<std::string>(0));
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "load region");
-    return unexpected<std::exception_ptr>(std::current_exception());
-} catch (const std::runtime_error& ex) {
-    // Catch errors from malformed offline region definitions
-    // and skip them (as above).
-    handleError(ex, "load region");
+} catch (...) {
+    handleError("load region");
     return unexpected<std::exception_ptr>(std::current_exception());
 }
 
@@ -1092,8 +1098,8 @@ expected<OfflineRegionStatus, std::exception_ptr> OfflineDatabase::getRegionComp
     result.completedResourceSize += result.completedTileSize;
 
     return result;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "get region status");
+} catch (...) {
+    handleError("get region status");
     return unexpected<std::exception_ptr>(std::current_exception());
 }
 
@@ -1232,10 +1238,9 @@ std::exception_ptr OfflineDatabase::setMaximumAmbientCacheSize(uint64_t size) {
         }
 
         return nullptr;
-    } catch (const mapbox::sqlite::Exception& ex) {
+    } catch (...) {
         maximumAmbientCacheSize = previousMaximumAmbientCacheSize;
-        handleError(ex, "set maximum ambient cache size");
-
+        handleError("set maximum ambient cache size");
         return std::current_exception();
     }
 }
@@ -1274,8 +1279,8 @@ uint64_t OfflineDatabase::getOfflineMapboxTileCount() try {
 
     offlineMapboxTileCount = query.get<int64_t>(0);
     return *offlineMapboxTileCount;
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "get offline Mapbox tile count");
+} catch (...) {
+    handleError("get offline Mapbox tile count");
     return std::numeric_limits<uint64_t>::max();
 }
 
@@ -1294,8 +1299,8 @@ void OfflineDatabase::markUsedResources(int64_t regionID, const std::list<Resour
         markUsed(regionID, resource);
     }
     transaction.commit();
-} catch (const mapbox::sqlite::Exception& ex) {
-    handleError(ex, "mark resources as used");
+} catch (...) {
+    handleError("mark resources as used");
 }
 
 std::exception_ptr OfflineDatabase::resetDatabase() try {
@@ -1303,6 +1308,7 @@ std::exception_ptr OfflineDatabase::resetDatabase() try {
     initialize();
     return nullptr;
 } catch (...) {
+    handleError("reset database");
     return std::current_exception();
 }