[core] move retry logic to DefaultFileSource

13 files changed, 299 insertions, 230 deletions

diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index b6ebffe2e2..8935457ca5 100644
--- a/src/mbgl/map/map_context.cpp
+++ b/src/mbgl/map/map_context.cpp
@@ -112,14 +112,17 @@ void MapContext::setStyleURL(const std::string& url) {
         }
         styleRequest = nullptr;
 
-        if (res.status == Response::Successful) {
-            loadStyleJSON(*res.data, base);
-        } else if (res.status == Response::NotFound && styleURL.find("mapbox://") == 0) {
-            Log::Error(Event::Setup, "style %s could not be found or is an incompatible legacy map or style", styleURL.c_str());
+        if (res.error) {
+            if (res.error->reason == Response::Error::Reason::NotFound && styleURL.find("mapbox://") == 0) {
+                Log::Error(Event::Setup, "style %s could not be found or is an incompatible legacy map or style", styleURL.c_str());
+            } else {
+                Log::Error(Event::Setup, "loading style failed: %s", res.error->message.c_str());
+                data.loading = false;
+            }
         } else {
-            Log::Error(Event::Setup, "loading style failed: %s", res.message.c_str());
-            data.loading = false;
+            loadStyleJSON(*res.data, base);
         }
+
     });
 }
 
diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp
index 2427e9f55d..775619ff05 100644
--- a/src/mbgl/map/raster_tile_data.cpp
+++ b/src/mbgl/map/raster_tile_data.cpp
@@ -37,17 +37,15 @@ void RasterTileData::request(float pixelRatio,
         }
         req = nullptr;
 
-        if (res.status == Response::NotFound) {
-            state = State::parsed;
-            callback();
-            return;
-        }
-
-        if (res.status != Response::Successful) {
-            std::stringstream message;
-            message <<  "Failed to load [" << url << "]: " << res.message;
-            error = message.str();
-            state = State::obsolete;
+        if (res.error) {
+            if (res.error->reason == Response::Error::Reason::NotFound) {
+                state = State::parsed;
+            } else {
+                std::stringstream message;
+                message <<  "Failed to load [" << url << "]: " << res.error->message;
+                error = message.str();
+                state = State::obsolete;
+            }
             callback();
             return;
         }
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index 9a916537aa..a6fd9665bb 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -155,9 +155,9 @@ void Source::load() {
         }
         req = nullptr;
 
-        if (res.status != Response::Successful) {
+        if (res.error) {
             std::stringstream message;
-            message <<  "Failed to load [" << info.url << "]: " << res.message;
+            message <<  "Failed to load [" << info.url << "]: " << res.error->message;
             emitSourceLoadingFailed(message.str());
             return;
         }
diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp
index 84e01324b5..518979c5f0 100644
--- a/src/mbgl/map/sprite.cpp
+++ b/src/mbgl/map/sprite.cpp
@@ -47,13 +47,14 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_)
             return;
         }
         loader->jsonRequest = nullptr;
-        if (res.status == Response::Successful) {
-            loader->json = res.data;
-        } else {
+
+        if (res.error) {
             std::stringstream message;
-            message << "Failed to load [" << jsonURL << "]: " << res.message;
+            message << "Failed to load [" << jsonURL << "]: " << res.error->message;
             emitSpriteLoadingFailed(message.str());
             return;
+        } else {
+            loader->json = res.data;
         }
         emitSpriteLoadedIfComplete();
     });
@@ -66,13 +67,14 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_)
                 return;
             }
             loader->spriteRequest = nullptr;
-            if (res.status == Response::Successful) {
-                loader->image = res.data;
-            } else {
+
+            if (res.error) {
                 std::stringstream message;
-                message << "Failed to load [" << spriteURL << "]: " << res.message;
+                message << "Failed to load [" << spriteURL << "]: " << res.error->message;
                 emitSpriteLoadingFailed(message.str());
                 return;
+            } else {
+                loader->image = res.data;
             }
             emitSpriteLoadedIfComplete();
         });
