diff options
39 files changed, 607 insertions, 621 deletions
diff --git a/include/mbgl/storage/file_cache.hpp b/include/mbgl/storage/file_cache.hpp index a9071b1b67..b32bdf67e6 100644 --- a/include/mbgl/storage/file_cache.hpp +++ b/include/mbgl/storage/file_cache.hpp @@ -16,7 +16,7 @@ class FileCache : private util::noncopyable { public: virtual ~FileCache() = default; - enum class Hint : uint8_t { Full, Refresh, No }; + enum class Hint : bool { Full, Refresh }; using Callback = std::function<void(std::unique_ptr<Response>)>; virtual std::unique_ptr<WorkRequest> get(const Resource &resource, Callback callback) = 0; diff --git a/include/mbgl/storage/response.hpp b/include/mbgl/storage/response.hpp index b5973457b5..8ab6170ba2 100644 --- a/include/mbgl/storage/response.hpp +++ b/include/mbgl/storage/response.hpp @@ -8,20 +8,47 @@ namespace mbgl { class Response { public: + Response() = default; + Response(const Response&); + Response& operator=(const Response&); + bool isExpired() const; public: - enum Status { Error, Successful, NotFound }; + class Error; + // When this object is empty, the response was successful. + std::unique_ptr<const Error> error; - Status status = Error; + // Stale responses are fetched from cache and are expired. bool stale = false; - std::string message; + + // The actual data of the response. This is guaranteed to never be empty. + std::shared_ptr<const std::string> data; + int64_t modified = 0; int64_t expires = 0; std::string etag; - std::shared_ptr<const std::string> data; }; -} +class Response::Error { +public: + enum class Reason : uint8_t { + // Success = 1, // Reserve 1 for Success. + NotFound = 2, + Server = 3, + Connection = 4, + Canceled = 5, + Other = 6, + } reason = Reason::Other; + + // An error message from the request handler, e.g. a server message or a system message + // informing the user about the reason for the failure. + std::string message; + +public: + Error(Reason, const std::string& = ""); +}; + +} // namespace mbgl #endif diff --git a/platform/darwin/http_request_nsurl.mm b/platform/darwin/http_request_nsurl.mm index 10f1d0450e..5c78bfc90f 100644 --- a/platform/darwin/http_request_nsurl.mm +++ b/platform/darwin/http_request_nsurl.mm @@ -25,33 +25,24 @@ public: ~HTTPNSURLRequest(); void cancel() final; - void retry() final; private: - void start(); void handleResult(NSData *data, NSURLResponse *res, NSError *error); void handleResponse(); - void retry(uint64_t timeout) final; HTTPNSURLContext *context = nullptr; bool cancelled = false; NSURLSessionDataTask *task = nullptr; std::unique_ptr<Response> response; const std::shared_ptr<const Response> existingResponse; - ResponseStatus status = ResponseStatus::PermanentError; uv::async async; - uv::timer timer; - int attempts = 0; - enum : bool { PreemptImmediately, ExponentialBackoff } strategy = PreemptImmediately; - - static const int maxAttempts = 4; }; // ------------------------------------------------------------------------------------------------- class HTTPNSURLContext : public HTTPContextBase { public: - HTTPNSURLContext(uv_loop_t *loop); + HTTPNSURLContext(); ~HTTPNSURLContext(); HTTPRequestBase* createRequest(const Resource&, @@ -64,10 +55,10 @@ public: NSInteger accountType = 0; }; -HTTPNSURLContext::HTTPNSURLContext(uv_loop_t *loop_) : HTTPContextBase(loop_) { +HTTPNSURLContext::HTTPNSURLContext() { @autoreleasepool { - NSURLSessionConfiguration *sessionConfig = - [NSURLSessionConfiguration defaultSessionConfiguration]; + NSURLSessionConfiguration* sessionConfig = + [NSURLSessionConfiguration defaultSessionConfiguration]; sessionConfig.timeoutIntervalForResource = 30; sessionConfig.HTTPMaximumConnectionsPerHost = 8; sessionConfig.requestCachePolicy = NSURLRequestReloadIgnoringLocalCacheData; @@ -78,7 +69,7 @@ HTTPNSURLContext::HTTPNSURLContext(uv_loop_t *loop_) : HTTPContextBase(loop_) { // Write user agent string userAgent = @"MapboxGL"; - + accountType = [[NSUserDefaults standardUserDefaults] integerForKey:@"MGLMapboxAccountType"]; } } @@ -100,43 +91,29 @@ HTTPRequestBase* HTTPNSURLContext::createRequest(const Resource& resource, // ------------------------------------------------------------------------------------------------- -HTTPNSURLRequest::HTTPNSURLRequest(HTTPNSURLContext* context_, const Resource& resource_, Callback callback_, uv_loop_t *loop, std::shared_ptr<const Response> existingResponse_) +HTTPNSURLRequest::HTTPNSURLRequest(HTTPNSURLContext* context_, + const Resource& resource_, + Callback callback_, + uv_loop_t* loop, + std::shared_ptr<const Response> existingResponse_) : HTTPRequestBase(resource_, callback_), context(context_), existingResponse(existingResponse_), - async(loop, [this] { handleResponse(); }), - timer(loop) { - context->addRequest(this); - start(); -} - -HTTPNSURLRequest::~HTTPNSURLRequest() { - assert(!task); - - // Stop the backoff timer to avoid re-triggering this request. - timer.stop(); - - context->removeRequest(this); -} - -void HTTPNSURLRequest::start() { - assert(!task); - - attempts++; - + async(loop, [this] { handleResponse(); }) { @autoreleasepool { - NSURL *url = [NSURL URLWithString:@(resource.url.c_str())]; + NSURL* url = [NSURL URLWithString:@(resource.url.c_str())]; if (context->accountType == 0 && ([url.host isEqualToString:@"mapbox.com"] || [url.host hasSuffix:@".mapbox.com"])) { - NSString *absoluteString = [url.absoluteString stringByAppendingFormat: - (url.query ? @"&%@" : @"?%@"), @"events=true"]; + NSString* absoluteString = [url.absoluteString + stringByAppendingFormat:(url.query ? @"&%@" : @"?%@"), @"events=true"]; url = [NSURL URLWithString:absoluteString]; } - NSMutableURLRequest *req = [NSMutableURLRequest requestWithURL:url]; + NSMutableURLRequest* req = [NSMutableURLRequest requestWithURL:url]; if (existingResponse) { if (!existingResponse->etag.empty()) { - [req addValue:@(existingResponse->etag.c_str()) forHTTPHeaderField:@"If-None-Match"]; + [req addValue:@(existingResponse->etag.c_str()) + forHTTPHeaderField:@"If-None-Match"]; } else if (existingResponse->modified) { const std::string time = util::rfc1123(existingResponse->modified); [req addValue:@(time.c_str()) forHTTPHeaderField:@"If-Modified-Since"]; @@ -145,14 +122,20 @@ void HTTPNSURLRequest::start() { [req addValue:context->userAgent forHTTPHeaderField:@"User-Agent"]; - task = [context->session dataTaskWithRequest:req - completionHandler:^(NSData *data, NSURLResponse *res, - NSError *error) { handleResult(data, res, error); }]; + task = [context->session + dataTaskWithRequest:req + completionHandler:^(NSData* data, NSURLResponse* res, NSError* error) { + handleResult(data, res, error); + }]; [task retain]; [task resume]; } } +HTTPNSURLRequest::~HTTPNSURLRequest() { + assert(!task); +} + void HTTPNSURLRequest::handleResponse() { if (task) { [task release]; @@ -160,34 +143,16 @@ void HTTPNSURLRequest::handleResponse() { } if (!cancelled) { - if (status == ResponseStatus::TemporaryError && attempts < maxAttempts) { - strategy = ExponentialBackoff; - return retry((1 << (attempts - 1)) * 1000); - } else if (status == ResponseStatus::ConnectionError && attempts < maxAttempts) { - // By default, we will retry every 30 seconds (network change notification will - // preempt the timeout). - strategy = PreemptImmediately; - return retry(30000); - } - // Actually return the response. - if (status == ResponseStatus::NotModified) { - notify(std::move(response), FileCache::Hint::Refresh); - } else { - notify(std::move(response), FileCache::Hint::Full); - } + notify(std::move(response)); } delete this; } void HTTPNSURLRequest::cancel() { - context->removeRequest(this); cancelled = true; - // Stop the backoff timer to avoid re-triggering this request. - timer.stop(); - if (task) { [task cancel]; [task release]; @@ -198,48 +163,47 @@ void HTTPNSURLRequest::cancel() { } void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *error) { + response = std::make_unique<Response>(); + using Error = Response::Error; + if (error) { if ([error code] == NSURLErrorCancelled) { - status = ResponseStatus::Canceled; + response->error = + std::make_unique<Error>(Error::Reason::Canceled, "Request was cancelled"); } else { - // TODO: Use different codes for host not found, timeout, invalid URL etc. - // These can be categorized in temporary and permanent errors. - response = std::make_unique<Response>(); if (data) { - response->data = std::make_shared<std::string>((const char *)[data bytes], [data length]); + response->data = + std::make_shared<std::string>((const char*)[data bytes], [data length]); } - response->status = Response::Error; - response->message = [[error localizedDescription] UTF8String]; switch ([error code]) { - case NSURLErrorBadServerResponse: // 5xx errors - status = ResponseStatus::TemporaryError; - break; - - case NSURLErrorTimedOut: - case NSURLErrorUserCancelledAuthentication: - status = ResponseStatus::SingularError; // retry immediately - break; - - case NSURLErrorNetworkConnectionLost: - case NSURLErrorCannotFindHost: - case NSURLErrorCannotConnectToHost: - case NSURLErrorDNSLookupFailed: - case NSURLErrorNotConnectedToInternet: - case NSURLErrorInternationalRoamingOff: - case NSURLErrorCallIsActive: - case NSURLErrorDataNotAllowed: - status = ResponseStatus::ConnectionError; - break; - - default: - status = ResponseStatus::PermanentError; + case NSURLErrorBadServerResponse: // 5xx errors + response->error = std::make_unique<Error>( + Error::Reason::Server, [[error localizedDescription] UTF8String]); + break; + + case NSURLErrorNetworkConnectionLost: + case NSURLErrorCannotFindHost: + case NSURLErrorCannotConnectToHost: + case NSURLErrorDNSLookupFailed: + case NSURLErrorNotConnectedToInternet: + case NSURLErrorInternationalRoamingOff: + case NSURLErrorCallIsActive: + case NSURLErrorDataNotAllowed: + case NSURLErrorTimedOut: + response->error = std::make_unique<Error>( + Error::Reason::Connection, [[error localizedDescription] UTF8String]); + break; + + default: + response->error = std::make_unique<Error>( + Error::Reason::Other, [[error localizedDescription] UTF8String]); + break; } } } else if ([res isKindOfClass:[NSHTTPURLResponse class]]) { const long responseCode = [(NSHTTPURLResponse *)res statusCode]; - response = std::make_unique<Response>(); response->data = std::make_shared<std::string>((const char *)[data bytes], [data length]); NSDictionary *headers = [(NSHTTPURLResponse *)res allHeaderFields]; @@ -263,68 +227,45 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e response->etag = [etag UTF8String]; } - if (responseCode == 304) { + if (responseCode == 200) { + // Nothing to do; this is what we want. + } else if (responseCode == 304) { if (existingResponse) { // We're going to copy over the existing response's data. - response->status = existingResponse->status; - response->message = existingResponse->message; + if (existingResponse->error) { + response->error = std::make_unique<Error>(*existingResponse->error); + } + response->data = existingResponse->data; response->modified = existingResponse->modified; + // We're not updating `expired`, it was probably set during the request. response->etag = existingResponse->etag; - response->data = existingResponse->data; - status = ResponseStatus::NotModified; } else { // This is an unsolicited 304 response and should only happen on malfunctioning // HTTP servers. It likely doesn't include any data, but we don't have much options. - response->status = Response::Successful; - status = ResponseStatus::Successful; } - } else if (responseCode == 200) { - response->status = Response::Successful; - status = ResponseStatus::Successful; } else if (responseCode == 404) { - response->status = Response::NotFound; - status = ResponseStatus::Successful; + response->error = + std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404"); } else if (responseCode >= 500 && responseCode < 600) { - // Server errors may be temporary, so back off exponentially. - response->status = Response::Error; - response->message = "HTTP status code " + std::to_string(responseCode); - status = ResponseStatus::TemporaryError; + response->error = + std::make_unique<Error>(Error::Reason::Server, std::string{ "HTTP status code " } + + std::to_string(responseCode)); } else { - // We don't know how to handle any other errors, so declare them as permanently failing. - response->status = Response::Error; - response->message = "HTTP status code " + std::to_string(responseCode); - status = ResponseStatus::PermanentError; + response->error = + std::make_unique<Error>(Error::Reason::Other, std::string{ "HTTP status code " } + + std::to_string(responseCode)); } } else { // This should never happen. - status = ResponseStatus::PermanentError; - response = std::make_unique<Response>(); - response->status = Response::Error; - response->message = "response class is not NSHTTPURLResponse"; + response->error = std::make_unique<Error>(Error::Reason::Other, + "Response class is not NSHTTPURLResponse"); } async.send(); } -void HTTPNSURLRequest::retry(uint64_t timeout) { - response.reset(); - - timer.stop(); - timer.start(timeout, 0, [this] { start(); }); -} - -void HTTPNSURLRequest::retry() { - // All batons get notified when the network status changed, but some of them - // might not actually wait for the network to become available again. - if (strategy == PreemptImmediately && !task) { - // Triggers the timer upon the next event loop iteration. - timer.stop(); - timer.start(0, 0, [this] { start(); }); - } -} - -std::unique_ptr<HTTPContextBase> HTTPContextBase::createContext(uv_loop_t* loop) { - return std::make_unique<HTTPNSURLContext>(loop); +std::unique_ptr<HTTPContextBase> HTTPContextBase::createContext(uv_loop_t*) { + return std::make_unique<HTTPNSURLContext>(); } } diff --git a/platform/default/asset_request_fs.cpp b/platform/default/asset_request_fs.cpp index ac54cda0a1..c169abf2b8 100644 --- a/platform/default/asset_request_fs.cpp +++ b/platform/default/asset_request_fs.cpp @@ -115,13 +115,14 @@ void AssetRequest::fileStated(uv_fs_t *req) { // File is too large for us to open this way because uv_buf's only support unsigned // ints as maximum size. auto response = std::make_unique<Response>(); - response->status = Response::Error; #if UV_VERSION_MAJOR == 0 && UV_VERSION_MINOR <= 10 - response->message = uv_strerror(uv_err_t {UV_EFBIG, 0}); + auto message = uv_strerror(uv_err_t {UV_EFBIG, 0}); #else - response->message = uv_strerror(UV_EFBIG); + auto message = uv_strerror(UV_EFBIG); #endif - self->notify(std::move(response), FileCache::Hint::No); + response->error = + std::make_unique<Response::Error>(Response::Error::Reason::Other, message); + self->notify(std::move(response)); uv_fs_req_cleanup(req); uv_fs_close(req->loop, req, self->fd, fileClosed); @@ -163,8 +164,7 @@ void AssetRequest::fileRead(uv_fs_t *req) { notifyError(req); } else { // File was successfully read. - self->response->status = Response::Successful; - self->notify(std::move(self->response), FileCache::Hint::No); + self->notify(std::move(self->response)); } uv_fs_req_cleanup(req); @@ -191,9 +191,13 @@ void AssetRequest::notifyError(uv_fs_t *req) { if (req->result < 0 && !self->canceled && req->result != UV_ECANCELED) { auto response = std::make_unique<Response>(); - response->status = Response::Error; - response->message = uv::getFileRequestError(req); - self->notify(std::move(response), FileCache::Hint::No); + Response::Error::Reason reason = Response::Error::Reason::Other; + if (req->result == UV_ENOENT || req->result == UV_EISDIR) { + reason = Response::Error::Reason::NotFound; + } + response->error = std::make_unique<Response::Error>(reason, + uv::getFileRequestError(req)); + self->notify(std::move(response)); } } diff --git a/platform/default/asset_request_zip.cpp b/platform/default/asset_request_zip.cpp index 2947bb3c6a..2c0c54b6a0 100644 --- a/platform/default/asset_request_zip.cpp +++ b/platform/default/asset_request_zip.cpp @@ -90,7 +90,7 @@ private: void fileClosed(uv_zip_t *zip); void cleanup(uv_zip_t *zip); - void notifyError(const char *message); + void notifyError(const char *message, Response::Error::Reason); }; RequestBase* AssetZipContext::createRequest(const Resource& resource, @@ -126,7 +126,7 @@ void AssetRequest::openZipArchive() { uv_fs_open(context.loop, req, root.c_str(), O_RDONLY, S_IRUSR, [](uv_fs_t *fsReq) { if (fsReq->result < 0) { auto impl = reinterpret_cast<AssetRequest *>(fsReq->data); - impl->notifyError(uv::getFileRequestError(fsReq)); + impl->notifyError(uv::getFileRequestError(fsReq), Response::Error::Reason::Connection); delete impl; } else { uv_zip_t *zip = new uv_zip_t(); @@ -135,7 +135,7 @@ void AssetRequest::openZipArchive() { uv_zip_fdopen(fsReq->loop, zip, uv_file(fsReq->result), 0, [](uv_zip_t *openZip) { auto impl = reinterpret_cast<AssetRequest *>(openZip->data); if (openZip->result < 0) { - impl->notifyError(openZip->message); + impl->notifyError(openZip->message, Response::Error::Reason::Other); delete openZip; delete impl; } else { @@ -169,12 +169,12 @@ void AssetRequest::fileStated(uv_zip_t *zip) { if (cancelled || zip->result < 0) { // Stat failed, probably because the file doesn't exist. if (zip->result < 0) { - notifyError(zip->message); + notifyError(zip->message, Response::Error::Reason::NotFound); } cleanup(zip); } else if (!(zip->stat->valid & ZIP_STAT_SIZE)) { // We couldn't obtain the size of the file. - notifyError("Could not determine file size in zip file"); + notifyError("Could not determine file size in zip file", Response::Error::Reason::Other); cleanup(zip); } else { response = std::make_unique<Response>(); @@ -203,7 +203,7 @@ void AssetRequest::fileOpened(uv_zip_t *zip) { if (zip->result < 0) { // Opening failed. - notifyError(zip->message); + notifyError(zip->message, Response::Error::Reason::Other); cleanup(zip); } else if (cancelled) { // The request was canceled. Close the file again. @@ -218,10 +218,9 @@ void AssetRequest::fileRead(uv_zip_t *zip) { if (zip->result < 0) { // Reading failed. We still have an open file handle though. - notifyError(zip->message); + notifyError(zip->message, Response::Error::Reason::Other); } else if (!cancelled) { - response->status = Response::Successful; - notify(std::move(response), FileCache::Hint::No); + notify(std::move(response)); } uv_zip_fclose(context.loop, zip, zip->file, INVOKE_MEMBER(fileClosed)); @@ -244,14 +243,13 @@ void AssetRequest::cleanup(uv_zip_t *zip) { delete this; } -void AssetRequest::notifyError(const char *message) { +void AssetRequest::notifyError(const char *message, Response::Error::Reason reason) { MBGL_VERIFY_THREAD(tid); if (!cancelled) { response = std::make_unique<Response>(); - response->status = Response::Error; - response->message = message; - notify(std::move(response), FileCache::Hint::No); + response->error = std::make_unique<Response::Error>(reason, message); + notify(std::move(response)); } else { // The request was already canceled and deleted. } diff --git a/platform/default/http_request_curl.cpp b/platform/default/http_request_curl.cpp index efc4efadf7..6d4011dd2b 100644 --- a/platform/default/http_request_curl.cpp +++ b/platform/default/http_request_curl.cpp @@ -94,7 +94,6 @@ public: ~HTTPCURLRequest(); void cancel() final; - void retry() final; void handleResult(CURLcode code); @@ -102,11 +101,6 @@ private: static size_t headerCallback(char *const buffer, const size_t size, const size_t nmemb, void *userp); static size_t writeCallback(void *const contents, const size_t size, const size_t nmemb, void *userp); - void retry(uint64_t timeout) final; - static void restart(UV_TIMER_PARAMS(timer)); - void finish(ResponseStatus status); - void start(); - HTTPCURLContext *context = nullptr; // Will store the current response. @@ -119,12 +113,6 @@ private: CURL *handle = nullptr; curl_slist *headers = nullptr; - uv_timer_t *timer = nullptr; - enum : bool { PreemptImmediately, ExponentialBackoff } strategy = PreemptImmediately; - int attempts = 0; - - static const int maxAttempts = 4; - char error[CURL_ERROR_SIZE]; }; @@ -167,7 +155,7 @@ private: // ------------------------------------------------------------------------------------------------- HTTPCURLContext::HTTPCURLContext(uv_loop_t *loop_) - : HTTPContextBase(loop_), + : HTTPContextBase(), loop(loop_) { if (curl_global_init(CURL_GLOBAL_ALL)) { throw std::runtime_error("Could not init cURL"); @@ -428,8 +416,6 @@ HTTPCURLRequest::HTTPCURLRequest(HTTPCURLContext* context_, const Resource& reso context(context_), existingResponse(response_), handle(context->getHandle()) { - context->addRequest(this); - // Zero out the error buffer. memset(error, 0, sizeof(error)); @@ -472,25 +458,17 @@ HTTPCURLRequest::HTTPCURLRequest(HTTPCURLContext* context_, const Resource& reso handleError(curl_easy_setopt(handle, CURLOPT_USERAGENT, "MapboxGL/1.0")); handleError(curl_easy_setopt(handle, CURLOPT_SHARE, context->share)); - start(); + // Start requesting the information. + handleError(curl_multi_add_handle(context->multi, handle)); } HTTPCURLRequest::~HTTPCURLRequest() { MBGL_VERIFY_THREAD(tid); - context->removeRequest(this); - handleError(curl_multi_remove_handle(context->multi, handle)); context->returnHandle(handle); handle = nullptr; - if (timer) { - // Stop the backoff timer to avoid re-triggering this request. - uv_timer_stop(timer); - uv::close(timer); - timer = nullptr; - } - if (headers) { curl_slist_free_all(headers); headers = nullptr; @@ -501,14 +479,6 @@ void HTTPCURLRequest::cancel() { delete this; } -void HTTPCURLRequest::start() { - // Count up the attempts. - attempts++; - - // Start requesting the information. - handleError(curl_multi_add_handle(context->multi, handle)); -} - // This function is called when we have new data for a request. We just append it to the string // containing the previous data. size_t HTTPCURLRequest::writeCallback(void *const contents, const size_t size, const size_t nmemb, void *userp) { @@ -569,60 +539,6 @@ size_t HTTPCURLRequest::headerCallback(char *const buffer, const size_t size, co return length; } -void HTTPCURLRequest::retry(uint64_t timeout) { - handleError(curl_multi_remove_handle(context->multi, handle)); - - response.reset(); - - assert(!timer); - timer = new uv_timer_t; - timer->data = this; - uv_timer_init(context->loop, timer); - uv_timer_start(timer, restart, timeout, 0); -} - -void HTTPCURLRequest::retry() { - // All batons get notified when the network status changed, but some of them - // might not actually wait for the network to become available again. - if (timer && strategy == PreemptImmediately) { - // Triggers the timer upon the next event loop iteration. - uv_timer_stop(timer); - uv_timer_start(timer, restart, 0, 0); - } -} - -void HTTPCURLRequest::restart(UV_TIMER_PARAMS(timer)) { - // Restart the request. - auto baton = reinterpret_cast<HTTPCURLRequest *>(timer->data); - - // Get rid of the timer. - baton->timer = nullptr; - uv::close(timer); - - baton->start(); -} - -void HTTPCURLRequest::finish(ResponseStatus status) { - if (status == ResponseStatus::TemporaryError && attempts < maxAttempts) { - strategy = ExponentialBackoff; - return retry((1 << (attempts - 1)) * 1000); - } else if (status == ResponseStatus::ConnectionError && attempts < maxAttempts) { - // By default, we will retry every 30 seconds (network change notification will - // preempt the timeout). - strategy = PreemptImmediately; - return retry(30000); - } - - // Actually return the response. - if (status == ResponseStatus::NotModified) { - notify(std::move(response), FileCache::Hint::Refresh); - } else { - notify(std::move(response), FileCache::Hint::Full); - } - - delete this; -} - void HTTPCURLRequest::handleResult(CURLcode code) { MBGL_VERIFY_THREAD(tid); @@ -633,77 +549,79 @@ void HTTPCURLRequest::handleResult(CURLcode code) { return; } - // Make sure a response object exists in case we haven't got any headers - // or content. + // Make sure a response object exists in case we haven't got any headers or content. if (!response) { response = std::make_unique<Response>(); } + using Error = Response::Error; + // Add human-readable error code if (code != CURLE_OK) { - response->status = Response::Error; - response->message = std::string { curl_easy_strerror(code) } + ": " + error; - switch (code) { case CURLE_COULDNT_RESOLVE_PROXY: case CURLE_COULDNT_RESOLVE_HOST: case CURLE_COULDNT_CONNECT: - return finish(ResponseStatus::ConnectionError); - case CURLE_OPERATION_TIMEDOUT: - return finish(ResponseStatus::TemporaryError); + + response->error = std::make_unique<Error>( + Error::Reason::Connection, std::string{ curl_easy_strerror(code) } + ": " + error); + break; default: - return finish(ResponseStatus::PermanentError); + response->error = std::make_unique<Error>( + Error::Reason::Other, std::string{ curl_easy_strerror(code) } + ": " + error); + break; } } else { long responseCode = 0; curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &responseCode); - if (responseCode == 304) { + // Move over any data we got. We're storing this in a separate object because the Response + // object defines it as const. + if (data) { + response->data = std::move(data); + } else { + response->data = std::make_shared<std::string>(); + } + + if (responseCode == 200) { + // Nothing to do; this is what we want. + } else if (responseCode == 304) { if (existingResponse) { // We're going to copy over the existing response's data. - response->status = existingResponse->status; - response->message = existingResponse->message; + if (existingResponse->error) { + response->error = std::make_unique<Error>(*existingResponse->error); + } + response->data = existingResponse->data; response->modified = existingResponse->modified; + // We're not updating `expired`, it was probably set during the request. response->etag = existingResponse->etag; - response->data = existingResponse->data; - return finish(ResponseStatus::NotModified); } else { // This is an unsolicited 304 response and should only happen on malfunctioning // HTTP servers. It likely doesn't include any data, but we don't have much options. - response->status = Response::Successful; - response->data = std::move(data); - return finish(ResponseStatus::Successful); } - } else if (responseCode == 200) { - response->status = Response::Successful; - response->data = std::move(data); - return finish(ResponseStatus::Successful); } else if (responseCode == 404) { - response->status = Response::NotFound; - response->data = std::move(data); - return finish(ResponseStatus::Successful); + response->error = + std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404"); } else if (responseCode >= 500 && responseCode < 600) { - // Server errors may be temporary, so back off exponentially. - response->status = Response::Error; - response->data = std::move(data); - response->message = "HTTP status code " + util::toString(responseCode); - return finish(ResponseStatus::TemporaryError); + response->error = + std::make_unique<Error>(Error::Reason::Server, std::string{ "HTTP status code " } + + std::to_string(responseCode)); } else { - // We don't know how to handle any other errors, so declare them as permanently failing. - response->status = Response::Error; - response->data = std::move(data); - response->message = "HTTP status code " + util::toString(responseCode); - return finish(ResponseStatus::PermanentError); + response->error = + std::make_unique<Error>(Error::Reason::Other, std::string{ "HTTP status code " } + + std::to_string(responseCode)); } } - throw std::runtime_error("Response hasn't been handled"); + // Actually return the response. + notify(std::move(response)); + delete this; } std::unique_ptr<HTTPContextBase> HTTPContextBase::createContext(uv_loop_t* loop) { return std::make_unique<HTTPCURLContext>(loop); } -} +} // namespace mbgl diff --git a/platform/default/sqlite_cache.cpp b/platform/default/sqlite_cache.cpp index dfb6290548..d8b79c8f9b 100644 --- a/platform/default/sqlite_cache.cpp +++ b/platform/default/sqlite_cache.cpp @@ -113,7 +113,11 @@ void SQLiteCache::Impl::get(const Resource &resource, Callback callback) { if (getStmt->run()) { // There is data. auto response = std::make_unique<Response>(); - response->status = Response::Status(getStmt->get<int>(0)); + const auto status = getStmt->get<int>(0); + if (status > 1) { + // Status codes > 1 indicate an error + response->error = std::make_unique<Response::Error>(Response::Error::Reason(status)); + } response->modified = getStmt->get<int64_t>(1); response->etag = getStmt->get<std::string>(2); response->expires = getStmt->get<int64_t>(3); @@ -164,7 +168,11 @@ void SQLiteCache::Impl::put(const Resource& resource, std::shared_ptr<const Resp const auto canonicalURL = util::mapbox::canonicalURL(resource.url); putStmt->bind(1 /* url */, canonicalURL.c_str()); - putStmt->bind(2 /* status */, int(response->status)); + if (response->error) { + putStmt->bind(2 /* status */, int(response->error->reason)); + } else { + putStmt->bind(2 /* status */, 1 /* success */); + } putStmt->bind(3 /* kind */, int(resource.kind)); putStmt->bind(4 /* modified */, response->modified); putStmt->bind(5 /* etag */, response->etag.c_str()); 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; diff --git a/test/fixtures/database/cache.db b/test/fixtures/database/cache.db Binary files differindex c358381f1f..04bbdbb34b 100644 --- a/test/fixtures/database/cache.db +++ b/test/fixtures/database/cache.db diff --git a/test/fixtures/database/invalid.db b/test/fixtures/database/invalid.db Binary files differindex f07e4af03d..b068b15465 100644 --- a/test/fixtures/database/invalid.db +++ b/test/fixtures/database/invalid.db diff --git a/test/fixtures/mock_file_source.cpp b/test/fixtures/mock_file_source.cpp index 407f54408f..5049b5c001 100644 --- a/test/fixtures/mock_file_source.cpp +++ b/test/fixtures/mock_file_source.cpp @@ -52,13 +52,11 @@ private: void MockFileSource::Impl::replyWithSuccess(Request* req) const { std::shared_ptr<Response> res = std::make_shared<Response>(); - res->status = Response::Status::Successful; try { res->data = std::make_shared<const std::string>(std::move(util::read_file(req->resource.url))); } catch (const std::exception& err) { - res->status = Response::Status::Error; - res->message = err.what(); + res->error = std::make_unique<Response::Error>(Response::Error::Reason::Other, err.what()); } req->notify(res); @@ -81,8 +79,7 @@ void MockFileSource::Impl::replyWithFailure(Request* req) const { } std::shared_ptr<Response> res = std::make_shared<Response>(); - res->status = Response::Status::Error; - res->message = "Failed by the test case"; + res->error = std::make_unique<Response::Error>(Response::Error::Reason::Other, "Failed by the test case"); req->notify(res); } @@ -94,7 +91,6 @@ void MockFileSource::Impl::replyWithCorruptedData(Request* req) const { } std::shared_ptr<Response> res = std::make_shared<Response>(); - res->status = Response::Status::Successful; auto data = std::make_shared<std::string>(std::move(util::read_file(req->resource.url))); data->insert(0, "CORRUPTED"); res->data = std::move(data); diff --git a/test/storage/cache_response.cpp b/test/storage/cache_response.cpp index 5b0923900e..f70557e6e0 100644 --- a/test/storage/cache_response.cpp +++ b/test/storage/cache_response.cpp @@ -18,14 +18,13 @@ TEST_F(Storage, CacheResponse) { Request* req = fs.request(resource, uv_default_loop(), [&](const Response &res) { fs.cancel(req); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response 1", *res.data); EXPECT_LT(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); response = res; }); @@ -35,14 +34,13 @@ TEST_F(Storage, CacheResponse) { // again, we'd get different values. req = fs.request(resource, uv_default_loop(), [&](const Response &res) { fs.cancel(req); - EXPECT_EQ(response.status, res.status); + EXPECT_EQ(response.error, res.error); EXPECT_EQ(response.stale, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ(*response.data, *res.data); EXPECT_EQ(response.expires, res.expires); EXPECT_EQ(response.modified, res.modified); EXPECT_EQ(response.etag, res.etag); - EXPECT_EQ(response.message, res.message); CacheResponse.finish(); }); diff --git a/test/storage/cache_revalidate.cpp b/test/storage/cache_revalidate.cpp index d7dcbe71cd..6d9d30a3ec 100644 --- a/test/storage/cache_revalidate.cpp +++ b/test/storage/cache_revalidate.cpp @@ -25,14 +25,13 @@ TEST_F(Storage, CacheRevalidateSame) { } first = false; - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("snowfall", res.etag); - EXPECT_EQ("", res.message); req2 = fs.request(revalidateSame, uv_default_loop(), [&, res](const Response &res2) { if (res2.stale) { @@ -48,8 +47,8 @@ TEST_F(Storage, CacheRevalidateSame) { fs.cancel(req2); req2 = nullptr; - EXPECT_EQ(Response::Successful, res2.status); - EXPECT_EQ(false, res.stale); + EXPECT_EQ(nullptr, res2.error); + EXPECT_EQ(false, res2.stale); ASSERT_TRUE(res2.data.get()); EXPECT_EQ(res.data, res2.data); EXPECT_EQ("Response", *res2.data); @@ -58,7 +57,6 @@ TEST_F(Storage, CacheRevalidateSame) { EXPECT_EQ(0, res2.modified); // We're not sending the ETag in the 304 reply, but it should still be there. EXPECT_EQ("snowfall", res2.etag); - EXPECT_EQ("", res2.message); CacheRevalidateSame.finish(); }); @@ -88,14 +86,13 @@ TEST_F(Storage, CacheRevalidateModified) { } first = false; - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(1420070400, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); req2 = fs.request(revalidateModified, uv_default_loop(), [&, res](const Response &res2) { if (res2.stale) { @@ -111,7 +108,7 @@ TEST_F(Storage, CacheRevalidateModified) { fs.cancel(req2); req2 = nullptr; - EXPECT_EQ(Response::Successful, res2.status); + EXPECT_EQ(nullptr, res2.error); EXPECT_EQ(false, res2.stale); ASSERT_TRUE(res2.data.get()); EXPECT_EQ("Response", *res2.data); @@ -120,7 +117,6 @@ TEST_F(Storage, CacheRevalidateModified) { EXPECT_LT(0, res2.expires); EXPECT_EQ(1420070400, res2.modified); EXPECT_EQ("", res2.etag); - EXPECT_EQ("", res2.message); CacheRevalidateModified.finish(); }); @@ -149,14 +145,13 @@ TEST_F(Storage, CacheRevalidateEtag) { } first = false; - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response 1", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("response-1", res.etag); - EXPECT_EQ("", res.message); req2 = fs.request(revalidateEtag, uv_default_loop(), [&, res](const Response &res2) { if (res2.stale) { @@ -172,15 +167,14 @@ TEST_F(Storage, CacheRevalidateEtag) { fs.cancel(req2); req2 = nullptr; - EXPECT_EQ(Response::Successful, res2.status); - EXPECT_EQ(false, res.stale); + EXPECT_EQ(nullptr, res2.error); + EXPECT_EQ(false, res2.stale); ASSERT_TRUE(res2.data.get()); EXPECT_NE(res.data, res2.data); EXPECT_EQ("Response 2", *res2.data); EXPECT_EQ(0, res2.expires); EXPECT_EQ(0, res2.modified); EXPECT_EQ("response-2", res2.etag); - EXPECT_EQ("", res2.message); CacheRevalidateEtag.finish(); }); diff --git a/test/storage/directory_reading.cpp b/test/storage/directory_reading.cpp index f101252761..c30f504f3d 100644 --- a/test/storage/directory_reading.cpp +++ b/test/storage/directory_reading.cpp @@ -18,16 +18,17 @@ TEST_F(Storage, AssetReadDirectory) { Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage" }, uv_default_loop(), [&](const Response &res) { fs.cancel(req); - EXPECT_EQ(Response::Error, res.status); + ASSERT_NE(nullptr, res.error); + EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason); EXPECT_EQ(false, res.stale); ASSERT_FALSE(res.data.get()); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); #ifdef MBGL_ASSET_ZIP - EXPECT_EQ("No such file", res.message); + EXPECT_EQ("No such file", res.error->message); #elif MBGL_ASSET_FS - EXPECT_EQ("illegal operation on a directory", res.message); + EXPECT_EQ("illegal operation on a directory", res.error->message); #endif ReadDirectory.finish(); }); diff --git a/test/storage/file_reading.cpp b/test/storage/file_reading.cpp index ab7ba326ad..c1b7f27a98 100644 --- a/test/storage/file_reading.cpp +++ b/test/storage/file_reading.cpp @@ -19,14 +19,13 @@ TEST_F(Storage, AssetEmptyFile) { Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/empty" }, uv_default_loop(), [&](const Response &res) { fs.cancel(req); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("", *res.data); EXPECT_EQ(0, res.expires); EXPECT_LT(1420000000, res.modified); EXPECT_NE("", res.etag); - EXPECT_EQ("", res.message); EmptyFile.finish(); }); @@ -47,14 +46,13 @@ TEST_F(Storage, AssetNonEmptyFile) { Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/nonempty" }, uv_default_loop(), [&](const Response &res) { fs.cancel(req); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("content is here\n", *res.data); EXPECT_EQ(0, res.expires); EXPECT_LT(1420000000, res.modified); EXPECT_NE("", res.etag); - EXPECT_EQ("", res.message); ASSERT_TRUE(res.data.get()); EXPECT_EQ("content is here\n", *res.data); NonEmptyFile.finish(); @@ -77,16 +75,17 @@ TEST_F(Storage, AssetNonExistentFile) { Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/does_not_exist" }, uv_default_loop(), [&](const Response &res) { fs.cancel(req); - EXPECT_EQ(Response::Error, res.status); + ASSERT_NE(nullptr, res.error); + EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason); EXPECT_EQ(false, res.stale); ASSERT_FALSE(res.data.get()); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); #ifdef MBGL_ASSET_ZIP - EXPECT_EQ("No such file", res.message); + EXPECT_EQ("No such file", res.error->message); #elif MBGL_ASSET_FS - EXPECT_EQ("no such file or directory", res.message); + EXPECT_EQ("no such file or directory", res.error->message); #endif NonExistentFile.finish(); }); diff --git a/test/storage/http_cancel.cpp b/test/storage/http_cancel.cpp index 7dfe9c58d5..cbe273e05d 100644 --- a/test/storage/http_cancel.cpp +++ b/test/storage/http_cancel.cpp @@ -38,14 +38,13 @@ TEST_F(Storage, HTTPCancelMultiple) { }); Request* req = fs.request(resource, uv_default_loop(), [&](const Response &res) { fs.cancel(req); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); HTTPCancelMultiple.finish(); }); fs.cancel(req2); diff --git a/test/storage/http_coalescing.cpp b/test/storage/http_coalescing.cpp index 2748fb8757..ab776eebfb 100644 --- a/test/storage/http_coalescing.cpp +++ b/test/storage/http_coalescing.cpp @@ -26,13 +26,12 @@ TEST_F(Storage, HTTPCoalescing) { EXPECT_EQ(&res, reference); } - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); if (counter >= total) { HTTPCoalescing.finish(); @@ -69,13 +68,12 @@ TEST_F(Storage, HTTPMultiple) { reference = &res; // Do not cancel the request right away. - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(2147483647, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); // Start a second request for the same resource after the first one has been completed. req2 = fs.request(resource, uv_default_loop(), [&] (const Response &res2) { @@ -86,13 +84,12 @@ TEST_F(Storage, HTTPMultiple) { fs.cancel(req1); fs.cancel(req2); - EXPECT_EQ(Response::Successful, res2.status); + EXPECT_EQ(nullptr, res2.error); ASSERT_TRUE(res2.data.get()); EXPECT_EQ("Hello World!", *res2.data); EXPECT_EQ(2147483647, res2.expires); EXPECT_EQ(0, res2.modified); EXPECT_EQ("", res2.etag); - EXPECT_EQ("", res2.message); HTTPMultiple.finish(); }); @@ -117,14 +114,13 @@ TEST_F(Storage, HTTPStale) { Request* req2 = nullptr; req1 = fs.request(resource, uv_default_loop(), [&] (const Response &res) { // Do not cancel the request right away. - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(false, res.stale); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); // Don't start the request twice in case this callback gets fired multiple times. if (req2) { @@ -135,13 +131,12 @@ TEST_F(Storage, HTTPStale) { // Start a second request for the same resource after the first one has been completed. req2 = fs.request(resource, uv_default_loop(), [&] (const Response &res2) { - EXPECT_EQ(Response::Successful, res2.status); + EXPECT_EQ(nullptr, res2.error); ASSERT_TRUE(res2.data.get()); EXPECT_EQ("Hello World!", *res2.data); EXPECT_EQ(0, res2.expires); EXPECT_EQ(0, res2.modified); EXPECT_EQ("", res2.etag); - EXPECT_EQ("", res2.message); if (res2.stale) { EXPECT_EQ(0, stale); diff --git a/test/storage/http_error.cpp b/test/storage/http_error.cpp index 85bc70e546..00deeb063a 100644 --- a/test/storage/http_error.cpp +++ b/test/storage/http_error.cpp @@ -19,67 +19,80 @@ TEST_F(Storage, HTTPError) { using namespace mbgl; - uv_timer_t statusChange; - uv_timer_init(uv_default_loop(), &statusChange); - uv_timer_start(&statusChange, [](UV_TIMER_PARAMS) { - NetworkStatus::Reachable(); - }, 500, 500); - uv_unref((uv_handle_t *)&statusChange); - DefaultFileSource fs(nullptr); - auto start = uv_hrtime(); + const auto start = uv_hrtime(); Request* req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/temporary-error" }, uv_default_loop(), [&](const Response &res) { - fs.cancel(req1); - const auto duration = double(uv_hrtime() - start) / 1e9; - EXPECT_LT(1, duration) << "Backoff timer didn't wait 1 second"; - EXPECT_GT(1.2, duration) << "Backoff timer fired too late"; - EXPECT_EQ(Response::Successful, res.status); - EXPECT_EQ(false, res.stale); - ASSERT_TRUE(res.data.get()); - EXPECT_EQ("Hello World!", *res.data); - EXPECT_EQ(0, res.expires); - EXPECT_EQ(0, res.modified); - EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); - - HTTPTemporaryError.finish(); + static int counter = 0; + switch (counter++) { + case 0: { + const auto duration = double(uv_hrtime() - start) / 1e9; + EXPECT_GT(0.2, duration) << "Initial error request took too long"; + ASSERT_NE(nullptr, res.error); + EXPECT_EQ(Response::Error::Reason::Server, res.error->reason); + EXPECT_EQ("HTTP status code 500", res.error->message); + EXPECT_EQ(false, res.stale); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("", *res.data); + EXPECT_EQ(0, res.expires); + EXPECT_EQ(0, res.modified); + EXPECT_EQ("", res.etag); + } break; + case 1: { + fs.cancel(req1); + const auto duration = double(uv_hrtime() - start) / 1e9; + EXPECT_LT(0.99, duration) << "Backoff timer didn't wait 1 second"; + EXPECT_GT(1.2, duration) << "Backoff timer fired too late"; + EXPECT_EQ(nullptr, res.error); + EXPECT_EQ(false, res.stale); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); + EXPECT_EQ(0, res.expires); + EXPECT_EQ(0, res.modified); + EXPECT_EQ("", res.etag); + HTTPTemporaryError.finish(); + } break; + } }); Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3001/" }, uv_default_loop(), [&](const Response &res) { - fs.cancel(req2); + static int counter = 0; + static int wait = 0; const auto duration = double(uv_hrtime() - start) / 1e9; - // 1.5 seconds == 4 retries, with a 500ms timeout (see above). - EXPECT_LT(1.4, duration) << "Resource wasn't retried the correct number of times"; - EXPECT_GT(1.7, duration) << "Resource wasn't retried the correct number of times"; - EXPECT_EQ(Response::Error, res.status); - EXPECT_EQ(false, res.stale); + EXPECT_LT(wait - 0.01, duration) << "Backoff timer didn't wait 1 second"; + EXPECT_GT(wait + 0.2, duration) << "Backoff timer fired too late"; + ASSERT_NE(nullptr, res.error); + EXPECT_EQ(Response::Error::Reason::Connection, res.error->reason); #ifdef MBGL_HTTP_NSURL - EXPECT_TRUE(res.message == - "The operation couldn’t be completed. (NSURLErrorDomain error -1004.)" || - res.message == "Could not connect to the server.") - << "Full message is: \"" << res.message << "\""; + 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.message.substr(0, prefix.size()).c_str()) << "Full message is: \"" << res.message << "\""; + 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); - HTTPConnectionError.finish(); + + if (counter == 2) { + fs.cancel(req2); + HTTPConnectionError.finish(); + } + wait += (1 << counter); + counter++; }); uv_run(uv_default_loop(), UV_RUN_DEFAULT); - uv_timer_stop(&statusChange); - uv_close(reinterpret_cast<uv_handle_t *>(&statusChange), nullptr); - // 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_header_parsing.cpp b/test/storage/http_header_parsing.cpp index 17c3ce16cf..aab8711ea0 100644 --- a/test/storage/http_header_parsing.cpp +++ b/test/storage/http_header_parsing.cpp @@ -19,14 +19,13 @@ TEST_F(Storage, HTTPHeaderParsing) { "http://127.0.0.1:3000/test?modified=1420794326&expires=1420797926&etag=foo" }, uv_default_loop(), [&](const Response &res) { fs.cancel(req1); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(1420797926, res.expires); EXPECT_EQ(1420794326, res.modified); EXPECT_EQ("foo", res.etag); - EXPECT_EQ("", res.message); HTTPExpiresTest.finish(); }); @@ -36,14 +35,13 @@ TEST_F(Storage, HTTPHeaderParsing) { Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test?cachecontrol=max-age=120" }, uv_default_loop(), [&](const Response &res) { fs.cancel(req2); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_GT(2, std::abs(res.expires - now - 120)) << "Expiration date isn't about 120 seconds in the future"; EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); HTTPCacheControlTest.finish(); }); diff --git a/test/storage/http_issue_1369.cpp b/test/storage/http_issue_1369.cpp index c5135b3ab9..c6f0929feb 100644 --- a/test/storage/http_issue_1369.cpp +++ b/test/storage/http_issue_1369.cpp @@ -32,14 +32,13 @@ TEST_F(Storage, HTTPIssue1369) { fs.cancel(req); req = fs.request(resource, uv_default_loop(), [&](const Response &res) { fs.cancel(req); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); HTTPIssue1369.finish(); }); diff --git a/test/storage/http_load.cpp b/test/storage/http_load.cpp index 987e463507..ece859c6d3 100644 --- a/test/storage/http_load.cpp +++ b/test/storage/http_load.cpp @@ -24,14 +24,13 @@ TEST_F(Storage, HTTPLoad) { uv_default_loop(), [&, i, current](const Response &res) { fs.cancel(reqs[i]); reqs[i] = nullptr; - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ(std::string("Request ") + std::to_string(current), *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); if (number <= max) { req(i); diff --git a/test/storage/http_other_loop.cpp b/test/storage/http_other_loop.cpp index f6e805285f..b00d54eb7b 100644 --- a/test/storage/http_other_loop.cpp +++ b/test/storage/http_other_loop.cpp @@ -15,14 +15,13 @@ TEST_F(Storage, HTTPOtherLoop) { Request* req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, uv_default_loop(), [&](const Response &res) { fs.cancel(req); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); HTTPOtherLoop.finish(); }); diff --git a/test/storage/http_reading.cpp b/test/storage/http_reading.cpp index 468ba33248..1c0df9df73 100644 --- a/test/storage/http_reading.cpp +++ b/test/storage/http_reading.cpp @@ -22,14 +22,13 @@ TEST_F(Storage, HTTPReading) { [&](const Response &res) { fs.cancel(req1); EXPECT_EQ(uv_thread_self(), mainThread); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); HTTPTest.finish(); }); @@ -37,11 +36,12 @@ TEST_F(Storage, HTTPReading) { [&](const Response &res) { fs.cancel(req2); EXPECT_EQ(uv_thread_self(), mainThread); - EXPECT_EQ(Response::NotFound, res.status); + ASSERT_NE(nullptr, res.error); + EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Cannot GET /doesnotexist\n", *res.data); - EXPECT_EQ("", res.message); + EXPECT_EQ("HTTP status code 404", res.error->message); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); @@ -52,11 +52,12 @@ TEST_F(Storage, HTTPReading) { [&](const Response &res) { fs.cancel(req3); EXPECT_EQ(uv_thread_self(), mainThread); - EXPECT_EQ(Response::Error, res.status); + ASSERT_NE(nullptr, res.error); + EXPECT_EQ(Response::Error::Reason::Server, res.error->reason); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Server Error!", *res.data); - EXPECT_EQ("HTTP status code 500", res.message); + EXPECT_EQ("HTTP status code 500", res.error->message); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); diff --git a/test/storage/http_retry_network_status.cpp b/test/storage/http_retry_network_status.cpp index 14cdae02c7..d0209e0bf6 100644 --- a/test/storage/http_retry_network_status.cpp +++ b/test/storage/http_retry_network_status.cpp @@ -24,14 +24,13 @@ TEST_F(Storage, HTTPNetworkStatusChange) { // This request takes 200 milliseconds to answer. Request* req = fs.request(resource, uv_default_loop(), [&](const Response& res) { fs.cancel(req); - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response", *res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); HTTPNetworkStatusChange.finish(); }); diff --git a/test/storage/http_timeout.cpp b/test/storage/http_timeout.cpp index b5cd877e76..92a6671d7e 100644 --- a/test/storage/http_timeout.cpp +++ b/test/storage/http_timeout.cpp @@ -18,14 +18,13 @@ TEST_F(Storage, HTTPTimeout) { const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test?cachecontrol=max-age=1" }; Request* req = fs.request(resource, uv_default_loop(), [&](const Response &res) { counter++; - EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(nullptr, res.error); EXPECT_EQ(false, res.stale); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_LT(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); - EXPECT_EQ("", res.message); if (counter == 4) { fs.cancel(req); HTTPTimeout.finish(); diff --git a/test/storage/server.js b/test/storage/server.js index 249c6a124b..cd8c5e3958 100755 --- a/test/storage/server.js +++ b/test/storage/server.js @@ -9,6 +9,19 @@ var app = express(); // We're manually setting Etag headers. app.disable('etag'); +// Terminate after a certain time of inactivity. +function terminate() { + console.warn('Server terminated due to inactivity'); + process.exit(0); +}; +var inactivity = 5000; // milliseconds +var timeout = setTimeout(terminate, inactivity); +app.use(function(req, res, next) { + clearTimeout(timeout); + timeout = setTimeout(terminate, inactivity); + next(); +}); + app.get('/test', function (req, res) { if (req.query.modified) { res.setHeader('Last-Modified', (new Date(req.query.modified * 1000)).toUTCString()); diff --git a/test/storage/storage.hpp b/test/storage/storage.hpp index d2402166a0..4c14746c6a 100644 --- a/test/storage/storage.hpp +++ b/test/storage/storage.hpp @@ -2,7 +2,9 @@ #define MBGL_TEST_STORAGE_STORAGE #include "../fixtures/util.hpp" +#include <mbgl/storage/response.hpp> #include <uv.h> +#include <iostream> class Storage : public testing::Test { public: @@ -13,4 +15,21 @@ protected: static pid_t pid; }; +namespace mbgl { + +inline std::ostream& operator<<(std::ostream& os, Response::Error::Reason r) { + // Special case + if (uint8_t(r) == 1) return os << "Response::Error::Reason::Success"; + switch (r) { + case Response::Error::Reason::NotFound: return os << "Response::Error::Reason::NotFound"; + case Response::Error::Reason::Server: return os << "Response::Error::Reason::Server"; + case Response::Error::Reason::Connection: return os << "Response::Error::Reason::Connection"; + case Response::Error::Reason::Canceled: return os << "Response::Error::Reason::Canceled"; + case Response::Error::Reason::Other: return os << "Response::Error::Reason::Other"; + default: return os << "<Unknown>"; + } +} + +} // namespace mbgl + #endif |