[core] Don't use exceptions in MapObserver::onDidFailLoadingMap

Using different exception pointers to specify the loading failure makes
an awkward API. Most users rethrow the exception only to figure out what
type of error happened so it can be reported properly. So replace the
exception pointer with a enum an string description of the failure.
This commit is contained in:
Ander Conselvan de Oliveira
2019-02-22 09:31:21 +02:00
parent 565792606d
commit 7d19115720
11 changed files with 93 additions and 61 deletions

View File

@@ -3,11 +3,17 @@
#include <mbgl/style/source.hpp>
#include <cstdint>
#include <exception>
#include <string>
namespace mbgl {
enum class MapLoadError {
StyleParseError,
StyleLoadError,
NotFoundError,
UnknownError,
};
class MapObserver {
public:
virtual ~MapObserver() = default;
@@ -32,7 +38,7 @@ public:
virtual void onCameraDidChange(CameraChangeMode) {}
virtual void onWillStartLoadingMap() {}
virtual void onDidFinishLoadingMap() {}
virtual void onDidFailLoadingMap(std::exception_ptr) {}
virtual void onDidFailLoadingMap(MapLoadError, const std::string&) {}
virtual void onWillStartRenderingFrame() {}
virtual void onDidFinishRenderingFrame(RenderMode) {}
virtual void onWillStartRenderingMap() {}

View File

@@ -164,13 +164,12 @@ void NativeMapView::onDidFinishLoadingMap() {
}
}
void NativeMapView::onDidFailLoadingMap(std::exception_ptr exceptionPtr) {
void NativeMapView::onDidFailLoadingMap(MapLoadError, const std::string& error) {
assert(vm != nullptr);
android::UniqueEnv _env = android::AttachEnv();
static auto& javaClass = jni::Class<NativeMapView>::Singleton(*_env);
static auto onDidFailLoadingMap = javaClass.GetMethod<void (jni::String)>(*_env, "onDidFailLoadingMap");
std::string error = mbgl::util::toString(exceptionPtr);
auto weakReference = javaPeer.get(*_env);
if (weakReference) {
weakReference.Call(*_env, onDidFailLoadingMap, jni::Make<jni::String>(*_env, error));

View File

@@ -62,7 +62,7 @@ public:
void onCameraDidChange(MapObserver::CameraChangeMode) override;
void onWillStartLoadingMap() override;
void onDidFinishLoadingMap() override;
void onDidFailLoadingMap(std::exception_ptr) override;
void onDidFailLoadingMap(MapLoadError, const std::string&) override;
void onWillStartRenderingFrame() override;
void onDidFinishRenderingFrame(MapObserver::RenderMode) override;
void onWillStartRenderingMap() override;

View File

@@ -6633,27 +6633,29 @@ public:
[nativeView mapViewDidFinishLoadingMap];
}
void onDidFailLoadingMap(std::exception_ptr exception) override {
void onDidFailLoadingMap(mbgl::MapLoadError mapError, const std::string& what) override {
NSString *description;
MGLErrorCode code;
try {
std::rethrow_exception(exception);
} catch (const mbgl::util::StyleParseException&) {
code = MGLErrorCodeParseStyleFailed;
description = NSLocalizedStringWithDefaultValue(@"PARSE_STYLE_FAILED_DESC", nil, nil, @"The map failed to load because the style is corrupted.", @"User-friendly error description");
} catch (const mbgl::util::StyleLoadException&) {
code = MGLErrorCodeLoadStyleFailed;
description = NSLocalizedStringWithDefaultValue(@"LOAD_STYLE_FAILED_DESC", nil, nil, @"The map failed to load because the style can't be loaded.", @"User-friendly error description");
} catch (const mbgl::util::NotFoundException&) {
code = MGLErrorCodeNotFound;
description = NSLocalizedStringWithDefaultValue(@"STYLE_NOT_FOUND_DESC", nil, nil, @"The map failed to load because the style can’t be found or is incompatible.", @"User-friendly error description");
} catch (...) {
code = MGLErrorCodeUnknown;
description = NSLocalizedStringWithDefaultValue(@"LOAD_MAP_FAILED_DESC", nil, nil, @"The map failed to load because an unknown error occurred.", @"User-friendly error description");
switch (mapError) {
case mbgl::MapLoadError::StyleParseError:
code = MGLErrorCodeParseStyleFailed;
description = NSLocalizedStringWithDefaultValue(@"PARSE_STYLE_FAILED_DESC", nil, nil, @"The map failed to load because the style is corrupted.", @"User-friendly error description");
break;
case mbgl::MapLoadError::StyleLoadError:
code = MGLErrorCodeLoadStyleFailed;
description = NSLocalizedStringWithDefaultValue(@"LOAD_STYLE_FAILED_DESC", nil, nil, @"The map failed to load because the style can't be loaded.", @"User-friendly error description");
break;
case mbgl::MapLoadError::NotFoundError:
code = MGLErrorCodeNotFound;
description = NSLocalizedStringWithDefaultValue(@"STYLE_NOT_FOUND_DESC", nil, nil, @"The map failed to load because the style can’t be found or is incompatible.", @"User-friendly error description");
break;
default:
code = MGLErrorCodeUnknown;
description = NSLocalizedStringWithDefaultValue(@"LOAD_MAP_FAILED_DESC", nil, nil, @"The map failed to load because an unknown error occurred.", @"User-friendly error description");
}
NSDictionary *userInfo = @{
NSLocalizedDescriptionKey: description,
NSLocalizedFailureReasonErrorKey: @(mbgl::util::toString(exception).c_str()),
NSLocalizedFailureReasonErrorKey: @(what.c_str()),
};
NSError *error = [NSError errorWithDomain:MGLErrorDomain code:code userInfo:userInfo];
[nativeView mapViewDidFailLoadingMapWithError:error];

View File

@@ -3062,27 +3062,29 @@ public:
[nativeView mapViewDidFinishLoadingMap];
}
void onDidFailLoadingMap(std::exception_ptr exception) override {
void onDidFailLoadingMap(mbgl::MapLoadError mapError, const std::string& what) override {
NSString *description;
MGLErrorCode code;
try {
std::rethrow_exception(exception);
} catch (const mbgl::util::StyleParseException&) {
code = MGLErrorCodeParseStyleFailed;
description = NSLocalizedStringWithDefaultValue(@"PARSE_STYLE_FAILED_DESC", nil, nil, @"The map failed to load because the style is corrupted.", @"User-friendly error description");
} catch (const mbgl::util::StyleLoadException&) {
code = MGLErrorCodeLoadStyleFailed;
description = NSLocalizedStringWithDefaultValue(@"LOAD_STYLE_FAILED_DESC", nil, nil, @"The map failed to load because the style can't be loaded.", @"User-friendly error description");
} catch (const mbgl::util::NotFoundException&) {
code = MGLErrorCodeNotFound;
description = NSLocalizedStringWithDefaultValue(@"STYLE_NOT_FOUND_DESC", nil, nil, @"The map failed to load because the style can’t be found or is incompatible.", @"User-friendly error description");
} catch (...) {
code = MGLErrorCodeUnknown;
description = NSLocalizedStringWithDefaultValue(@"LOAD_MAP_FAILED_DESC", nil, nil, @"The map failed to load because an unknown error occurred.", @"User-friendly error description");
switch (mapError) {
case mbgl::MapLoadError::StyleParseError:
code = MGLErrorCodeParseStyleFailed;
description = NSLocalizedStringWithDefaultValue(@"PARSE_STYLE_FAILED_DESC", nil, nil, @"The map failed to load because the style is corrupted.", @"User-friendly error description");
break;
case mbgl::MapLoadError::StyleLoadError:
code = MGLErrorCodeLoadStyleFailed;
description = NSLocalizedStringWithDefaultValue(@"LOAD_STYLE_FAILED_DESC", nil, nil, @"The map failed to load because the style can't be loaded.", @"User-friendly error description");
break;
case mbgl::MapLoadError::NotFoundError:
code = MGLErrorCodeNotFound;
description = NSLocalizedStringWithDefaultValue(@"STYLE_NOT_FOUND_DESC", nil, nil, @"The map failed to load because the style can’t be found or is incompatible.", @"User-friendly error description");
break;
default:
code = MGLErrorCodeUnknown;
description = NSLocalizedStringWithDefaultValue(@"LOAD_MAP_FAILED_DESC", nil, nil, @"The map failed to load because an unknown error occurred.", @"User-friendly error description");
}
NSDictionary *userInfo = @{
NSLocalizedDescriptionKey: description,
NSLocalizedFailureReasonErrorKey: @(mbgl::util::toString(exception).c_str()),
NSLocalizedFailureReasonErrorKey: @(what.c_str()),
};
NSError *error = [NSError errorWithDomain:MGLErrorDomain code:code userInfo:userInfo];
[nativeView mapViewDidFailLoadingMapWithError:error];

View File

@@ -3,8 +3,6 @@
#include "node_feature.hpp"
#include "node_conversion.hpp"
#include <mbgl/map/projection_mode.hpp>
#include <mbgl/util/exception.hpp>
#include <mbgl/renderer/renderer.hpp>
#include <mbgl/gl/headless_frontend.hpp>
#include <mbgl/style/conversion/source.hpp>
@@ -26,6 +24,7 @@
#include <mbgl/style/image.hpp>
#include <mbgl/style/light.hpp>
#include <mbgl/map/map_observer.hpp>
#include <mbgl/util/exception.hpp>
#include <mbgl/util/premultiply.hpp>
#include <unistd.h>
@@ -54,8 +53,14 @@ static const char* releasedMessage() {
return "Map resources have already been released";
}
void NodeMapObserver::onDidFailLoadingMap(std::exception_ptr error) {
std::rethrow_exception(error);
void NodeMapObserver::onDidFailLoadingMap(mbgl::MapLoadError error, const std::string& description) {
switch (error) {
case mbgl::MapLoadError::StyleParseError:
Nan::ThrowError(NodeMap::ParseError(description.c_str()));
break;
default:
Nan::ThrowError(description.c_str());
}
}
void NodeMap::Init(v8::Local<v8::Object> target) {

View File

@@ -22,7 +22,7 @@ class HeadlessFrontend;
namespace node_mbgl {
class NodeMapObserver : public mbgl::MapObserver {
void onDidFailLoadingMap(std::exception_ptr) override;
void onDidFailLoadingMap(mbgl::MapLoadError, const std::string&) override;
};
class NodeMap;

View File

@@ -48,27 +48,25 @@ void QMapboxGLMapObserver::onDidFinishLoadingMap()
emit mapChanged(QMapboxGL::MapChangeDidFinishLoadingMap);
}
void QMapboxGLMapObserver::onDidFailLoadingMap(std::exception_ptr exception)
void QMapboxGLMapObserver::onDidFailLoadingMap(mbgl::MapLoadError error, const std::string& what)
{
emit mapChanged(QMapboxGL::MapChangeDidFailLoadingMap);
QMapboxGL::MapLoadingFailure type;
QString description;
QString description(what.c_str());
try {
std::rethrow_exception(exception);
} catch (const mbgl::util::StyleParseException& e) {
type = QMapboxGL::MapLoadingFailure::StyleParseFailure;
description = e.what();
} catch (const mbgl::util::StyleLoadException& e) {
type = QMapboxGL::MapLoadingFailure::StyleLoadFailure;
description = e.what();
} catch (const mbgl::util::NotFoundException& e) {
type = QMapboxGL::MapLoadingFailure::NotFoundFailure;
description = e.what();
} catch (const std::exception& e) {
type = QMapboxGL::MapLoadingFailure::UnknownFailure;
description = e.what();
switch (error) {
case mbgl::MapLoadError::StyleParseError:
type = QMapboxGL::MapLoadingFailure::StyleParseFailure;
break;
case mbgl::MapLoadError::StyleLoadError:
type = QMapboxGL::MapLoadingFailure::StyleLoadFailure;
break;
case mbgl::MapLoadError::NotFoundError:
type = QMapboxGL::MapLoadingFailure::NotFoundFailure;
break;
default:
type = QMapboxGL::MapLoadingFailure::UnknownFailure;
}
emit mapLoadingFailed(type, description);

View File

@@ -26,7 +26,7 @@ public:
void onCameraDidChange(mbgl::MapObserver::CameraChangeMode) final;
void onWillStartLoadingMap() final;
void onDidFinishLoadingMap() final;
void onDidFailLoadingMap(std::exception_ptr) final;
void onDidFailLoadingMap(mbgl::MapLoadError, const std::string&) final;
void onWillStartRenderingFrame() final;
void onDidFinishRenderingFrame(mbgl::MapObserver::RenderMode) final;
void onWillStartRenderingMap() final;

View File

@@ -2,6 +2,7 @@
#include <mbgl/map/map_impl.hpp>
#include <mbgl/renderer/update_parameters.hpp>
#include <mbgl/style/style_impl.hpp>
#include <mbgl/util/exception.hpp>
namespace mbgl {
@@ -97,7 +98,26 @@ void Map::Impl::onStyleLoaded() {
}
void Map::Impl::onStyleError(std::exception_ptr error) {
observer.onDidFailLoadingMap(error);
MapLoadError type;
std::string description;
try {
std::rethrow_exception(error);
} catch (const mbgl::util::StyleParseException& e) {
type = MapLoadError::StyleParseError;
description = e.what();
} catch (const mbgl::util::StyleLoadException& e) {
type = MapLoadError::StyleLoadError;
description = e.what();
} catch (const mbgl::util::NotFoundException& e) {
type = MapLoadError::NotFoundError;
description = e.what();
} catch (const std::exception& e) {
type = MapLoadError::UnknownError;
description = e.what();
}
observer.onDidFailLoadingMap(type, description);
}
#pragma mark - Map::Impl RendererObserver

View File

@@ -20,7 +20,7 @@ public:
}
}
void onDidFailLoadingMap(std::exception_ptr) final {
void onDidFailLoadingMap(MapLoadError, const std::string&) final {
if (didFailLoadingMapCallback) {
didFailLoadingMapCallback();
}