From dac5dd562b37b1a9c7ab570770ec93cfe3bd9004 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 29 Nov 2024 13:48:19 +1000 Subject: [PATCH] HTTPDownloader: Improve error reporting Give something human-readable when an error occurs. --- src/common/error.h | 2 +- src/core/achievements.cpp | 31 +++++--- src/core/game_list.cpp | 65 ++++++++-------- src/duckstation-qt/autoupdaterdialog.cpp | 27 +++---- src/duckstation-qt/autoupdaterdialog.h | 6 +- src/duckstation-qt/qthost.cpp | 24 +++--- src/util/CMakeLists.txt | 2 - src/util/http_downloader.cpp | 10 ++- src/util/http_downloader.h | 7 +- src/util/http_downloader_curl.cpp | 49 +++++++++--- src/util/http_downloader_curl.h | 36 --------- src/util/http_downloader_winhttp.cpp | 95 +++++++++++++++++++----- src/util/http_downloader_winhttp.h | 38 ---------- src/util/util.vcxproj | 4 - src/util/util.vcxproj.filters | 2 - 15 files changed, 214 insertions(+), 184 deletions(-) delete mode 100644 src/util/http_downloader_curl.h delete mode 100644 src/util/http_downloader_winhttp.h diff --git a/src/common/error.h b/src/common/error.h index 1e9eebbfa..5712cbe47 100644 --- a/src/common/error.h +++ b/src/common/error.h @@ -141,6 +141,6 @@ private: void AddPrefixFmtArgs(fmt::string_view fmt, fmt::format_args args); void AddSuffixFmtArgs(fmt::string_view fmt, fmt::format_args args); - Type m_type = Type::None; std::string m_description; + Type m_type = Type::None; }; diff --git a/src/core/achievements.cpp b/src/core/achievements.cpp index b8d75c51f..54f560701 100644 --- a/src/core/achievements.cpp +++ b/src/core/achievements.cpp @@ -354,10 +354,13 @@ std::string Achievements::GetLocalImagePath(const std::string_view image_name, i void Achievements::DownloadImage(std::string url, std::string cache_filename) { - auto callback = [cache_filename](s32 status_code, const std::string& content_type, + auto callback = [cache_filename](s32 status_code, const Error& error, const std::string& content_type, HTTPDownloader::Request::Data data) { if (status_code != HTTPDownloader::HTTP_STATUS_OK) + { + ERROR_LOG("Failed to download badge '{}': {}", Path::GetFileName(cache_filename), error.GetDescription()); return; + } if (!FileSystem::WriteBinaryFile(cache_filename.c_str(), data.data(), data.size())) { @@ -753,18 +756,22 @@ 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::Request::Callback hd_callback = - [callback, callback_data](s32 status_code, const std::string& content_type, HTTPDownloader::Request::Data data) { - rc_api_server_response_t rr; - rr.http_status_code = (status_code <= 0) ? (status_code == HTTPDownloader::HTTP_STATUS_CANCELLED ? - RC_API_SERVER_RESPONSE_CLIENT_ERROR : - RC_API_SERVER_RESPONSE_RETRYABLE_CLIENT_ERROR) : - status_code; - rr.body_length = data.size(); - rr.body = reinterpret_cast(data.data()); + HTTPDownloader::Request::Callback hd_callback = [callback, callback_data](s32 status_code, const Error& error, + const std::string& content_type, + HTTPDownloader::Request::Data data) { + if (status_code != HTTPDownloader::HTTP_STATUS_OK) + ERROR_LOG("Server call failed: {}", error.GetDescription()); - callback(&rr, callback_data); - }; + rc_api_server_response_t rr; + rr.http_status_code = (status_code <= 0) ? (status_code == HTTPDownloader::HTTP_STATUS_CANCELLED ? + RC_API_SERVER_RESPONSE_CLIENT_ERROR : + RC_API_SERVER_RESPONSE_RETRYABLE_CLIENT_ERROR) : + status_code; + rr.body_length = data.size(); + rr.body = reinterpret_cast(data.data()); + + callback(&rr, callback_data); + }; HTTPDownloader* http = static_cast(rc_client_get_userdata(client)); diff --git a/src/core/game_list.cpp b/src/core/game_list.cpp index bffd38f26..bf7f93877 100644 --- a/src/core/game_list.cpp +++ b/src/core/game_list.cpp @@ -1454,10 +1454,11 @@ bool GameList::DownloadCovers(const std::vector& url_templates, boo return false; } - std::unique_ptr downloader(HTTPDownloader::Create(Host::GetHTTPUserAgent())); + Error error; + std::unique_ptr downloader(HTTPDownloader::Create(Host::GetHTTPUserAgent(), &error)); if (!downloader) { - progress->DisplayError("Failed to create HTTP downloader."); + progress->DisplayError(fmt::format("Failed to create HTTP downloader:\n{}", error.GetDescription())); return false; } @@ -1484,39 +1485,43 @@ bool GameList::DownloadCovers(const std::vector& url_templates, boo // we could actually do a few in parallel here... std::string filename = Path::URLDecode(url); - downloader->CreateRequest( - std::move(url), [use_serial, &save_callback, entry_path = std::move(entry_path), filename = std::move(filename)]( - s32 status_code, const std::string& content_type, HTTPDownloader::Request::Data data) { - if (status_code != HTTPDownloader::HTTP_STATUS_OK || data.empty()) - return; + downloader->CreateRequest(std::move(url), [use_serial, &save_callback, entry_path = std::move(entry_path), + filename = std::move(filename)](s32 status_code, const Error& error, + const std::string& content_type, + HTTPDownloader::Request::Data data) { + if (status_code != HTTPDownloader::HTTP_STATUS_OK || data.empty()) + { + ERROR_LOG("Download for {} failed: {}", Path::GetFileName(filename), error.GetDescription()); + return; + } - std::unique_lock lock(s_mutex); - const GameList::Entry* entry = GetEntryForPath(entry_path); - if (!entry || !GetCoverImagePathForEntry(entry).empty()) - return; + std::unique_lock lock(s_mutex); + const GameList::Entry* entry = GetEntryForPath(entry_path); + if (!entry || !GetCoverImagePathForEntry(entry).empty()) + return; - // 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. - std::string template_filename; - std::string content_type_extension(HTTPDownloader::GetExtensionForContentType(content_type)); + // 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. + 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('/'); - const std::string::size_type last_dot = filename.find('.'); - if (!content_type_extension.empty()) - template_filename = fmt::format("cover.{}", content_type_extension); - else if (last_slash != std::string::npos && last_dot != std::string::npos && last_dot > last_slash) - template_filename = Path::GetFileName(filename); - else - template_filename = "cover.jpg"; + // don't treat the domain name as an extension.. + const std::string::size_type last_slash = filename.find('/'); + const std::string::size_type last_dot = filename.find('.'); + if (!content_type_extension.empty()) + template_filename = fmt::format("cover.{}", content_type_extension); + else if (last_slash != std::string::npos && last_dot != std::string::npos && last_dot > last_slash) + template_filename = Path::GetFileName(filename); + else + template_filename = "cover.jpg"; - std::string write_path(GetNewCoverImagePathForEntry(entry, template_filename.c_str(), use_serial)); - if (write_path.empty()) - return; + std::string write_path(GetNewCoverImagePathForEntry(entry, template_filename.c_str(), use_serial)); + if (write_path.empty()) + return; - if (FileSystem::WriteBinaryFile(write_path.c_str(), data.data(), data.size()) && save_callback) - save_callback(entry, std::move(write_path)); - }); + if (FileSystem::WriteBinaryFile(write_path.c_str(), data.data(), data.size()) && save_callback) + save_callback(entry, std::move(write_path)); + }); downloader->WaitForAllRequests(); progress->IncrementProgressValue(); } diff --git a/src/duckstation-qt/autoupdaterdialog.cpp b/src/duckstation-qt/autoupdaterdialog.cpp index 26f6abbb4..e5f7d8365 100644 --- a/src/duckstation-qt/autoupdaterdialog.cpp +++ b/src/duckstation-qt/autoupdaterdialog.cpp @@ -88,9 +88,10 @@ AutoUpdaterDialog::AutoUpdaterDialog(QWidget* parent /* = nullptr */) : QDialog( connect(m_ui.skipThisUpdate, &QPushButton::clicked, this, &AutoUpdaterDialog::skipThisUpdateClicked); connect(m_ui.remindMeLater, &QPushButton::clicked, this, &AutoUpdaterDialog::remindMeLaterClicked); - m_http = HTTPDownloader::Create(Host::GetHTTPUserAgent()); + Error error; + m_http = HTTPDownloader::Create(Host::GetHTTPUserAgent(), &error); if (!m_http) - ERROR_LOG("Failed to create HTTP downloader, auto updater will not be available."); + ERROR_LOG("Failed to create HTTP downloader, auto updater will not be available:\n{}", error.GetDescription()); } AutoUpdaterDialog::~AutoUpdaterDialog() = default; @@ -277,7 +278,7 @@ void AutoUpdaterDialog::queueUpdateCheck(bool display_message) } m_http->CreateRequest(LATEST_TAG_URL, std::bind(&AutoUpdaterDialog::getLatestTagComplete, this, std::placeholders::_1, - std::placeholders::_3)); + std::placeholders::_2, std::placeholders::_4)); #else emit updateCheckCompleted(); #endif @@ -294,11 +295,11 @@ void AutoUpdaterDialog::queueGetLatestRelease() std::string url = fmt::format(fmt::runtime(LATEST_RELEASE_URL), getCurrentUpdateTag()); m_http->CreateRequest(std::move(url), std::bind(&AutoUpdaterDialog::getLatestReleaseComplete, this, - std::placeholders::_1, std::placeholders::_3)); + std::placeholders::_1, std::placeholders::_2, std::placeholders::_4)); #endif } -void AutoUpdaterDialog::getLatestTagComplete(s32 status_code, std::vector response) +void AutoUpdaterDialog::getLatestTagComplete(s32 status_code, const Error& error, std::vector response) { #ifdef UPDATE_CHECKER_SUPPORTED const std::string selected_tag(getCurrentUpdateTag()); @@ -351,14 +352,14 @@ void AutoUpdaterDialog::getLatestTagComplete(s32 status_code, std::vector re else { if (m_display_messages) - reportError(fmt::format("Failed to download latest tag info: HTTP {}", status_code)); + reportError(fmt::format("Failed to download latest tag info: {}", error.GetDescription())); } emit updateCheckCompleted(); #endif } -void AutoUpdaterDialog::getLatestReleaseComplete(s32 status_code, std::vector response) +void AutoUpdaterDialog::getLatestReleaseComplete(s32 status_code, const Error& error, std::vector response) { #ifdef UPDATE_CHECKER_SUPPORTED if (status_code == HTTPDownloader::HTTP_STATUS_OK) @@ -415,7 +416,7 @@ void AutoUpdaterDialog::getLatestReleaseComplete(s32 status_code, std::vectorCreateRequest(std::move(url), std::bind(&AutoUpdaterDialog::getChangesComplete, this, std::placeholders::_1, - std::placeholders::_3)); + std::placeholders::_2, std::placeholders::_4)); #endif } -void AutoUpdaterDialog::getChangesComplete(s32 status_code, std::vector response) +void AutoUpdaterDialog::getChangesComplete(s32 status_code, const Error& error, std::vector response) { #ifdef UPDATE_CHECKER_SUPPORTED if (status_code == HTTPDownloader::HTTP_STATUS_OK) @@ -503,7 +504,7 @@ void AutoUpdaterDialog::getChangesComplete(s32 status_code, std::vector resp } else { - reportError(fmt::format("Failed to download change list: HTTP {}", status_code)); + reportError(fmt::format("Failed to download change list: {}", error.GetDescription())); } #endif } @@ -527,13 +528,13 @@ void AutoUpdaterDialog::downloadUpdateClicked() m_http->CreateRequest( m_download_url.toStdString(), - [this, &download_result](s32 status_code, const std::string&, std::vector response) { + [this, &download_result](s32 status_code, const Error& error, const std::string&, std::vector response) { if (status_code == HTTPDownloader::HTTP_STATUS_CANCELLED) return; if (status_code != HTTPDownloader::HTTP_STATUS_OK) { - reportError(fmt::format("Download failed: HTTP status code {}", status_code)); + reportError(fmt::format("Download failed: {}", error.GetDescription())); download_result = false; return; } diff --git a/src/duckstation-qt/autoupdaterdialog.h b/src/duckstation-qt/autoupdaterdialog.h index 713f0e84e..f8d83a8a2 100644 --- a/src/duckstation-qt/autoupdaterdialog.h +++ b/src/duckstation-qt/autoupdaterdialog.h @@ -58,11 +58,11 @@ private: bool updateNeeded() const; std::string getCurrentUpdateTag() const; - void getLatestTagComplete(s32 status_code, std::vector response); - void getLatestReleaseComplete(s32 status_code, std::vector response); + void getLatestTagComplete(s32 status_code, const Error& error, std::vector response); + void getLatestReleaseComplete(s32 status_code, const Error& error, std::vector response); void queueGetChanges(); - void getChangesComplete(s32 status_code, std::vector response); + void getChangesComplete(s32 status_code, const Error& error, std::vector response); bool processUpdate(const std::vector& update_data); diff --git a/src/duckstation-qt/qthost.cpp b/src/duckstation-qt/qthost.cpp index 50c05715c..9946875a2 100644 --- a/src/duckstation-qt/qthost.cpp +++ b/src/duckstation-qt/qthost.cpp @@ -261,11 +261,13 @@ std::optional QtHost::DownloadFile(QWidget* parent, const QString& title, { static constexpr u32 HTTP_POLL_INTERVAL = 10; - std::unique_ptr http = HTTPDownloader::Create(Host::GetHTTPUserAgent()); + Error error; + std::unique_ptr http = HTTPDownloader::Create(Host::GetHTTPUserAgent(), &error); if (!http) { QMessageBox::critical(parent, qApp->translate("QtHost", "Error"), - qApp->translate("QtHost", "Failed to create HTTPDownloader.")); + qApp->translate("QtHost", "Failed to create HTTPDownloader:\n%1") + .arg(QString::fromStdString(error.GetDescription()))); return false; } @@ -281,14 +283,16 @@ std::optional QtHost::DownloadFile(QWidget* parent, const QString& title, http->CreateRequest( std::move(url), - [parent, data, &download_result](s32 status_code, const std::string&, std::vector hdata) { + [parent, data, &download_result](s32 status_code, const Error& error, const std::string&, std::vector hdata) { if (status_code == HTTPDownloader::HTTP_STATUS_CANCELLED) return; if (status_code != HTTPDownloader::HTTP_STATUS_OK) { QMessageBox::critical(parent, qApp->translate("QtHost", "Error"), - qApp->translate("QtHost", "Download failed with HTTP status code %1.").arg(status_code)); + qApp->translate("QtHost", "Download failed with HTTP status code %1:\n%2") + .arg(status_code) + .arg(QString::fromStdString(error.GetDescription()))); download_result = false; return; } @@ -1913,11 +1917,13 @@ std::string Host::GetClipboardText() { // Hope this doesn't deadlock... std::string ret; - QtHost::RunOnUIThread([&ret]() { - QClipboard* clipboard = QGuiApplication::clipboard(); - if (clipboard) - ret = clipboard->text().toStdString(); - }, true); + QtHost::RunOnUIThread( + [&ret]() { + QClipboard* clipboard = QGuiApplication::clipboard(); + if (clipboard) + ret = clipboard->text().toStdString(); + }, + true); return ret; } diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index f1c6e91ec..68fa264fa 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -224,7 +224,6 @@ if(WIN32) dinput_source.cpp dinput_source.h http_downloader_winhttp.cpp - http_downloader_winhttp.h platform_misc_win32.cpp win32_raw_input_source.cpp win32_raw_input_source.h @@ -275,7 +274,6 @@ endif() if(NOT WIN32 AND NOT ANDROID) target_sources(util PRIVATE http_downloader_curl.cpp - http_downloader_curl.h ) target_link_libraries(util PRIVATE CURL::libcurl diff --git a/src/util/http_downloader.cpp b/src/util/http_downloader.cpp index abb34a9d2..11ef826b9 100644 --- a/src/util/http_downloader.cpp +++ b/src/util/http_downloader.cpp @@ -109,7 +109,8 @@ void HTTPDownloader::LockedPollRequests(std::unique_lock& lock) m_pending_http_requests.erase(m_pending_http_requests.begin() + index); lock.unlock(); - req->callback(HTTP_STATUS_TIMEOUT, std::string(), Request::Data()); + req->error.SetStringFmt("Request timed out after {} seconds.", m_timeout); + req->callback(HTTP_STATUS_TIMEOUT, req->error, std::string(), Request::Data()); CloseRequest(req); @@ -126,7 +127,8 @@ void HTTPDownloader::LockedPollRequests(std::unique_lock& lock) m_pending_http_requests.erase(m_pending_http_requests.begin() + index); lock.unlock(); - req->callback(HTTP_STATUS_CANCELLED, std::string(), Request::Data()); + req->error.SetStringView("Request was cancelled."); + req->callback(HTTP_STATUS_CANCELLED, req->error, std::string(), Request::Data()); CloseRequest(req); @@ -159,7 +161,9 @@ void HTTPDownloader::LockedPollRequests(std::unique_lock& lock) // run callback with lock unheld lock.unlock(); - req->callback(req->status_code, req->content_type, std::move(req->data)); + if (req->status_code != HTTP_STATUS_OK) + req->error.SetStringFmt("Request failed with HTTP status code {}", req->status_code); + req->callback(req->status_code, req->error, req->content_type, std::move(req->data)); CloseRequest(req); lock.lock(); } diff --git a/src/util/http_downloader.h b/src/util/http_downloader.h index 75149d1b8..4fa2b95f3 100644 --- a/src/util/http_downloader.h +++ b/src/util/http_downloader.h @@ -3,6 +3,7 @@ #pragma once +#include "common/error.h" #include "common/types.h" #include @@ -29,7 +30,8 @@ public: struct Request { using Data = std::vector; - using Callback = std::function; + using Callback = + std::function; enum class Type { @@ -53,6 +55,7 @@ public: std::string post_data; std::string content_type; Data data; + Error error; u64 start_time; s32 status_code = 0; u32 content_length = 0; @@ -64,7 +67,7 @@ public: HTTPDownloader(); virtual ~HTTPDownloader(); - static std::unique_ptr Create(std::string user_agent = DEFAULT_USER_AGENT); + static std::unique_ptr Create(std::string user_agent = DEFAULT_USER_AGENT, Error* error = nullptr); static std::string GetExtensionForContentType(const std::string& content_type); void SetTimeout(float timeout); diff --git a/src/util/http_downloader_curl.cpp b/src/util/http_downloader_curl.cpp index eca5429d7..cece67103 100644 --- a/src/util/http_downloader_curl.cpp +++ b/src/util/http_downloader_curl.cpp @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: 2019-2024 Connor McLaughlin // SPDX-License-Identifier: CC-BY-NC-ND-4.0 -#include "http_downloader_curl.h" +#include "http_downloader.h" #include "common/assert.h" #include "common/log.h" @@ -9,12 +9,41 @@ #include "common/timer.h" #include +#include #include #include #include LOG_CHANNEL(HTTPDownloader); +namespace { +class HTTPDownloaderCurl final : public HTTPDownloader +{ +public: + HTTPDownloaderCurl(); + ~HTTPDownloaderCurl() override; + + bool Initialize(std::string user_agent, Error* error); + +protected: + Request* InternalCreateRequest() override; + void InternalPollRequests() override; + bool StartRequest(HTTPDownloader::Request* request) override; + void CloseRequest(HTTPDownloader::Request* request) override; + +private: + struct Request : HTTPDownloader::Request + { + CURL* handle = nullptr; + }; + + static size_t WriteCallback(char* ptr, size_t size, size_t nmemb, void* userdata); + + CURLM* m_multi_handle = nullptr; + std::string m_user_agent; +}; +} // namespace + HTTPDownloaderCurl::HTTPDownloaderCurl() : HTTPDownloader() { } @@ -25,11 +54,11 @@ HTTPDownloaderCurl::~HTTPDownloaderCurl() curl_multi_cleanup(m_multi_handle); } -std::unique_ptr HTTPDownloader::Create(std::string user_agent) +std::unique_ptr HTTPDownloader::Create(std::string user_agent, Error* error) { - std::unique_ptr instance(std::make_unique()); - if (!instance->Initialize(std::move(user_agent))) - return {}; + std::unique_ptr instance = std::make_unique(); + if (!instance->Initialize(std::move(user_agent), error)) + instance.reset(); return instance; } @@ -37,7 +66,7 @@ std::unique_ptr HTTPDownloader::Create(std::string user_agent) static bool s_curl_initialized = false; static std::once_flag s_curl_initialized_once_flag; -bool HTTPDownloaderCurl::Initialize(std::string user_agent) +bool HTTPDownloaderCurl::Initialize(std::string user_agent, Error* error) { if (!s_curl_initialized) { @@ -53,7 +82,7 @@ bool HTTPDownloaderCurl::Initialize(std::string user_agent) }); if (!s_curl_initialized) { - ERROR_LOG("curl_global_init() failed"); + Error::SetStringView(error, "curl_global_init() failed"); return false; } } @@ -61,7 +90,7 @@ bool HTTPDownloaderCurl::Initialize(std::string user_agent) m_multi_handle = curl_multi_init(); if (!m_multi_handle) { - ERROR_LOG("curl_multi_init() failed"); + Error::SetStringView(error, "curl_multi_init() failed"); return false; } @@ -153,6 +182,7 @@ void HTTPDownloaderCurl::InternalPollRequests() else { ERROR_LOG("Request for '{}' returned error {}", req->url, static_cast(msg->data.result)); + req->error.SetStringFmt("Request failed: {}", curl_easy_strerror(msg->data.result)); } req->state.store(Request::State::Complete, std::memory_order_release); @@ -187,7 +217,8 @@ bool HTTPDownloaderCurl::StartRequest(HTTPDownloader::Request* request) if (err != CURLM_OK) { ERROR_LOG("curl_multi_add_handle() returned {}", static_cast(err)); - req->callback(HTTP_STATUS_ERROR, std::string(), req->data); + req->error.SetStringFmt("curl_multi_add_handle() failed: {}", curl_multi_strerror(err)); + req->callback(HTTP_STATUS_ERROR, req->error, std::string(), req->data); curl_easy_cleanup(req->handle); delete req; return false; diff --git a/src/util/http_downloader_curl.h b/src/util/http_downloader_curl.h deleted file mode 100644 index e687be8cd..000000000 --- a/src/util/http_downloader_curl.h +++ /dev/null @@ -1,36 +0,0 @@ -// SPDX-FileCopyrightText: 2019-2024 Connor McLaughlin -// SPDX-License-Identifier: CC-BY-NC-ND-4.0 - -#pragma once -#include "http_downloader.h" - -#include -#include -#include -#include - -class HTTPDownloaderCurl final : public HTTPDownloader -{ -public: - HTTPDownloaderCurl(); - ~HTTPDownloaderCurl() override; - - bool Initialize(std::string user_agent); - -protected: - Request* InternalCreateRequest() override; - void InternalPollRequests() override; - bool StartRequest(HTTPDownloader::Request* request) override; - void CloseRequest(HTTPDownloader::Request* request) override; - -private: - struct Request : HTTPDownloader::Request - { - CURL* handle = nullptr; - }; - - static size_t WriteCallback(char* ptr, size_t size, size_t nmemb, void* userdata); - - CURLM* m_multi_handle = nullptr; - std::string m_user_agent; -}; diff --git a/src/util/http_downloader_winhttp.cpp b/src/util/http_downloader_winhttp.cpp index e2518c330..e0b7fdc58 100644 --- a/src/util/http_downloader_winhttp.cpp +++ b/src/util/http_downloader_winhttp.cpp @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: 2019-2024 Connor McLaughlin // SPDX-License-Identifier: CC-BY-NC-ND-4.0 -#include "http_downloader_winhttp.h" +#include "http_downloader.h" #include "common/assert.h" #include "common/log.h" @@ -10,6 +10,41 @@ #include +#include "common/windows_headers.h" + +#include + +namespace { +class HTTPDownloaderWinHttp final : public HTTPDownloader +{ +public: + HTTPDownloaderWinHttp(); + ~HTTPDownloaderWinHttp() override; + + bool Initialize(std::string user_agent, Error* error); + +protected: + Request* InternalCreateRequest() override; + void InternalPollRequests() override; + bool StartRequest(HTTPDownloader::Request* request) override; + void CloseRequest(HTTPDownloader::Request* request) override; + +private: + struct Request : HTTPDownloader::Request + { + std::wstring object_name; + HINTERNET hConnection = NULL; + HINTERNET hRequest = NULL; + u32 io_position = 0; + }; + + static void CALLBACK HTTPStatusCallback(HINTERNET hInternet, DWORD_PTR dwContext, DWORD dwInternetStatus, + LPVOID lpvStatusInformation, DWORD dwStatusInformationLength); + + HINTERNET m_hSession = NULL; +}; +} // namespace + LOG_CHANNEL(HTTPDownloader); HTTPDownloaderWinHttp::HTTPDownloaderWinHttp() : HTTPDownloader() @@ -25,16 +60,16 @@ HTTPDownloaderWinHttp::~HTTPDownloaderWinHttp() } } -std::unique_ptr HTTPDownloader::Create(std::string user_agent) +std::unique_ptr HTTPDownloader::Create(std::string user_agent, Error* error) { std::unique_ptr instance(std::make_unique()); - if (!instance->Initialize(std::move(user_agent))) - return {}; + if (!instance->Initialize(std::move(user_agent), error)) + instance.reset(); return instance; } -bool HTTPDownloaderWinHttp::Initialize(std::string user_agent) +bool HTTPDownloaderWinHttp::Initialize(std::string user_agent, Error* error) { static constexpr DWORD dwAccessType = WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY; @@ -42,7 +77,7 @@ bool HTTPDownloaderWinHttp::Initialize(std::string user_agent) WINHTTP_FLAG_ASYNC); if (m_hSession == NULL) { - ERROR_LOG("WinHttpOpen() failed: {}", GetLastError()); + Error::SetWin32(error, "WinHttpOpen() failed: ", GetLastError()); return false; } @@ -51,7 +86,7 @@ bool HTTPDownloaderWinHttp::Initialize(std::string user_agent) if (WinHttpSetStatusCallback(m_hSession, HTTPStatusCallback, notification_flags, NULL) == WINHTTP_INVALID_STATUS_CALLBACK) { - ERROR_LOG("WinHttpSetStatusCallback() failed: {}", GetLastError()); + Error::SetWin32(error, "WinHttpSetStatusCallback() failed: ", GetLastError()); return false; } @@ -91,6 +126,7 @@ void CALLBACK HTTPDownloaderWinHttp::HTTPStatusCallback(HINTERNET hRequest, DWOR const WINHTTP_ASYNC_RESULT* res = reinterpret_cast(lpvStatusInformation); ERROR_LOG("WinHttp async function {} returned error {}", res->dwResult, res->dwError); req->status_code = HTTP_STATUS_ERROR; + req->error.SetStringFmt("WinHttp async function {} returned error {}", res->dwResult, res->dwError); req->state.store(Request::State::Complete); return; } @@ -99,8 +135,10 @@ void CALLBACK HTTPDownloaderWinHttp::HTTPStatusCallback(HINTERNET hRequest, DWOR DEV_LOG("SendRequest complete"); if (!WinHttpReceiveResponse(hRequest, nullptr)) { - ERROR_LOG("WinHttpReceiveResponse() failed: {}", GetLastError()); + const DWORD err = GetLastError(); + ERROR_LOG("WinHttpReceiveResponse() failed: {}", err); req->status_code = HTTP_STATUS_ERROR; + req->error.SetWin32("WinHttpReceiveResponse() failed: ", err); req->state.store(Request::State::Complete); } @@ -114,8 +152,10 @@ void CALLBACK HTTPDownloaderWinHttp::HTTPStatusCallback(HINTERNET hRequest, DWOR if (!WinHttpQueryHeaders(hRequest, WINHTTP_QUERY_STATUS_CODE | WINHTTP_QUERY_FLAG_NUMBER, WINHTTP_HEADER_NAME_BY_INDEX, &req->status_code, &buffer_size, WINHTTP_NO_HEADER_INDEX)) { - ERROR_LOG("WinHttpQueryHeaders() for status code failed: {}", GetLastError()); + const DWORD err = GetLastError(); + ERROR_LOG("WinHttpQueryHeaders() for status code failed: {}", err); req->status_code = HTTP_STATUS_ERROR; + req->error.SetWin32("WinHttpQueryHeaders() failed: ", err); req->state.store(Request::State::Complete); return; } @@ -125,8 +165,9 @@ void CALLBACK HTTPDownloaderWinHttp::HTTPStatusCallback(HINTERNET hRequest, DWOR WINHTTP_HEADER_NAME_BY_INDEX, &req->content_length, &buffer_size, WINHTTP_NO_HEADER_INDEX)) { - if (GetLastError() != ERROR_WINHTTP_HEADER_NOT_FOUND) - WARNING_LOG("WinHttpQueryHeaders() for content length failed: {}", GetLastError()); + const DWORD err = GetLastError(); + if (err != ERROR_WINHTTP_HEADER_NOT_FOUND) + WARNING_LOG("WinHttpQueryHeaders() for content length failed: {}", err); req->content_length = 0; } @@ -152,8 +193,10 @@ void CALLBACK HTTPDownloaderWinHttp::HTTPStatusCallback(HINTERNET hRequest, DWOR // start reading if (!WinHttpQueryDataAvailable(hRequest, nullptr) && GetLastError() != ERROR_IO_PENDING) { - ERROR_LOG("WinHttpQueryDataAvailable() failed: {}", GetLastError()); + const DWORD err = GetLastError(); + ERROR_LOG("WinHttpQueryDataAvailable() failed: {}", err); req->status_code = HTTP_STATUS_ERROR; + req->error.SetWin32("WinHttpQueryDataAvailable() failed: ", err); req->state.store(Request::State::Complete); } @@ -178,8 +221,10 @@ void CALLBACK HTTPDownloaderWinHttp::HTTPStatusCallback(HINTERNET hRequest, DWOR if (!WinHttpReadData(hRequest, req->data.data() + req->io_position, bytes_available, nullptr) && GetLastError() != ERROR_IO_PENDING) { - ERROR_LOG("WinHttpReadData() failed: {}", GetLastError()); + const DWORD err = GetLastError(); + ERROR_LOG("WinHttpReadData() failed: {}", err); req->status_code = HTTP_STATUS_ERROR; + req->error.SetWin32("WinHttpReadData() failed: ", err); req->state.store(Request::State::Complete); } @@ -196,8 +241,10 @@ void CALLBACK HTTPDownloaderWinHttp::HTTPStatusCallback(HINTERNET hRequest, DWOR if (!WinHttpQueryDataAvailable(hRequest, nullptr) && GetLastError() != ERROR_IO_PENDING) { - ERROR_LOG("WinHttpQueryDataAvailable() failed: {}", GetLastError()); + const DWORD err = GetLastError(); + ERROR_LOG("WinHttpQueryDataAvailable() failed: {}", err); req->status_code = HTTP_STATUS_ERROR; + req->error.SetWin32("WinHttpQueryDataAvailable() failed: ", err); req->state.store(Request::State::Complete); } @@ -238,8 +285,10 @@ bool HTTPDownloaderWinHttp::StartRequest(HTTPDownloader::Request* request) const std::wstring url_wide(StringUtil::UTF8StringToWideString(req->url)); if (!WinHttpCrackUrl(url_wide.c_str(), static_cast(url_wide.size()), 0, &uc)) { - ERROR_LOG("WinHttpCrackUrl() failed: {}", GetLastError()); - req->callback(HTTP_STATUS_ERROR, std::string(), req->data); + const DWORD err = GetLastError(); + ERROR_LOG("WinHttpCrackUrl() failed: {}", err); + req->error.SetWin32("WinHttpCrackUrl() failed: ", err); + req->callback(HTTP_STATUS_ERROR, req->error, std::string(), req->data); delete req; return false; } @@ -250,8 +299,10 @@ bool HTTPDownloaderWinHttp::StartRequest(HTTPDownloader::Request* request) req->hConnection = WinHttpConnect(m_hSession, host_name.c_str(), uc.nPort, 0); if (!req->hConnection) { - ERROR_LOG("Failed to start HTTP request for '{}': {}", req->url, GetLastError()); - req->callback(HTTP_STATUS_ERROR, std::string(), req->data); + 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, std::string(), req->data); delete req; return false; } @@ -262,7 +313,9 @@ bool HTTPDownloaderWinHttp::StartRequest(HTTPDownloader::Request* request) req->object_name.c_str(), NULL, NULL, NULL, request_flags); if (!req->hRequest) { - ERROR_LOG("WinHttpOpenRequest() failed: {}", GetLastError()); + const DWORD err = GetLastError(); + ERROR_LOG("WinHttpOpenRequest() failed: {}", err); + req->error.SetWin32("WinHttpSendRequest() failed: ", err); WinHttpCloseHandle(req->hConnection); return false; } @@ -283,8 +336,10 @@ bool HTTPDownloaderWinHttp::StartRequest(HTTPDownloader::Request* request) if (!result && GetLastError() != ERROR_IO_PENDING) { - ERROR_LOG("WinHttpSendRequest() failed: {}", GetLastError()); + const DWORD err = GetLastError(); + ERROR_LOG("WinHttpSendRequest() failed: {}", err); req->status_code = HTTP_STATUS_ERROR; + req->error.SetWin32("WinHttpSendRequest() failed: ", err); req->state.store(Request::State::Complete); } diff --git a/src/util/http_downloader_winhttp.h b/src/util/http_downloader_winhttp.h deleted file mode 100644 index 8540020fa..000000000 --- a/src/util/http_downloader_winhttp.h +++ /dev/null @@ -1,38 +0,0 @@ -// SPDX-FileCopyrightText: 2019-2024 Connor McLaughlin -// SPDX-License-Identifier: CC-BY-NC-ND-4.0 - -#pragma once -#include "http_downloader.h" - -#include "common/windows_headers.h" - -#include - -class HTTPDownloaderWinHttp final : public HTTPDownloader -{ -public: - HTTPDownloaderWinHttp(); - ~HTTPDownloaderWinHttp() override; - - bool Initialize(std::string user_agent); - -protected: - Request* InternalCreateRequest() override; - void InternalPollRequests() override; - bool StartRequest(HTTPDownloader::Request* request) override; - void CloseRequest(HTTPDownloader::Request* request) override; - -private: - struct Request : HTTPDownloader::Request - { - std::wstring object_name; - HINTERNET hConnection = NULL; - HINTERNET hRequest = NULL; - u32 io_position = 0; - }; - - static void CALLBACK HTTPStatusCallback(HINTERNET hInternet, DWORD_PTR dwContext, DWORD dwInternetStatus, - LPVOID lpvStatusInformation, DWORD dwStatusInformationLength); - - HINTERNET m_hSession = NULL; -}; diff --git a/src/util/util.vcxproj b/src/util/util.vcxproj index e6b414fbd..bd53ddbeb 100644 --- a/src/util/util.vcxproj +++ b/src/util/util.vcxproj @@ -28,10 +28,6 @@ - - true - - diff --git a/src/util/util.vcxproj.filters b/src/util/util.vcxproj.filters index 10c0f48f2..60392b279 100644 --- a/src/util/util.vcxproj.filters +++ b/src/util/util.vcxproj.filters @@ -57,8 +57,6 @@ - -