Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Improve OfflineDatabase error handling #15906

Merged
merged 1 commit into from
Nov 11, 2019
Merged

[core] Improve OfflineDatabase error handling #15906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions platform/default/include/mbgl/storage/offline_database.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class OfflineDatabase : private util::noncopyable {
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();
Expand Down
116 changes: 61 additions & 55 deletions platform/default/src/mbgl/storage/offline_database.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
Expand Down Expand Up @@ -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");
}
}

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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_) {
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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();
}

Expand All @@ -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());
}

Expand All @@ -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());
}

Expand Down Expand Up @@ -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());
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
}

Expand All @@ -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());
}

Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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();
}

Expand All @@ -1294,15 +1299,16 @@ 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 {
removeExisting();
initialize();
return nullptr;
} catch (...) {
handleError("reset database");
return std::current_exception();
}

Expand Down