diff options
author | Ander Conselvan de Oliveira <ander.deoliveira@mapbox.com> | 2019-02-22 09:31:21 +0200 |
---|---|---|
committer | Ander Conselvan de Oliveira <ander.deoliveira@mapbox.com> | 2019-03-13 11:58:27 +0200 |
commit | 7d1911572000d1353c1c0109402431323fcd8639 (patch) | |
tree | 4881a1e159415cd796e255bec951a7bb32b23ee8 | |
parent | 565792606d5d03d0cc9889f112bb50345c899005 (diff) | |
download | qtlocation-mapboxgl-7d1911572000d1353c1c0109402431323fcd8639.tar.gz |
[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.
-rw-r--r-- | include/mbgl/map/map_observer.hpp | 10 | ||||
-rwxr-xr-x | platform/android/src/native_map_view.cpp | 3 | ||||
-rwxr-xr-x | platform/android/src/native_map_view.hpp | 2 | ||||
-rw-r--r-- | platform/ios/src/MGLMapView.mm | 34 | ||||
-rw-r--r-- | platform/macos/src/MGLMapView.mm | 34 | ||||
-rw-r--r-- | platform/node/src/node_map.cpp | 13 | ||||
-rw-r--r-- | platform/node/src/node_map.hpp | 2 | ||||
-rw-r--r-- | platform/qt/src/qmapboxgl_map_observer.cpp | 32 | ||||
-rw-r--r-- | platform/qt/src/qmapboxgl_map_observer.hpp | 2 | ||||
-rw-r--r-- | src/mbgl/map/map_impl.cpp | 22 | ||||
-rw-r--r-- | test/src/mbgl/test/stub_map_observer.hpp | 2 |
11 files changed, 94 insertions, 62 deletions
diff --git a/include/mbgl/map/map_observer.hpp b/include/mbgl/map/map_observer.hpp index 98b218f8f0..8ad9e93d0b 100644 --- a/include/mbgl/map/map_observer.hpp +++ b/include/mbgl/map/map_observer.hpp @@ -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() {} diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp index 2f23f0b7d9..83a158efa9 100755 --- a/platform/android/src/native_map_view.cpp +++ b/platform/android/src/native_map_view.cpp @@ -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)); diff --git a/platform/android/src/native_map_view.hpp b/platform/android/src/native_map_view.hpp index 1bb4f23cbe..d695a91ce0 100755 --- a/platform/android/src/native_map_view.hpp +++ b/platform/android/src/native_map_view.hpp @@ -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; diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index c45f436c33..f6ebd8bced 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -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]; diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm index fa207ec187..7edf34574d 100644 --- a/platform/macos/src/MGLMapView.mm +++ b/platform/macos/src/MGLMapView.mm @@ -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]; diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp index d56d71ab6b..2beab1833f 100644 --- a/platform/node/src/node_map.cpp +++ b/platform/node/src/node_map.cpp @@ -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) { diff --git a/platform/node/src/node_map.hpp b/platform/node/src/node_map.hpp index 2214035b17..9e3eb1ad12 100644 --- a/platform/node/src/node_map.hpp +++ b/platform/node/src/node_map.hpp @@ -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; diff --git a/platform/qt/src/qmapboxgl_map_observer.cpp b/platform/qt/src/qmapboxgl_map_observer.cpp index 44cb8c41d5..4a842026cd 100644 --- a/platform/qt/src/qmapboxgl_map_observer.cpp +++ b/platform/qt/src/qmapboxgl_map_observer.cpp @@ -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; - - 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(); + QString description(what.c_str()); + + 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); diff --git a/platform/qt/src/qmapboxgl_map_observer.hpp b/platform/qt/src/qmapboxgl_map_observer.hpp index 98da5b6add..a12e5e9c70 100644 --- a/platform/qt/src/qmapboxgl_map_observer.hpp +++ b/platform/qt/src/qmapboxgl_map_observer.hpp @@ -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; diff --git a/src/mbgl/map/map_impl.cpp b/src/mbgl/map/map_impl.cpp index fc67d3cf56..863604f99c 100644 --- a/src/mbgl/map/map_impl.cpp +++ b/src/mbgl/map/map_impl.cpp @@ -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 diff --git a/test/src/mbgl/test/stub_map_observer.hpp b/test/src/mbgl/test/stub_map_observer.hpp index 1371577473..00a039e732 100644 --- a/test/src/mbgl/test/stub_map_observer.hpp +++ b/test/src/mbgl/test/stub_map_observer.hpp @@ -20,7 +20,7 @@ public: } } - void onDidFailLoadingMap(std::exception_ptr) final { + void onDidFailLoadingMap(MapLoadError, const std::string&) final { if (didFailLoadingMapCallback) { didFailLoadingMapCallback(); } |