diff options
-rw-r--r-- | platform/node/src/node_request.cpp | 11 | ||||
-rw-r--r-- | src/mbgl/storage/default_file_source.cpp | 27 | ||||
-rw-r--r-- | src/mbgl/storage/default_file_source_impl.hpp | 4 | ||||
-rw-r--r-- | test/storage/http_error.cpp | 3 | ||||
-rw-r--r-- | test/storage/http_retry_network_status.cpp | 57 |
5 files changed, 92 insertions, 10 deletions
diff --git a/platform/node/src/node_request.cpp b/platform/node/src/node_request.cpp index 211a696cd7..14af3ad5eb 100644 --- a/platform/node/src/node_request.cpp +++ b/platform/node/src/node_request.cpp @@ -65,16 +65,17 @@ NAN_METHOD(NodeRequest::Respond) { if (info.Length() < 1) { auto response = std::make_shared<mbgl::Response>(); - response->status = mbgl::Response::NotFound; + using Error = mbgl::Response::Error; + response->error = std::make_unique<Error>(Error::Reason::NotFound); source->notify(*resource, response); } else if (info[0]->BooleanValue()) { auto response = std::make_shared<mbgl::Response>(); - response->status = mbgl::Response::Error; - // Store the error string. const Nan::Utf8String message { info[0]->ToString() }; - response->message = std::string { *message, size_t(message.length()) }; + using Error = mbgl::Response::Error; + response->error = std::make_unique<Error>( + Error::Reason::Other, std::string{ *message, size_t(message.length()) }); source->notify(*resource, response); } else if (info.Length() < 2 || !info[1]->IsObject()) { @@ -83,8 +84,6 @@ NAN_METHOD(NodeRequest::Respond) { auto response = std::make_shared<mbgl::Response>(); auto res = info[1]->ToObject(); - response->status = mbgl::Response::Successful; - if (Nan::Has(res, Nan::New("modified").ToLocalChecked()).FromJust()) { const double modified = Nan::Get(res, Nan::New("modified").ToLocalChecked()).ToLocalChecked()->ToNumber()->Value(); if (!std::isnan(modified)) { diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp index 773875c390..64c802d770 100644 --- a/src/mbgl/storage/default_file_source.cpp +++ b/src/mbgl/storage/default_file_source.cpp @@ -2,6 +2,7 @@ #include <mbgl/storage/request.hpp> #include <mbgl/storage/asset_context_base.hpp> #include <mbgl/storage/http_context_base.hpp> +#include <mbgl/storage/network_status.hpp> #include <mbgl/storage/response.hpp> #include <mbgl/platform/platform.hpp> @@ -87,7 +88,29 @@ DefaultFileSource::Impl::Impl(FileCache* cache_, const std::string& root) cache(cache_), assetRoot(root.empty() ? platform::assetRoot() : root), assetContext(AssetContextBase::createContext(loop)), - httpContext(HTTPContextBase::createContext(loop)) { + httpContext(HTTPContextBase::createContext(loop)), + reachability(std::make_unique<uv::async>(loop, std::bind(&Impl::networkIsReachableAgain, this))) { + // Subscribe to network status changes, but make sure that this async handle doesn't keep the + // loop alive; otherwise our app wouldn't terminate. After all, we only need status change + // notifications when our app is still running. + NetworkStatus::Subscribe(reachability->get()); + reachability->unref(); +} + +DefaultFileSource::Impl::~Impl() { + NetworkStatus::Unsubscribe(reachability->get()); +} + +void DefaultFileSource::Impl::networkIsReachableAgain() { + for (auto& req : pending) { + auto& request = req.second; + auto& response = request.getResponse(); + if (!request.realRequest && response && response->error && response->error->reason == Response::Error::Reason::Connection) { + // We need all requests to fail at least once before we are going to start retrying + // them, and we only immediately restart request that failed due to connection issues. + startRealRequest(request); + } + } } void DefaultFileSource::Impl::add(Request* req) { @@ -157,6 +180,8 @@ void DefaultFileSource::Impl::startCacheRequest(DefaultFileRequest& request) { } void DefaultFileSource::Impl::startRealRequest(DefaultFileRequest& request) { + assert(!request.realRequest); + // Cancel the timer if we have one. if (request.timerRequest) { request.timerRequest->stop(); diff --git a/src/mbgl/storage/default_file_source_impl.hpp b/src/mbgl/storage/default_file_source_impl.hpp index 2e5ca93ccc..4ca8e42721 100644 --- a/src/mbgl/storage/default_file_source_impl.hpp +++ b/src/mbgl/storage/default_file_source_impl.hpp @@ -69,6 +69,9 @@ private: class DefaultFileSource::Impl { public: Impl(FileCache*, const std::string& = ""); + ~Impl(); + + void networkIsReachableAgain(); void add(Request*); void cancel(Request*); @@ -85,6 +88,7 @@ private: const std::string assetRoot; const std::unique_ptr<AssetContextBase> assetContext; const std::unique_ptr<HTTPContextBase> httpContext; + const std::unique_ptr<uv::async> reachability; }; } diff --git a/test/storage/http_error.cpp b/test/storage/http_error.cpp index 00deeb063a..94a3f29a4e 100644 --- a/test/storage/http_error.cpp +++ b/test/storage/http_error.cpp @@ -92,7 +92,4 @@ TEST_F(Storage, HTTPError) { }); uv_run(uv_default_loop(), UV_RUN_DEFAULT); - - // Run again so that the timer handle can be properly closed. - uv_run(uv_default_loop(), UV_RUN_DEFAULT); } diff --git a/test/storage/http_retry_network_status.cpp b/test/storage/http_retry_network_status.cpp index d0209e0bf6..3f8c469cf9 100644 --- a/test/storage/http_retry_network_status.cpp +++ b/test/storage/http_retry_network_status.cpp @@ -47,3 +47,60 @@ TEST_F(Storage, HTTPNetworkStatusChange) { uv_run(uv_default_loop(), UV_RUN_DEFAULT); } + +// Tests that a change in network status preempts requests that failed due to connection or +// reachability issues. +TEST_F(Storage, HTTPNetworkStatusChangePreempt) { + SCOPED_TEST(HTTPNetworkStatusChangePreempt) + + using namespace mbgl; + + DefaultFileSource fs(nullptr); + + const auto start = uv_hrtime(); + + const Resource resource{ Resource::Unknown, "http://127.0.0.1:3001/test" }; + Request* req = fs.request(resource, uv_default_loop(), [&](const Response& res) { + static int counter = 0; + const auto duration = double(uv_hrtime() - start) / 1e9; + if (counter == 0) { + EXPECT_GT(0.2, duration) << "Response came in too late"; + } else if (counter == 1) { + EXPECT_LT(0.39, duration) << "Preempted retry triggered too early"; + EXPECT_GT(0.6, duration) << "Preempted retry triggered too late"; + } else if (counter > 1) { + FAIL() << "Retried too often"; + } + ASSERT_NE(nullptr, res.error); + EXPECT_EQ(Response::Error::Reason::Connection, res.error->reason); +#ifdef MBGL_HTTP_NSURL + EXPECT_TRUE(res.error->message == + "The operation couldn’t be completed. (NSURLErrorDomain error -1004.)" || + res.error->message == "Could not connect to the server.") + << "Full message is: \"" << res.error->message << "\""; +#elif MBGL_HTTP_CURL + const std::string prefix { "Couldn't connect to server: " }; + EXPECT_STREQ(prefix.c_str(), res.error->message.substr(0, prefix.size()).c_str()) << "Full message is: \"" << res.error->message << "\""; +#else + FAIL(); +#endif + EXPECT_EQ(false, res.stale); + ASSERT_FALSE(res.data.get()); + EXPECT_EQ(0, res.expires); + EXPECT_EQ(0, res.modified); + EXPECT_EQ("", res.etag); + + if (counter++ == 1) { + fs.cancel(req); + HTTPNetworkStatusChangePreempt.finish(); + } + }); + + // After 400 milliseconds, we're going to trigger a NetworkStatus change. + uv::timer reachableTimer(uv_default_loop()); + reachableTimer.start(400, 0, [] () { + mbgl::NetworkStatus::Reachable(); + }); + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); +} |