diff --git a/src/mbgl/map/vector_tile.cpp b/src/mbgl/map/vector_tile.cpp
index 7e301c91d9..b044250f8c 100644
--- a/src/mbgl/map/vector_tile.cpp
+++ b/src/mbgl/map/vector_tile.cpp
@@ -189,16 +189,16 @@ Request* VectorTileMonitor::monitorTile(std::function<void (std::exception_ptr,
             return;
         }
 
-        if (res.status == Response::NotFound) {
-            callback(nullptr, nullptr);
-            return;
-        }
-
-        if (res.status != Response::Successful) {
-            std::stringstream message;
-            message << "Failed to load [" << url << "]: " << res.message;
-            callback(std::make_exception_ptr(std::runtime_error(message.str())), nullptr);
-            return;
+        if (res.error) {
+            if (res.error->reason == Response::Error::Reason::NotFound) {
+                callback(nullptr, nullptr);
+                return;
+            } else {
+                std::stringstream message;
+                message << "Failed to load [" << url << "]: " << res.error->message;
+                callback(std::make_exception_ptr(std::runtime_error(message.str())), nullptr);
+                return;
+            }
         }
 
         data = res.data;
diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp
index 3e4c94ce40..773875c390 100644
--- a/src/mbgl/storage/default_file_source.cpp
+++ b/src/mbgl/storage/default_file_source.cpp
@@ -23,22 +23,22 @@
 #include <algorithm>
 #include <cassert>
 
-
 namespace algo = boost::algorithm;
 
 namespace mbgl {
 
 DefaultFileSource::DefaultFileSource(FileCache* cache, const std::string& root)
-    : thread(std::make_unique<util::Thread<Impl>>(util::ThreadContext{"FileSource", util::ThreadType::Unknown, util::ThreadPriority::Low}, cache, root)) {
+    : thread(std::make_unique<util::Thread<Impl>>(
+          util::ThreadContext{ "FileSource", util::ThreadType::Unknown, util::ThreadPriority::Low },
+          cache,
+          root)) {
 }
 
 DefaultFileSource::~DefaultFileSource() {
     MBGL_VERIFY_THREAD(tid);
 }
 
-Request* DefaultFileSource::request(const Resource& resource,
-                                    uv_loop_t* l,
-                                    Callback callback) {
+Request* DefaultFileSource::request(const Resource& resource, uv_loop_t* l, Callback callback) {
     assert(l);
 
     if (!callback) {
@@ -74,7 +74,7 @@ Request* DefaultFileSource::request(const Resource& resource,
     return req;
 }
 
-void DefaultFileSource::cancel(Request *req) {
+void DefaultFileSource::cancel(Request* req) {
     assert(req);
     req->cancel();
     thread->invoke(&Impl::cancel, req);
@@ -90,56 +90,37 @@ DefaultFileSource::Impl::Impl(FileCache* cache_, const std::string& root)
       httpContext(HTTPContextBase::createContext(loop)) {
 }
 
-DefaultFileRequest* DefaultFileSource::Impl::find(const Resource& resource) {
-    const auto it = pending.find(resource);
-    if (it != pending.end()) {
-        return &it->second;
-    }
-    return nullptr;
-}
-
 void DefaultFileSource::Impl::add(Request* req) {
-    const Resource& resource = req->resource;
-    DefaultFileRequest* request = find(resource);
+    auto& request = pending.emplace(req->resource, req->resource).first->second;
 
-    if (!request) {
-        request = &pending.emplace(resource, resource).first->second;
-    }
+    // Trigger a potentially required refresh of this Request
+    update(request);
 
     // Add this request as an observer so that it'll get notified when something about this
     // request changes.
-    request->observers.insert(req);
-
-    update(request);
-
-    if (request->response) {
-        // We've got a response, so send the (potentially stale) response to the requester.
-        req->notify(request->response);
-    }
+    request.addObserver(req);
 }
 
-void DefaultFileSource::Impl::update(DefaultFileRequest* request) {
-    if (request->response) {
+void DefaultFileSource::Impl::update(DefaultFileRequest& request) {
+    if (request.getResponse()) {
         // We've at least obtained a cache value, potentially we also got a final response.
         // The observers have been notified already; send what we have to the new one as well.
 
-        // Before returning the existing response, make sure that it is still fresh.
-        if (!request->response->stale && request->response->isExpired()) {
-            // Create a new Response object with `stale = true`, but the same data, and
-            // replace the current request object we have.
-            auto response = std::make_shared<Response>(*request->response);
-            response->stale = true;
-            request->response = response;
-        }
+        // Before returning the existing response, make sure that it is still fresh, or update the
+        // `stale` flag.
+        request.checkResponseFreshness();
 
-        if (request->response->stale && !request->realRequest) {
+        if (request.getResponse()->stale && !request.realRequest) {
             // We've returned a stale response; now make sure the requester also gets a fresh
             // response eventually. It's possible that there's already a request in progress.
             // Note that this will also trigger updates to all other existing listeners.
             // Since we already have data, we're going to verify
-            startRealRequest(request, request->response);
+            startRealRequest(request);
+        } else {
+            // The response is still fresh (or there's already a request for refreshing the resource
+            // in progress), so there's nothing we need to do.
         }
-    } else if (!request->cacheRequest && !request->realRequest) {
+    } else if (!request.cacheRequest && !request.realRequest) {
         // There is no request in progress, and we don't have a response yet. This means we'll have
         // to start the request ourselves.
         if (cache) {
@@ -152,60 +133,71 @@ void DefaultFileSource::Impl::update(DefaultFileRequest* request) {
     }
 }
 
-void DefaultFileSource::Impl::startCacheRequest(DefaultFileRequest* request) {
+void DefaultFileSource::Impl::startCacheRequest(DefaultFileRequest& request) {
     // Check the cache for existing data so that we can potentially
     // revalidate the information without having to redownload everything.
-    request->cacheRequest = cache->get(request->resource, [this, request](std::shared_ptr<Response> response) {
-        request->cacheRequest = nullptr;
-        if (response) {
-            response->stale = response->isExpired();
-        }
+    request.cacheRequest =
+        cache->get(request.resource, [this, &request](std::shared_ptr<Response> response) {
+            request.cacheRequest = nullptr;
+            if (response) {
+                response->stale = response->isExpired();
+                request.setResponse(response);
+            }
 
-        if (!response || response->stale) {
-            // No response or stale cache. Run the real request.
-            startRealRequest(request, response);
-        }
+            if (!response || response->stale) {
+                // No response or stale cache. Run the real request.
+                startRealRequest(request);
+            }
 
-        if (response) {
             // Notify in all cases; requestors can decide whether they want to use stale responses.
-            notify(request, response, FileCache::Hint::No);
-        }
-    });
+            request.notify();
+
+            reschedule(request);
+        });
 }
 
-void DefaultFileSource::Impl::startRealRequest(DefaultFileRequest* request, std::shared_ptr<const Response> response) {
+void DefaultFileSource::Impl::startRealRequest(DefaultFileRequest& request) {
     // Cancel the timer if we have one.
-    if (request->timerRequest) {
-        request->timerRequest->stop();
+    if (request.timerRequest) {
+        request.timerRequest->stop();
     }
 
-    auto callback = [request, this] (std::shared_ptr<const Response> res, FileCache::Hint hint) {
-        request->realRequest = nullptr;
-        notify(request, res, hint);
+    auto callback = [this, &request](std::shared_ptr<const Response> response) {
+        request.realRequest = nullptr;
+
+        if (cache) {
+            // Store response in database. Make sure we only refresh the expires column if the data
+            // didn't change.
+            FileCache::Hint hint = FileCache::Hint::Full;
+            if (request.getResponse() && response->data == request.getResponse()->data) {
+                hint = FileCache::Hint::Refresh;
+            }
+            cache->put(request.resource, response, hint);
+        }
+
+        request.setResponse(response);
+        request.notify();
+        reschedule(request);
     };
 
-    if (algo::starts_with(request->resource.url, "asset://")) {
-        request->realRequest = assetContext->createRequest(request->resource, callback, loop, assetRoot);
+    if (algo::starts_with(request.resource.url, "asset://")) {
+        request.realRequest =
+            assetContext->createRequest(request.resource, callback, loop, assetRoot);
     } else {
-        request->realRequest = httpContext->createRequest(request->resource, callback, loop, response);
+        request.realRequest =
+            httpContext->createRequest(request.resource, callback, loop, request.getResponse());
     }
 }
 
 void DefaultFileSource::Impl::cancel(Request* req) {
-    DefaultFileRequest* request = find(req->resource);
-
-    if (request) {
+    auto it = pending.find(req->resource);
+    if (it != pending.end()) {
         // If the number of dependent requests of the DefaultFileRequest drops to zero,
         // cancel the request and remove it from the pending list.
-        request->observers.erase(req);
-        if (request->observers.empty()) {
-            if (request->cacheRequest) {
-                request->cacheRequest.reset();
-            }
-            if (request->realRequest) {
-                request->realRequest->cancel();
-            }
-            pending.erase(request->resource);
+        auto& request = it->second;
+        request.removeObserver(req);
+        if (!request.hasObservers()) {
+            pending.erase(it);
         }
     } else {
         // There is no request for this URL anymore. Likely, the request already completed
@@ -217,42 +209,134 @@ void DefaultFileSource::Impl::cancel(Request* req) {
     req->destruct();
 }
 
-void DefaultFileSource::Impl::notify(DefaultFileRequest* request, std::shared_ptr<const Response> response, FileCache::Hint hint) {
-    // First, remove the request, since it might be destructed at any point now.
-    assert(find(request->resource) == request);
-    assert(response);
+void DefaultFileSource::Impl::reschedule(DefaultFileRequest& request) {
+    if (request.realRequest) {
+        // There's already a request in progress; don't start another one.
+        return;
+    }
+
+    const auto timeout = request.getRetryTimeout();
+
+    if (timeout == 0) {
+        update(request);
+    } else if (timeout > 0) {
+        if (!request.timerRequest) {
+            request.timerRequest = std::make_unique<uv::timer>(util::RunLoop::getLoop());
+        }
+
+        // timeout is in seconds, but the timer takes milliseconds.
+        request.timerRequest->start(1000 * timeout, 0, [this, &request] {
+            assert(!request.realRequest);
+            startRealRequest(request);
+        });
+    }
+}
+
+// ----- DefaultFileRequest -----
+
+DefaultFileRequest::~DefaultFileRequest() {
+    if (realRequest) {
+        realRequest->cancel();
+        realRequest = nullptr;
+    }
+    // timerRequest and cacheRequest are automatically canceld upon destruction.
+}
 
-    // Notify all observers.
-    request->response = response;
-    for (auto req : request->observers) {
+void DefaultFileRequest::addObserver(Request* req) {
+    observers.insert(req);
+
+    if (response) {
+        // We've got a response, so send the (potentially stale) response to the requester.
         req->notify(response);
     }
+}
+
+void DefaultFileRequest::removeObserver(Request* req) {
+    observers.erase(req);
+}
+
+bool DefaultFileRequest::hasObservers() const {
+    return !observers.empty();
+}
 
-    if (cache) {
-        // Store response in database
-        cache->put(request->resource, response, hint);
+void DefaultFileRequest::notify() {
+    if (response) {
+        for (auto req : observers) {
+            req->notify(response);
+        }
     }
+}
 
-    // Set timer for requests that have a known expiry times. Expiry times of 0 are technically
-    // expiring immediately, but we can't continually request.
-    if (!request->realRequest && response->expires > 0) {
-        const int64_t now = std::chrono::duration_cast<std::chrono::seconds>(
-                                    SystemClock::now().time_since_epoch()).count();
-        const int64_t timeout = response->expires - now;
+void DefaultFileRequest::setResponse(const std::shared_ptr<const Response>& response_) {
+    response = response_;
 
-        if (timeout <= 0) {
-            update(request);
-        } else {
-            if (!request->timerRequest) {
-                request->timerRequest = std::make_unique<uv::timer>(util::RunLoop::getLoop());
-            }
+    if (response->error) {
+        failedRequests++;
+    } else {
+        // Reset the number of subsequent failed requests after we got a successful one.
+        failedRequests = 0;
+    }
+}
+
+const std::shared_ptr<const Response>& DefaultFileRequest::getResponse() const {
+    return response;
+}
 
-            // timeout is in seconds, but the timer takes milliseconds.
-            request->timerRequest->start(1000 * timeout, 0, [this, request] {
-                update(request);
-            });
+int64_t DefaultFileRequest::getRetryTimeout() const {
+    if (!response) {
+        // If we don't have a response, we should retry immediately.
+        return 0;
+    }
+
+    // A value < 0 means that we should not retry.
+    int64_t timeout = -1;
+
+    if (response->error) {
+        assert(failedRequests > 0);
+        switch (response->error->reason) {
+        case Response::Error::Reason::Server: {
+            // Retry immediately, unless we have a certain number of attempts already
+            const int graceRetries = 3;
+            if (failedRequests <= graceRetries) {
+                timeout = 1;
+            } else {
+                timeout = 1 << std::min(failedRequests - graceRetries, 32);
+            }
+        } break;
+        case Response::Error::Reason::Connection: {
+            // Exponential backoff
+            timeout = 1 << std::min(failedRequests - 1, 32);
+        } break;
+        default:
+            // Do not retry due to error.
+            break;
         }
     }
+
+    // Check to see if this response expires earlier than a potential error retry.
+    if (response->expires > 0) {
+        const int64_t expires =
+            response->expires -
+            std::chrono::duration_cast<std::chrono::seconds>(SystemClock::now().time_since_epoch())
+                .count();
+        // Only update the timeout if we don't have one yet, and only if the new timeout is shorter
+        // than the previous one.
+        timeout = timeout < 0 ? expires : std::min(timeout, std::max<int64_t>(0, expires));
+    }
+
+    return timeout;
 }
 
+void DefaultFileRequest::checkResponseFreshness() {
+    if (response && !response->stale && response->isExpired()) {
+        // Create a new Response object with `stale = true`, but the same data, and
+        // replace the current request object we have.
+        // We're not immediately swapping the member variable because it's declared as `const`, and
+        // we first have to update the `stale` flag.
+        auto staleResponse = std::make_shared<Response>(*response);
+        staleResponse->stale = true;
+        response = staleResponse;
+    }
 }
+
+} // namespace mbgl
diff --git a/src/mbgl/storage/default_file_source_impl.hpp b/src/mbgl/storage/default_file_source_impl.hpp
index 387c6ce66e..2e5ca93ccc 100644
--- a/src/mbgl/storage/default_file_source_impl.hpp
+++ b/src/mbgl/storage/default_file_source_impl.hpp
@@ -12,11 +12,9 @@ namespace mbgl {
 
 class RequestBase;
 
-struct DefaultFileRequest {
+class DefaultFileRequest {
+public:
     const Resource resource;
-    std::set<Request*> observers;
-    std::shared_ptr<const Response> response;
-
     std::unique_ptr<WorkRequest> cacheRequest;
     RequestBase* realRequest = nullptr;
     std::unique_ptr<uv::timer> timerRequest;
@@ -24,11 +22,48 @@ struct DefaultFileRequest {
     inline DefaultFileRequest(const Resource& resource_)
         : resource(resource_) {}
 
-    // Make it movable-only
+public:
+    // Make it movable-only.
     DefaultFileRequest(const DefaultFileRequest&) = delete;
     inline DefaultFileRequest(DefaultFileRequest&&) = default;
     DefaultFileRequest& operator=(const DefaultFileRequest&) = delete;
     inline DefaultFileRequest& operator=(DefaultFileRequest&&) = default;
+    ~DefaultFileRequest();
+
+    // Observer accessors.
+    void addObserver(Request*);
+    void removeObserver(Request*);
+    bool hasObservers() const;
+
+    // Updates/gets the response of this request object.
+    void setResponse(const std::shared_ptr<const Response>&);
+    const std::shared_ptr<const Response>& getResponse() const;
+
+    // Returns the seconds we have to wait until we need to redo this request. A value of 0
+    // means that we need to redo it immediately, and a negative value means that we're not setting
+    // a timeout at all.
+    int64_t getRetryTimeout() const;
+
+    // Checks the currently stored response and replaces it with an idential one, except with the
+    // stale flag set, if the response is expired.
+    void checkResponseFreshness();
+
+    // Notifies all observers.
+    void notify();
+
+
+private:
+    // Stores a set of all observing Request objects.
+    std::set<Request*> observers;
+
+    // The current response data. We're storing it because we can satisfy requests for the same
+    // resource directly by returning this response object. We also need it to create conditional
+    // HTTP requests, and to check whether new responses we got changed any data.
+    std::shared_ptr<const Response> response;
+
+    // Counts the number of subsequent failed requests. We're using this value for exponential
+    // backoff when retrying requests.
+    int failedRequests = 0;
 };
 
 class DefaultFileSource::Impl {
@@ -39,19 +74,17 @@ public:
     void cancel(Request*);
 
 private:
-    DefaultFileRequest* find(const Resource&);
-
-    void update(DefaultFileRequest*);
-    void startCacheRequest(DefaultFileRequest*);
-    void startRealRequest(DefaultFileRequest*, std::shared_ptr<const Response> = nullptr);
-    void notify(DefaultFileRequest*, std::shared_ptr<const Response>, FileCache::Hint);
+    void update(DefaultFileRequest&);
+    void startCacheRequest(DefaultFileRequest&);
+    void startRealRequest(DefaultFileRequest&);
+    void reschedule(DefaultFileRequest&);
 
     std::unordered_map<Resource, DefaultFileRequest, Resource::Hash> pending;
-    uv_loop_t* loop = nullptr;
-    FileCache* cache = nullptr;
+    uv_loop_t* const loop;
+    FileCache* const cache;
     const std::string assetRoot;
-    std::unique_ptr<AssetContextBase> assetContext;
-    std::unique_ptr<HTTPContextBase> httpContext;
+    const std::unique_ptr<AssetContextBase> assetContext;
+    const std::unique_ptr<HTTPContextBase> httpContext;
 };
 
 }
diff --git a/src/mbgl/storage/http_context_base.cpp b/src/mbgl/storage/http_context_base.cpp
index 8b09fc4dc2..a25f1b9185 100644
--- a/src/mbgl/storage/http_context_base.cpp
+++ b/src/mbgl/storage/http_context_base.cpp
@@ -2,29 +2,4 @@
 
 namespace mbgl {
 
-HTTPContextBase::HTTPContextBase(uv_loop_t* loop_)
-    : reachability(loop_, [this] { retryRequests(); }) {
-    NetworkStatus::Subscribe(reachability.get());
-    reachability.unref();
-}
-
-HTTPContextBase::~HTTPContextBase() {
-    assert(requests.empty());
-    NetworkStatus::Unsubscribe(reachability.get());
-}
-
-void HTTPContextBase::addRequest(HTTPRequestBase* request) {
-    requests.insert(request);
-}
-
-void HTTPContextBase::removeRequest(HTTPRequestBase* request) {
-    requests.erase(request);
-}
-
-void HTTPContextBase::retryRequests() {
-    for (auto request : requests) {
-        request->retry();
-    }
-}
-
 }
diff --git a/src/mbgl/storage/http_context_base.hpp b/src/mbgl/storage/http_context_base.hpp
index 66da59bb11..fd88d7dba9 100644
--- a/src/mbgl/storage/http_context_base.hpp
+++ b/src/mbgl/storage/http_context_base.hpp
@@ -14,26 +14,10 @@ class HTTPContextBase {
 public:
     static std::unique_ptr<HTTPContextBase> createContext(uv_loop_t*);
 
-    HTTPContextBase(uv_loop_t*);
-    virtual ~HTTPContextBase();
-
     virtual HTTPRequestBase* createRequest(const Resource&,
                                        RequestBase::Callback,
                                        uv_loop_t*,
                                        std::shared_ptr<const Response>) = 0;
-
-    void addRequest(HTTPRequestBase*);
-    void removeRequest(HTTPRequestBase*);
-
-private:
-    void retryRequests();
-
-    // Will be fired when the network status becomes reachable.
-    uv::async reachability;
-
-    // A list of all pending HTTPRequestImpls that we need to notify when the network status
-    // changes.
-    std::set<HTTPRequestBase*> requests;
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/storage/http_request_base.hpp b/src/mbgl/storage/http_request_base.hpp
index b28cb9fcc1..4c296050c7 100644
--- a/src/mbgl/storage/http_request_base.hpp
+++ b/src/mbgl/storage/http_request_base.hpp
@@ -5,35 +5,7 @@
 
 namespace mbgl {
 
-enum class ResponseStatus : uint8_t {
-    // This error probably won't be resolved by retrying anytime soon. We are giving up.
-    PermanentError,
-
-    // This error might be resolved by waiting some time (e.g. server issues).
-    // We are going to do an exponential back-off and will try again in a few seconds.
-    TemporaryError,
-
-    // This error was caused by a temporary error and it is likely that it will be resolved
-    // immediately. We are going to try again right away. This is like the TemporaryError, except
-    // that we will not perform exponential back-off.
-    SingularError,
-
-    // This error might be resolved once the network reachability status changes.
-    // We are going to watch the network status for changes and will retry as soon as the
-    // operating system notifies us of a network status change.
-    ConnectionError,
-
-    // The request was canceled mid-way.
-    Canceled,
-
-    // The request returned data successfully. We retrieved and decoded the data successfully.
-    Successful,
-
-    // The request confirmed that the data wasn't changed. We already have the data.
-    NotModified,
-};
-
-class HTTPRequestBase : public RequestBase {
+    class HTTPRequestBase : public RequestBase {
 public:
     HTTPRequestBase(const Resource& resource_, Callback notify_)
         : RequestBase(resource_, notify_)
@@ -42,8 +14,6 @@ public:
 
     virtual ~HTTPRequestBase() = default;
     virtual void cancel() override { cancelled = true; };
-    virtual void retry(uint64_t timeout) = 0;
-    virtual void retry() = 0;
 
 protected:
     static int64_t parseCacheControl(const char *value);
diff --git a/src/mbgl/storage/request_base.hpp b/src/mbgl/storage/request_base.hpp
index a147ccacf0..0f3e257ad1 100644
--- a/src/mbgl/storage/request_base.hpp
+++ b/src/mbgl/storage/request_base.hpp
@@ -14,7 +14,7 @@ class Response;
 
 class RequestBase : private util::noncopyable {
 public:
-    using Callback = std::function<void (std::shared_ptr<const Response> response, FileCache::Hint hint)>;
+    using Callback = std::function<void (std::shared_ptr<const Response> response)>;
 
     RequestBase(const Resource& resource_, Callback notify_)
         : resource(resource_)
diff --git a/src/mbgl/storage/response.cpp b/src/mbgl/storage/response.cpp
index 628a2a3b99..1dcd397b62 100644
--- a/src/mbgl/storage/response.cpp
+++ b/src/mbgl/storage/response.cpp
@@ -3,10 +3,30 @@
 
 namespace mbgl {
 
+Response::Response(const Response& res) {
+    *this = res;
+}
+
+Response& Response::operator=(const Response& res) {
+    error = res.error ? std::make_unique<Error>(*res.error) : nullptr;
+    stale = res.stale;
+    data = res.data;
+    modified = res.modified;
+    expires = res.expires;
+    etag = res.etag;
+    return *this;
+}
+
 bool Response::isExpired() const {
-    const int64_t now = std::chrono::duration_cast<std::chrono::seconds>(
-                                    SystemClock::now().time_since_epoch()).count();
+    const int64_t now =
+        std::chrono::duration_cast<std::chrono::seconds>(SystemClock::now().time_since_epoch())
+            .count();
+    // Note: expires == 0 also counts as expired!
     return expires <= now;
 }
 
+Response::Error::Error(Reason reason_, const std::string& message_)
+    : reason(reason_), message(message_) {
+}
+
 } // namespace mbgl
diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp
index 66008adb96..66327b333f 100644
--- a/src/mbgl/text/glyph_pbf.cpp
+++ b/src/mbgl/text/glyph_pbf.cpp
@@ -81,9 +81,9 @@ GlyphPBF::GlyphPBF(GlyphStore* store,
         }
         req = nullptr;
 
-        if (res.status != Response::Successful) {
+        if (res.error) {
             std::stringstream message;
-            message <<  "Failed to load [" << url << "]: " << res.message;
+            message <<  "Failed to load [" << url << "]: " << res.error->message;
             emitGlyphPBFLoadingFailed(message.str());
         } else {
             data = res.data;