HTTPDownloader: Use string view and move semantics for callback error

wip3-rebase
Stenzek 2 days ago
parent 46532474a2
commit 1067ebe9e6
No known key found for this signature in database

@ -833,16 +833,16 @@ uint32_t Achievements::ClientReadMemory(uint32_t address, uint8_t* buffer, uint3
void Achievements::ClientServerCall(const rc_api_request_t* request, rc_client_server_callback_t callback,
void* callback_data, rc_client_t* client)
{
HTTPDownloader::RequestCallback hd_callback = [callback, callback_data](s32 status_code, Error& error,
std::string& content_type,
HTTPDownloader::RequestData& data) {
if (status_code != HTTPDownloader::HTTP_STATUS_OK)
ERROR_LOG("Server call failed: {}", error.GetDescription());
HTTPDownloader::RequestCallback hd_callback =
[callback, callback_data](s32 status_code, std::string_view error_message, std::string_view content_type,
HTTPDownloader::RequestData data) {
if (status_code != HTTPDownloader::HTTP_STATUS_OK)
ERROR_LOG("Server call failed: {}", error_message);
const rc_api_server_response_t rr = MakeRCAPIServerResponse(status_code, data);
const auto lock = GetLock();
callback(&rr, callback_data);
};
const rc_api_server_response_t rr = MakeRCAPIServerResponse(status_code, data);
const auto lock = GetLock();
callback(&rr, callback_data);
};
const std::array<const char* const, 1> headers = {s_state.http_user_agent_header.c_str()};
if (request->post_data)

@ -1841,11 +1841,12 @@ bool GameList::DownloadCovers(const std::vector<std::string>& url_templates, boo
std::string filename = Path::URLDecode(url);
HTTPDownloader::CreateRequest(
std::move(url), &s_state,
[use_serial, &save_callback, entry_path = std::move(entry_path), filename = std::move(filename)](
s32 status_code, Error& error, std::string& content_type, HTTPDownloader::RequestData& data) {
[use_serial, &save_callback, entry_path = std::move(entry_path),
filename = std::move(filename)](s32 status_code, std::string_view error_message, std::string_view content_type,
HTTPDownloader::RequestData data) {
if (status_code != HTTPDownloader::HTTP_STATUS_OK || data.empty())
{
ERROR_LOG("Download for {} failed: {}", Path::GetFileName(filename), error.GetDescription());
ERROR_LOG("Download for {} failed: {}", Path::GetFileName(filename), error_message);
return;
}
@ -1856,8 +1857,8 @@ bool GameList::DownloadCovers(const std::vector<std::string>& url_templates, boo
// prefer the content type from the response for the extension
// otherwise, if it's missing, and the request didn't have an extension.. fall back to jpegs.
const std::string_view content_type_extension(HTTPDownloader::GetExtensionForContentType(content_type));
std::string template_filename;
std::string content_type_extension(HTTPDownloader::GetExtensionForContentType(content_type));
// don't treat the domain name as an extension..
const std::string::size_type last_slash = filename.find('/');

@ -27,8 +27,8 @@ void AsyncHTTPRequest::get(std::string url, const void* owner, ProgressCallback*
{
HTTPDownloader::CreateRequest(
std::move(url), owner,
[this](s32 status_code, Error& error, std::string& content_type, HTTPDownloader::RequestData& data) {
handleResponse(status_code, error, content_type, data);
[this](s32 status_code, std::string_view error, std::string_view content_type, HTTPDownloader::RequestData data) {
handleResponse(status_code, error, content_type, std::move(data));
},
progress, additional_headers, timeout_seconds);
}
@ -40,24 +40,25 @@ void AsyncHTTPRequest::post(std::string url, std::string post_data, const void*
{
HTTPDownloader::CreatePostRequest(
std::move(url), std::move(post_data), owner,
[this](s32 status_code, Error& error, std::string& content_type, HTTPDownloader::RequestData& data) {
handleResponse(status_code, error, content_type, data);
[this](s32 status_code, std::string_view error, std::string_view content_type, HTTPDownloader::RequestData data) {
handleResponse(status_code, error, content_type, std::move(data));
},
progress, additional_headers, timeout_seconds);
}
ALWAYS_INLINE_RELEASE void AsyncHTTPRequest::handleResponse(s32 status_code, Error& error, std::string& content_type,
HTTPDownloader::RequestData& data)
ALWAYS_INLINE_RELEASE void AsyncHTTPRequest::handleResponse(s32 status_code, const std::string_view& error_message,
const std::string_view& content_type,
HTTPDownloader::RequestData&& data)
{
m_status_code = status_code;
m_error = std::move(error);
m_content_type = std::move(content_type);
m_error_message = error_message;
m_content_type = content_type;
m_data = std::move(data);
QMetaObject::invokeMethod(this, &AsyncHTTPRequest::finishRequest, Qt::QueuedConnection);
}
void AsyncHTTPRequest::finishRequest()
{
emit requestComplete(m_status_code, m_error, m_content_type, m_data);
emit requestComplete(m_status_code, m_error_message, m_content_type, m_data);
deleteLater();
}

@ -28,15 +28,16 @@ public:
HTTPDownloader::HeaderList additional_headers = {}, std::optional<u16> timeout_seconds = {});
Q_SIGNALS:
void requestComplete(qint32 status_code, Error& error_message, std::string& content_type,
HTTPDownloader::RequestData& data);
void requestComplete(qint32 status_code, const std::string& error_message, const std::string& content_type,
const HTTPDownloader::RequestData& data);
private:
void handleResponse(s32 status_code, Error& error, std::string& content_type, HTTPDownloader::RequestData& data);
void handleResponse(s32 status_code, const std::string_view& error_message, const std::string_view& content_type,
HTTPDownloader::RequestData&& data);
void finishRequest();
s32 m_status_code = HTTPDownloader::HTTP_STATUS_ERROR;
Error m_error;
std::string m_error_message;
std::string m_content_type;
HTTPDownloader::RequestData m_data;
};

@ -334,8 +334,9 @@ void AutoUpdaterDialog::queueUpdateCheck(bool display_errors, bool ignore_skippe
AsyncHTTPRequest* const req = new AsyncHTTPRequest();
connect(req, &AsyncHTTPRequest::requestComplete, this,
[this, display_errors](s32 status_code, Error& error, std::string& content_type, std::vector<u8>& response) {
getLatestTagComplete(status_code, error, response, display_errors);
[this, display_errors](s32 status_code, const std::string& error_message, const std::string& content_type,
const std::vector<u8>& response) {
getLatestTagComplete(status_code, error_message, response, display_errors);
});
req->get(LATEST_TAG_URL, this);
}
@ -344,14 +345,13 @@ void AutoUpdaterDialog::queueGetLatestRelease()
{
AsyncHTTPRequest* const req = new AsyncHTTPRequest();
connect(req, &AsyncHTTPRequest::requestComplete, this,
[this](s32 status_code, Error& error, std::string& content_type, std::vector<u8>& response) {
getLatestReleaseComplete(status_code, error, response);
});
[this](s32 status_code, const std::string& error_message, const std::string& content_type,
const std::vector<u8>& response) { getLatestReleaseComplete(status_code, error_message, response); });
req->get(fmt::format(LATEST_RELEASE_URL, getCurrentUpdateTag()), this);
}
void AutoUpdaterDialog::getLatestTagComplete(s32 status_code, Error& error, std::vector<u8>& response,
bool display_errors)
void AutoUpdaterDialog::getLatestTagComplete(s32 status_code, const std::string& error_message,
const std::vector<u8>& response, bool display_errors)
{
if (handleCancelledRequest(status_code))
return;
@ -411,13 +411,14 @@ void AutoUpdaterDialog::getLatestTagComplete(s32 status_code, Error& error, std:
else
{
if (display_errors)
reportError(fmt::format("Failed to download latest tag info: {}", error.GetDescription()));
reportError(fmt::format("Failed to download latest tag info: {}", error_message));
}
emit updateCheckCompleted(false);
}
void AutoUpdaterDialog::getLatestReleaseComplete(s32 status_code, Error& error, std::vector<u8>& response)
void AutoUpdaterDialog::getLatestReleaseComplete(s32 status_code, const std::string& error_message,
const std::vector<u8>& response)
{
if (handleCancelledRequest(status_code))
return;
@ -489,7 +490,7 @@ void AutoUpdaterDialog::getLatestReleaseComplete(s32 status_code, Error& error,
}
else
{
reportError(fmt::format("Failed to download latest release info: {}", error.GetDescription()));
reportError(fmt::format("Failed to download latest release info: {}", error_message));
}
emit updateCheckCompleted(false);
@ -499,15 +500,15 @@ void AutoUpdaterDialog::queueGetChanges()
{
AsyncHTTPRequest* const req = new AsyncHTTPRequest();
connect(req, &AsyncHTTPRequest::requestComplete, this,
[this](s32 status_code, Error& error, std::string& content_type, std::vector<u8>& response) {
getChangesComplete(status_code, error, response);
});
[this](s32 status_code, const std::string& error_message, const std::string& content_type,
const std::vector<u8>& response) { getChangesComplete(status_code, error_message, response); });
req->get(fmt::format(CHANGES_URL, g_scm_hash_str, getCurrentUpdateTag()), this);
}
void AutoUpdaterDialog::getChangesComplete(s32 status_code, Error& error, std::vector<u8>& response)
void AutoUpdaterDialog::getChangesComplete(s32 status_code, const std::string& error_message,
const std::vector<u8>& response)
{
std::string_view error_message;
std::string_view error_message_to_display;
if (status_code == HTTPDownloader::HTTP_STATUS_OK)
{
@ -557,19 +558,19 @@ void AutoUpdaterDialog::getChangesComplete(s32 status_code, Error& error, std::v
}
else
{
error_message = "Change list JSON is not an object";
error_message_to_display = "Change list JSON is not an object";
}
}
else
{
error_message = error.GetDescription();
error_message_to_display = error_message;
}
m_ui.updateNotes->setText(QString::fromStdString(
fmt::format("<h2>Failed to download change list</h2><p>The error was:<pre>{}</pre></p><p>You may be able to "
"install this update anyway. If the download installation fails, you can download the update "
"from:</p><p><a href=\"" DOWNLOAD_PAGE_URL "\">" DOWNLOAD_PAGE_URL "</a></p>",
error_message, UPDATER_RELEASE_CHANNEL, UPDATER_RELEASE_CHANNEL)));
error_message_to_display, UPDATER_RELEASE_CHANNEL, UPDATER_RELEASE_CHANNEL)));
}
void AutoUpdaterDialog::downloadUpdateClicked()
@ -587,13 +588,13 @@ void AutoUpdaterDialog::downloadUpdateClicked()
AsyncHTTPRequest* const req = new AsyncHTTPRequest();
connect(req, &AsyncHTTPRequest::requestComplete, this,
[this](s32 status_code, Error& error, std::string&, std::vector<u8>& response) {
downloadUpdateComplete(status_code, error, response);
});
[this](s32 status_code, const std::string& error_message, const std::string& content_type,
const std::vector<u8>& response) { downloadUpdateComplete(status_code, error_message, response); });
req->get(m_download_url.toStdString(), this, m_download_progress_callback);
}
void AutoUpdaterDialog::downloadUpdateComplete(s32 status_code, Error& error, std::vector<u8>& response)
void AutoUpdaterDialog::downloadUpdateComplete(s32 status_code, const std::string& error_message,
const std::vector<u8>& response)
{
DebugAssert(m_download_progress_callback);
m_download_progress_callback->SetState(TRANSLATE_SV("AutoUpdaterWindow", "Processing Update..."), 1, 1);
@ -608,7 +609,7 @@ void AutoUpdaterDialog::downloadUpdateComplete(s32 status_code, Error& error, st
if (status_code != HTTPDownloader::HTTP_STATUS_OK)
{
reportError(fmt::format("Download failed: {}", error.GetDescription()));
reportError(fmt::format("Download failed: {}", error_message));
setDownloadSectionVisibility(false);
return;
}

@ -65,13 +65,14 @@ private:
bool updateNeeded() const;
void getLatestTagComplete(s32 status_code, Error& error, std::vector<u8>& response, bool display_errors);
void getLatestReleaseComplete(s32 status_code, Error& error, std::vector<u8>& response);
void getLatestTagComplete(s32 status_code, const std::string& error_message, const std::vector<u8>& response,
bool display_errors);
void getLatestReleaseComplete(s32 status_code, const std::string& error_message, const std::vector<u8>& response);
void queueGetChanges();
void getChangesComplete(s32 status_code, Error& error, std::vector<u8>& response);
void getChangesComplete(s32 status_code, const std::string& error_message, const std::vector<u8>& response);
void downloadUpdateComplete(s32 status_code, Error& error, std::vector<u8>& response);
void downloadUpdateComplete(s32 status_code, const std::string& error_message, const std::vector<u8>& response);
bool processUpdate(const std::vector<u8>& update_data);
#ifdef _WIN32

@ -812,10 +812,11 @@ void QtHost::DownloadFile(QWidget* parent, std::string url, std::string path,
bool result = false;
HTTPDownloader::CreateRequest(
std::move(url), parent,
[&result, &error, &path](s32 status_code, Error& http_error, std::string&, std::vector<u8>& hdata) {
[&result, &error, &path](s32 status_code, std::string_view http_error, std::string_view content_type,
std::vector<u8> hdata) {
if (status_code != HTTPDownloader::HTTP_STATUS_OK)
{
error.SetString(http_error.GetDescription());
error.SetStringView(http_error);
return;
}
else if (hdata.empty())

@ -33,8 +33,8 @@ static constexpr u32 CACHE_VERSION = 1;
static void QueueDownload(std::string_view url, FetchCallback callback, Error* error,
std::unique_lock<Threading::Mutex>&& lock);
static void DownloadCallback(const std::string& url, s32 status_code, const Error& error,
const std::string& content_type, const HTTPDownloader::RequestData& data);
static void DownloadCallback(const std::string& url, s32 status_code, const std::string_view& error,
const HTTPDownloader::RequestData& data);
namespace {
@ -199,15 +199,14 @@ void HTTPCache::QueueDownload(std::string_view url, FetchCallback callback, Erro
// release lock because CreateRequest() can fire the callback immediately
lock.unlock();
HTTPDownloader::CreateRequest(std::string(url), &s_locals,
[url = std::string(url)](s32 status_code, Error& error, std::string& content_type,
HTTPDownloader::RequestData& data) {
DownloadCallback(url, status_code, error, content_type, std::move(data));
});
HTTPDownloader::CreateRequest(
std::string(url), &s_locals,
[url = std::string(url)](s32 status_code, std::string_view error, std::string_view content_type,
HTTPDownloader::RequestData data) { DownloadCallback(url, status_code, error, data); });
}
void HTTPCache::DownloadCallback(const std::string& url, s32 status_code, const Error& error,
const std::string& content_type, const HTTPDownloader::RequestData& data)
void HTTPCache::DownloadCallback(const std::string& url, s32 status_code, const std::string_view& error,
const HTTPDownloader::RequestData& data)
{
// hold the lock for the insertion, so we don't create a duplicate request as described in Lookup()
std::unique_lock lock(s_locals.pending_downloads_lock);
@ -228,7 +227,7 @@ void HTTPCache::DownloadCallback(const std::string& url, s32 status_code, const
}
else
{
ERROR_LOG("Failed to download '{}': HTTP status code {}, error: {}", url, status_code, error.GetDescription());
ERROR_LOG("Failed to download '{}': HTTP status code {}, error: {}", url, status_code, error);
}
// invoke all callbacks. uses indexing in case something gets added in the callback

@ -291,7 +291,7 @@ void HTTPDownloader::LockedPollRequests(std::unique_lock<Threading::Mutex>& lock
lock.unlock();
req->error.SetStringFmt("Request timed out after {} seconds.", req->timeout_seconds);
req->callback(HTTP_STATUS_TIMEOUT, req->error, req->content_type, req->data);
req->callback(HTTP_STATUS_TIMEOUT, req->error.GetDescription(), {}, {});
CloseRequest(req);
@ -309,7 +309,7 @@ void HTTPDownloader::LockedPollRequests(std::unique_lock<Threading::Mutex>& lock
lock.unlock();
req->error.SetStringView("Request was cancelled.");
req->callback(HTTP_STATUS_CANCELLED, req->error, req->content_type, req->data);
req->callback(HTTP_STATUS_CANCELLED, req->error.GetDescription(), {}, {});
CloseRequest(req);
@ -347,7 +347,7 @@ void HTTPDownloader::LockedPollRequests(std::unique_lock<Threading::Mutex>& lock
else if (req->status_code < 0)
DEV_LOG("Request failed with error {}", req->error.GetDescription());
req->callback(req->status_code, req->error, req->content_type, req->data);
req->callback(req->status_code, req->error.GetDescription(), req->content_type, std::move(req->data));
CloseRequest(req);
lock.lock();
}
@ -537,8 +537,7 @@ void HTTPDownloader::CancelRequestsForOwner(const void* owner)
s_locals.pending_http_requests.erase(s_locals.pending_http_requests.begin() + index);
lock.unlock();
req->error.SetStringView("Request was cancelled.");
req->callback(HTTP_STATUS_CANCELLED, req->error, req->content_type, req->data);
req->callback(HTTP_STATUS_CANCELLED, "Request was cancelled.", {}, {});
// If pending, we can delete it immediately since it won't be processed by the worker thread.
// Otherwise, we need to close it so the worker thread can clean up properly.
@ -557,7 +556,7 @@ void HTTPDownloader::CancelRequestsForOwner(const void* owner)
Host::OnHTTPDownloaderActiveChanged(false);
}
std::string HTTPDownloader::GetExtensionForContentType(const std::string& content_type)
std::string_view HTTPDownloader::GetExtensionForContentType(std::string_view content_type)
{
// Based on https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types
static constexpr const char* table[][2] = {
@ -638,16 +637,14 @@ std::string HTTPDownloader::GetExtensionForContentType(const std::string& conten
{"application/x-7z-compressed", "7z"},
};
std::string ret;
for (size_t i = 0; i < std::size(table); i++)
const std::string_view mime = StringUtil::StripWhitespace(content_type.substr(0, content_type.find(';')));
for (const auto& [table_mime, table_extension] : table)
{
if (StringUtil::Strncasecmp(table[i][0], content_type.data(), content_type.length()) == 0)
{
ret = table[i][1];
break;
}
if (StringUtil::EqualNoCase(mime, table_mime))
return table_extension;
}
return ret;
return {};
}
#if defined(USE_WINHTTP)
@ -888,7 +885,7 @@ void HTTPDownloader::InternalCreateRequest(Request::Type type, std::string url,
if (!EnsureInitialized(&req->error))
{
callback(HTTP_STATUS_ERROR, req->error, req->content_type, req->data);
callback(HTTP_STATUS_ERROR, req->error.GetDescription(), req->content_type, req->data);
DeleteRequest(req);
return;
}
@ -935,7 +932,7 @@ bool HTTPDownloader::StartRequest(Request* req)
const DWORD err = GetLastError();
ERROR_LOG("WinHttpCrackUrl() failed: {}", err);
req->error.SetWin32("WinHttpCrackUrl() failed: ", err);
req->callback(HTTP_STATUS_ERROR, req->error, req->content_type, req->data);
req->callback(HTTP_STATUS_ERROR, req->error.GetDescription(), {}, {});
DeleteRequest(req);
return false;
}
@ -949,7 +946,7 @@ bool HTTPDownloader::StartRequest(Request* req)
const DWORD err = GetLastError();
ERROR_LOG("Failed to start HTTP request for '{}': {}", req->url, err);
req->error.SetWin32("WinHttpConnect() failed: ", err);
req->callback(HTTP_STATUS_ERROR, req->error, req->content_type, req->data);
req->callback(HTTP_STATUS_ERROR, req->error.GetDescription(), {}, {});
DeleteRequest(req);
return false;
}
@ -962,7 +959,7 @@ bool HTTPDownloader::StartRequest(Request* req)
const DWORD err = GetLastError();
ERROR_LOG("WinHttpOpenRequest() failed: {}", err);
req->error.SetWin32("WinHttpOpenRequest() failed: ", err);
req->callback(HTTP_STATUS_ERROR, req->error, req->content_type, req->data);
req->callback(HTTP_STATUS_ERROR, req->error.GetDescription(), {}, {});
WinHttpCloseHandle(req->hConnection);
DeleteRequest(req);
return false;
@ -976,7 +973,7 @@ bool HTTPDownloader::StartRequest(Request* req)
const DWORD err = GetLastError();
ERROR_LOG("WinHttpAddRequestHeaders() failed: {}", err);
req->error.SetWin32("WinHttpAddRequestHeaders() failed: ", err);
req->callback(HTTP_STATUS_ERROR, req->error, req->content_type, req->data);
req->callback(HTTP_STATUS_ERROR, req->error.GetDescription(), {}, {});
WinHttpCloseHandle(req->hRequest);
WinHttpCloseHandle(req->hConnection);
DeleteRequest(req);
@ -1144,7 +1141,7 @@ void HTTPDownloader::InternalCreateRequest(Request::Type type, std::string url,
if (!EnsureInitialized(&req->error))
{
callback(HTTP_STATUS_ERROR, req->error, req->content_type, req->data);
callback(HTTP_STATUS_ERROR, req->error.GetDescription(), req->content_type, req->data);
DeleteRequest(req);
return;
}
@ -1290,8 +1287,7 @@ bool HTTPDownloader::StartRequest(Request* req)
if (!req->handle)
{
ERROR_LOG("curl_easy_init() failed");
req->error.SetStringView("curl_easy_init() failed");
req->callback(HTTP_STATUS_ERROR, req->error, req->content_type, req->data);
req->callback(HTTP_STATUS_ERROR, "curl_easy_init() failed", {}, {});
DeleteRequest(req);
return false;
}

@ -29,12 +29,12 @@ using RequestData = std::vector<u8>;
/// Callback fired when a request completes, times out, or is cancelled.
/// Invoked on the thread that calls PollRequests(), with no internal locks held.
///
/// @param status_code HTTP status code, or one of the negative HTTP_STATUS_* sentinels on failure.
/// @param error Populated with a description when status_code < HTTP_STATUS_OK.
/// @param content_type Value of the response Content-Type header; empty if unavailable.
/// @param data Response body; empty if the request did not succeed.
/// @param status_code HTTP status code, or one of the negative HTTP_STATUS_* sentinels on failure.
/// @param error_message Populated with a description when status_code < HTTP_STATUS_OK.
/// @param content_type Value of the response Content-Type header; empty if unavailable.
/// @param data Response body; empty if the request did not succeed.
using RequestCallback =
std::function<void(s32 status_code, Error& error, std::string& content_type, RequestData& data)>;
std::function<void(s32 status_code, std::string_view error_message, std::string_view content_type, RequestData data)>;
/// Synthetic status codes used in place of a real HTTP status on failure.
enum : s32
@ -47,7 +47,7 @@ enum : s32
/// Returns the file extension (without leading dot) for the given MIME type,
/// or an empty string if the type is not recognised.
std::string GetExtensionForContentType(const std::string& content_type);
std::string_view GetExtensionForContentType(std::string_view content_type);
/// Sets the default timeout applied to new requests when none is explicitly provided.
/// The initial default is 30 seconds.

Loading…
Cancel
Save