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

[WIP] Throw/notify on map loading error #3063

Closed
wants to merge 1 commit into from

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Nov 18, 2015

Throw an Objective-C exception whenever mbgl encounters an error. (Developers are trained to break on Objective-C exceptions as opposed to C++ exceptions.) If mbgl is unable to set up the map and the delegate implements the map loading failure callback method, call that method instead of throwing the exception.

/ref #2762, #2775
/cc @friedbunny @incanus

Throw an Objective-C exception whenever mbgl encounters an error. (Developers are trained to break on Objective-C exceptions as opposed to C++ exceptions.) If mbgl is unable to set up the map and the delegate implements the map loading failure callback method, call that method instead of throwing the exception.

ref #2762
@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold in progress labels Nov 18, 2015
@1ec5 1ec5 self-assigned this Nov 18, 2015
@@ -202,6 +203,9 @@ - (void)setStyleURL:(nullable NSURL *)styleURL
- (void)commonInit
{
_isTargetingInterfaceBuilder = NSProcessInfo.processInfo.mgl_isInterfaceBuilderDesignablesAgent;

std::unique_ptr<MGLLogObserver> logObserver = std::make_unique<MGLLogObserver>(self);
mbgl::Log::setObserver(std::move(logObserver));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mbgl::Log is a singleton, so this approach won’t scale to multiple MGLMapViews.

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 18, 2015

I considered calling view.notifyMapChange(mbgl::MapViewDidFailLoadingMap) directly from within MapContext::setStyleURL(), but that method provides no way to specify an error string.

bool onRecord(mbgl::EventSeverity severity, mbgl::Event event, int64_t code, const std::string &msg)
{
__block bool didNotifyDelegate = false;
if (severity == mbgl::EventSeverity::Error && event == mbgl::Event::Setup)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How closely does mbgl::Event::Setup correlate to map loading errors (as opposed to networking errors)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, it’s only used inside MapContext::setStyleURL(), when the style itself fails to load due to a failed HTTP request. Is that consistent with what you outlined in #2775 (comment)?

@jfirebaugh
Copy link
Contributor

Closing as stale.

@jfirebaugh jfirebaugh closed this Jan 3, 2017
@1ec5 1ec5 deleted the 1ec5-did-fail-to-delegate-2762 branch January 3, 2017 23:24
@1ec5 1ec5 restored the 1ec5-did-fail-to-delegate-2762 branch February 16, 2017 23:13
@jfirebaugh jfirebaugh deleted the 1ec5-did-fail-to-delegate-2762 branch May 11, 2017 19:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants