[core] don't store error state

Previously, we could first get a stale response from cache, then immediately a "connection error" response that overwrote the error state variable. In the tileLoadingCompleteCallback, we'd only see the second overwritten error state and abort. This could lead to the map sometimes not being rendered at all until the user interacts with the map.

8 files changed, 35 insertions, 36 deletions

diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp
index da2dd808bd..92ffb451b0 100644
--- a/src/mbgl/map/raster_tile_data.cpp
+++ b/src/mbgl/map/raster_tile_data.cpp
@@ -27,6 +27,7 @@ void RasterTileData::request(const std::string& url,
     FileSource* fs = util::ThreadContext::getFileSource();
     req = fs->request({ Resource::Kind::Tile, url }, [url, callback, this](Response res) {
         if (res.error) {
+            std::exception_ptr error;
             if (res.error->reason == Response::Error::Reason::NotFound) {
                 // This is a 404 response. We're treating these as empty tiles.
                 workRequest.reset();
@@ -36,7 +37,7 @@ void RasterTileData::request(const std::string& url,
                 // This is a different error, e.g. a connection or server error.
                 error = std::make_exception_ptr(std::runtime_error(res.error->message));
             }
-            callback();
+            callback(error);
             return;
         }
 
@@ -60,6 +61,7 @@ void RasterTileData::request(const std::string& url,
                 return;
             }
 
+            std::exception_ptr error;
             if (result.is<std::unique_ptr<Bucket>>()) {
                 state = State::parsed;
                 bucket = std::move(result.get<std::unique_ptr<Bucket>>());
@@ -69,7 +71,7 @@ void RasterTileData::request(const std::string& url,
                 bucket.reset();
             }
 
-            callback();
+            callback(error);
         });
     });
 }
diff --git a/src/mbgl/map/raster_tile_data.hpp b/src/mbgl/map/raster_tile_data.hpp
index 34a79cad4c..e8efde48e4 100644
--- a/src/mbgl/map/raster_tile_data.hpp
+++ b/src/mbgl/map/raster_tile_data.hpp
@@ -16,7 +16,7 @@ public:
     RasterTileData(const TileID&, TexturePool&, Worker&);
     ~RasterTileData();
 
-    using Callback = std::function<void()>;
+    using Callback = std::function<void(std::exception_ptr)>;
 
     void request(const std::string& url,
                  const Callback& callback);
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index 613972e930..e34ba0748c 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -225,7 +225,12 @@ bool Source::handlePartialTile(const TileID& tileID, Worker&) {
         return true;
     }
 
-    return tileData->parsePending([this, tileID]() {
+    return tileData->parsePending([this, tileID](std::exception_ptr error) {
+        if (error) {
+            observer->onTileError(*this, tileID, error);
+            return;
+        }
+
         observer->onTileLoaded(*this, tileID, false);
     });
 }
@@ -259,7 +264,9 @@ TileData::State Source::addTile(const TileID& tileID, const StyleUpdateParameter
     }
 
     if (!newTile->data) {
-        auto callback = std::bind(&Source::tileLoadingCompleteCallback, this, normalizedID, parameters.transformState, parameters.debugOptions & MapDebugOptions::Collision);
+        auto callback = std::bind(&Source::tileLoadingCompleteCallback, this, normalizedID,
+                                  std::placeholders::_1, parameters.transformState,
+                                  parameters.debugOptions & MapDebugOptions::Collision);
 
         // If we don't find working tile data, we're just going to load it.
         if (type == SourceType::Raster) {
@@ -520,7 +527,10 @@ void Source::setObserver(Observer* observer_) {
     observer = observer_;
 }
 
-void Source::tileLoadingCompleteCallback(const TileID& tileID, const TransformState& transformState, bool collisionDebug) {
+void Source::tileLoadingCompleteCallback(const TileID& tileID,
+                                         std::exception_ptr error,
+                                         const TransformState& transformState,
+                                         bool collisionDebug) {
     auto it = tileDataMap.find(tileID);
     if (it == tileDataMap.end()) {
         return;
@@ -531,8 +541,8 @@ void Source::tileLoadingCompleteCallback(const TileID& tileID, const TransformSt
         return;
     }
 
-    if (tileData->hasError()) {
-        observer->onTileError(*this, tileID, tileData->getError());
+    if (error) {
+        observer->onTileError(*this, tileID, error);
         return;
     }
 
diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp
index f3688ed9e8..ade2317cc5 100644
--- a/src/mbgl/map/source.hpp
+++ b/src/mbgl/map/source.hpp
@@ -76,7 +76,10 @@ public:
     bool enabled = false;
 
 private:
-    void tileLoadingCompleteCallback(const TileID&, const TransformState&, bool collisionDebug);
+    void tileLoadingCompleteCallback(const TileID&,
+                                     std::exception_ptr,
+                                     const TransformState&,
+                                     bool collisionDebug);
     bool handlePartialTile(const TileID&, Worker& worker);
     bool findLoadedChildren(const TileID&, int32_t maxCoveringZoom, std::forward_list<TileID>& retain);
     void findLoadedParent(const TileID&, int32_t minCoveringZoom, std::forward_list<TileID>& retain);
diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp
index d9fd3f0047..edfe4d7ada 100644
--- a/src/mbgl/map/tile_data.cpp
+++ b/src/mbgl/map/tile_data.cpp
@@ -27,10 +27,6 @@ const char* TileData::StateToString(const State state) {
 void TileData::dumpDebugLogs() const {
     Log::Info(Event::General, "TileData::id: %s", std::string(id).c_str());
     Log::Info(Event::General, "TileData::state: %s", TileData::StateToString(state));
-
-    if (error) {
-        Log::Info(Event::General, "TileData::error: %s", util::toString(error).c_str());
-    }
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp
index e31cced964..1067c86d3d 100644
--- a/src/mbgl/map/tile_data.hpp
+++ b/src/mbgl/map/tile_data.hpp
@@ -77,7 +77,7 @@ public:
 
     virtual Bucket* getBucket(const StyleLayer&) = 0;
 
-    virtual bool parsePending(std::function<void ()>) { return true; }
+    virtual bool parsePending(std::function<void (std::exception_ptr)>) { return true; }
     virtual void redoPlacement(PlacementConfig) {}
 
     bool isReady() const {
@@ -88,14 +88,6 @@ public:
         return state;
     }
 
-    bool hasError() const {
-        return error.operator bool();
-    }
-
-    std::exception_ptr getError() const {
-        return error;
-    }
-
     void dumpDebugLogs() const;
 
     const TileID id;
@@ -107,7 +99,6 @@ public:
 
 protected:
     std::atomic<State> state;
-    std::exception_ptr error;
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp
index 386efe1afa..9e1390decd 100644
--- a/src/mbgl/map/vector_tile_data.cpp
+++ b/src/mbgl/map/vector_tile_data.cpp
@@ -13,7 +13,7 @@ VectorTileData::VectorTileData(const TileID& id_,
                                std::string sourceID,
                                Style& style_,
                                const MapMode mode_,
-                               const std::function<void()>& callback)
+                               const std::function<void(std::exception_ptr)>& callback)
     : TileData(id_),
       style(style_),
       worker(style_.workers),
@@ -32,15 +32,10 @@ VectorTileData::VectorTileData(const TileID& id_,
                                                         Seconds modified_,
                                                         Seconds expires_) {
         if (err) {
-            error = err;
-            callback();
+            callback(err);
             return;
         }
 
-        // Reset the error if we didn't receive one for this request. This is to prevent old error
-        // messages from being displayed again, e.g. after a connection trouble cleared up.
-        error = nullptr;
-
         modified = modified_;
         expires = expires_;
 
@@ -49,7 +44,7 @@ VectorTileData::VectorTileData(const TileID& id_,
             workRequest.reset();
             state = State::parsed;
             buckets.clear();
-            callback();
+            callback(err);
             return;
         }
 
@@ -69,6 +64,7 @@ VectorTileData::VectorTileData(const TileID& id_,
                 return;
             }
 
+            std::exception_ptr error;
             if (result.is<TileParseResultBuckets>()) {
                 auto& resultBuckets = result.get<TileParseResultBuckets>();
                 state = resultBuckets.state;
@@ -91,7 +87,7 @@ VectorTileData::VectorTileData(const TileID& id_,
                 state = State::obsolete;
             }
 
-            callback();
+            callback(error);
         });
     });
 }
@@ -100,7 +96,7 @@ VectorTileData::~VectorTileData() {
     cancel();
 }
 
-bool VectorTileData::parsePending(std::function<void()> callback) {
+bool VectorTileData::parsePending(std::function<void(std::exception_ptr)> callback) {
     if (workRequest) {
         // There's already parsing or placement going on.
         return false;
@@ -113,6 +109,7 @@ bool VectorTileData::parsePending(std::function<void()> callback) {
             return;
         }
 
+        std::exception_ptr error;
         if (result.is<TileParseResultBuckets>()) {
             auto& resultBuckets = result.get<TileParseResultBuckets>();
             state = resultBuckets.state;
@@ -137,7 +134,7 @@ bool VectorTileData::parsePending(std::function<void()> callback) {
             state = State::obsolete;
         }
 
-        callback();
+        callback(error);
     });
 
     return true;
diff --git a/src/mbgl/map/vector_tile_data.hpp b/src/mbgl/map/vector_tile_data.hpp
index 3d877de600..3ba572f42a 100644
--- a/src/mbgl/map/vector_tile_data.hpp
+++ b/src/mbgl/map/vector_tile_data.hpp
@@ -23,13 +23,13 @@ public:
                    std::string sourceID,
                    Style&,
                    const MapMode,
-                   const std::function<void()>& callback);
+                   const std::function<void(std::exception_ptr)>& callback);
 
     ~VectorTileData();
 
     Bucket* getBucket(const StyleLayer&) override;
 
-    bool parsePending(std::function<void()> callback) override;
+    bool parsePending(std::function<void(std::exception_ptr)> callback) override;
 
     void redoPlacement(PlacementConfig config) override;
     void redoPlacement();