From 2f272b37ca5e7d4dc035190d4aaa0681cc5425ad Mon Sep 17 00:00:00 2001 From: Mm2PL Date: Sat, 1 Jul 2023 11:03:16 +0000 Subject: [PATCH 1/3] Allow for customizing the behavior of `Right Click`ing of usernames. (#4622) Co-authored-by: Rasmus Karlsson --- CHANGELOG.md | 1 + src/singletons/Settings.hpp | 20 ++++++ src/widgets/helper/ChannelView.cpp | 81 ++++++++++++++++++----- src/widgets/settingspages/GeneralPage.cpp | 68 +++++++++++++++++++ 4 files changed, 155 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87c3da49264..66b8281b48e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ - Minor: Added a Send button in the input box so you can click to send a message. This is disabled by default and can be enabled with the "Show send message button" setting. (#4607) - Minor: Improved error messages when the updater fails a download. (#4594) - Minor: Added `/shield` and `/shieldoff` commands to toggle shield mode. (#4580) +- Minor: Allow for customizing the behavior of `Right Click`ing of usernames. (#4622) - Bugfix: Fixed the menu warping on macOS on Qt6. (#4595) - Bugfix: Fixed link tooltips not showing unless the thumbnail setting was enabled. (#4597) - Bugfix: Domains starting with `http` are now parsed as links again. (#4598) diff --git a/src/singletons/Settings.hpp b/src/singletons/Settings.hpp index 9440f8a9707..3ef633f1e12 100644 --- a/src/singletons/Settings.hpp +++ b/src/singletons/Settings.hpp @@ -83,6 +83,12 @@ enum ThumbnailPreviewMode : int { ShowOnShift = 2, }; +enum UsernameRightClickBehavior : int { + Reply = 0, + Mention = 1, + Ignore = 2, +}; + /// Settings which are availlable for reading and writing on the gui thread. // These settings are still accessed concurrently in the code but it is bad practice. class Settings : public ABSettings, public ConcurrentSettings @@ -194,6 +200,20 @@ class Settings : public ABSettings, public ConcurrentSettings BoolSetting autoCloseUserPopup = {"/behaviour/autoCloseUserPopup", true}; BoolSetting autoCloseThreadPopup = {"/behaviour/autoCloseThreadPopup", false}; + + EnumSetting usernameRightClickBehavior = { + "/behaviour/usernameRightClickBehavior", + UsernameRightClickBehavior::Mention, + }; + EnumSetting usernameRightClickModifierBehavior = + { + "/behaviour/usernameRightClickBehaviorWithModifier", + UsernameRightClickBehavior::Reply, + }; + EnumSetting usernameRightClickModifier = { + "/behaviour/usernameRightClickModifier", + Qt::KeyboardModifier::ShiftModifier}; + BoolSetting autoSubToParticipatedThreads = { "/behaviour/autoSubToParticipatedThreads", true, diff --git a/src/widgets/helper/ChannelView.cpp b/src/widgets/helper/ChannelView.cpp index 6676c273451..94e372f6529 100644 --- a/src/widgets/helper/ChannelView.cpp +++ b/src/widgets/helper/ChannelView.cpp @@ -2084,22 +2084,73 @@ void ChannelView::handleMouseClick(QMouseEvent *event, if (link.type == Link::UserInfo) { if (hoveredElement->getFlags().has( - MessageElementFlag::Username) && - event->modifiers() == Qt::ShiftModifier) + MessageElementFlag::Username)) { - // Start a new reply if Shift+Right-clicking the message username - this->setInputReply(layout->getMessagePtr()); - } - else - { - // Insert @username into split input - const bool commaMention = - getSettings()->mentionUsersWithComma; - const bool isFirstWord = - split && split->getInput().isEditFirstWord(); - auto userMention = formatUserMention( - link.value, isFirstWord, commaMention); - insertText("@" + userMention + " "); + Qt::KeyboardModifier userSpecifiedModifier = + getSettings()->usernameRightClickModifier; + + if (userSpecifiedModifier == + Qt::KeyboardModifier::NoModifier) + { + qCWarning(chatterinoCommon) + << "sanity check failed: " + "invalid settings detected " + "Settings::usernameRightClickModifier is " + "NoModifier, which should never happen"; + return; + } + + Qt::KeyboardModifiers modifiers{userSpecifiedModifier}; + auto isModifierHeld = event->modifiers() == modifiers; + + UsernameRightClickBehavior action{}; + if (isModifierHeld) + { + action = getSettings() + ->usernameRightClickModifierBehavior; + } + else + { + action = getSettings()->usernameRightClickBehavior; + } + + switch (action) + { + case UsernameRightClickBehavior::Mention: { + if (split == nullptr) + { + return; + } + + // Insert @username into split input + const bool commaMention = + getSettings()->mentionUsersWithComma; + const bool isFirstWord = + split->getInput().isEditFirstWord(); + auto userMention = formatUserMention( + link.value, isFirstWord, commaMention); + insertText("@" + userMention + " "); + } + break; + + case UsernameRightClickBehavior::Reply: { + // Start a new reply if matching user's settings + this->setInputReply(layout->getMessagePtr()); + } + break; + + case UsernameRightClickBehavior::Ignore: + break; + + default: { + qCWarning(chatterinoCommon) + << "unhandled or corrupted " + "UsernameRightClickBehavior value in " + "ChannelView::handleMouseClick:" + << action; + } + break; // unreachable + } } return; diff --git a/src/widgets/settingspages/GeneralPage.cpp b/src/widgets/settingspages/GeneralPage.cpp index 24d842d8a85..d55dc1f4d27 100644 --- a/src/widgets/settingspages/GeneralPage.cpp +++ b/src/widgets/settingspages/GeneralPage.cpp @@ -331,6 +331,74 @@ void GeneralPage::initLayout(GeneralPageView &layout) false, "Specify how Chatterino will handle messages that exceed Twitch " "message limits"); + layout.addDropdown::type>( + "Username right-click behavior", + { + "Reply", + "Mention", + "Ignore", + }, + s.usernameRightClickBehavior, + [](auto index) { + return index; + }, + [](auto args) { + return static_cast(args.index); + }, + false, + "Specify how Chatterino will handle right-clicking a username in " + "chat when not holding the modifier."); + layout.addDropdown::type>( + "Username right-click with modifier behavior", + { + "Reply", + "Mention", + "Ignore", + }, + s.usernameRightClickModifierBehavior, + [](auto index) { + return index; + }, + [](auto args) { + return static_cast(args.index); + }, + false, + "Specify how Chatterino will handle right-clicking a username in " + "chat when holding down the modifier."); + layout.addDropdown::type>( + "Modifier for alternate right-click action", + {"Shift", "Control", "Alt", META_KEY}, s.usernameRightClickModifier, + [](int index) { + switch (index) + { + case Qt::ShiftModifier: + return 0; + case Qt::ControlModifier: + return 1; + case Qt::AltModifier: + return 2; + case Qt::MetaModifier: + return 3; + default: + return 0; + } + }, + [](DropdownArgs args) { + switch (args.index) + { + case 0: + return Qt::ShiftModifier; + case 1: + return Qt::ControlModifier; + case 2: + return Qt::AltModifier; + case 3: + return Qt::MetaModifier; + default: + return Qt::NoModifier; + } + }, + false); layout.addTitle("Messages"); layout.addCheckbox( From d2f1516818387d90b8973e0b752c9012bb17fec7 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sat, 1 Jul 2023 14:01:47 +0200 Subject: [PATCH 2/3] Fix crash that could occur if closing the usercard quickly after blocking (#4711) * Specifically, this adds a caller to the network request, which makes the success or failure callback not fire. This has the unintended consequence of the block list not reloading if the usercard is closed, but it's not a big concern. * Add unrelated `-DUSE_ALTERNATE_LINKER` cmake option From https://github.com/heavyai/heavydb/blob/0517d99b467806f6af7b4c969e351368a667497d/CMakeLists.txt#L87-L103 --- CHANGELOG.md | 2 ++ CMakeLists.txt | 21 +++++++++++++++++++ mocks/include/mocks/Helix.hpp | 6 ++++-- .../commands/CommandController.cpp | 4 ++-- src/providers/twitch/TwitchAccount.cpp | 10 +++++---- src/providers/twitch/TwitchAccount.hpp | 7 +++++-- src/providers/twitch/api/Helix.cpp | 6 ++++-- src/providers/twitch/api/Helix.hpp | 9 ++++---- src/widgets/dialogs/UserInfoPopup.cpp | 5 +++-- 9 files changed, 52 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66b8281b48e..d5b6f3b4e14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Bugfix: Fix visual glitches with smooth scrolling. (#4501) - Bugfix: Fixed pings firing for the "Your username" highlight when not signed in. (#4698) - Bugfix: Fixed partially broken filters on Qt 6 builds. (#4702) +- Bugfix: Fixed crash that could occurr when closing the usercard too quickly after blocking or unblocking a user. (#4711) - Dev: Added command to set Qt's logging filter/rules at runtime (`/c2-set-logging-rules`). (#4637) - Dev: Added the ability to see & load custom themes from the Themes directory. No stable promises are made of this feature, changes might be made that breaks custom themes without notice. (#4570) - Dev: Added test cases for emote and tab completion. (#4644) @@ -30,6 +31,7 @@ - Dev: Added `sccache` in Windows CI. (#4678) - Dev: Moved preprocessor Git and date definitions to executables only. (#4681) - Dev: Refactored tests to be able to use `ctest` and run in debug builds. (#4700) +- Dev: Added the ability to use an alternate linker using the `-DUSE_ALTERNATE_LINKER=...` CMake parameter. (#4711) ## 2.4.4 diff --git a/CMakeLists.txt b/CMakeLists.txt index af05f742bbb..db82c9ed021 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,6 +49,27 @@ elseif (CCACHE_PROGRAM) set(_compiler_launcher ${CCACHE_PROGRAM}) endif () + +# Alternate linker code taken from heavyai/heavydb +# https://github.com/heavyai/heavydb/blob/0517d99b467806f6af7b4c969e351368a667497d/CMakeLists.txt#L87-L103 +macro(set_alternate_linker linker) + find_program(LINKER_EXECUTABLE ld.${USE_ALTERNATE_LINKER} ${USE_ALTERNATE_LINKER}) + if(LINKER_EXECUTABLE) + if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" AND "${CMAKE_CXX_COMPILER_VERSION}" VERSION_LESS 12.0.0) + add_link_options("-ld-path=${USE_ALTERNATE_LINKER}") + else() + add_link_options("-fuse-ld=${USE_ALTERNATE_LINKER}") + endif() + else() + set(USE_ALTERNATE_LINKER "" CACHE STRING "Use alternate linker" FORCE) + endif() +endmacro() + +set(USE_ALTERNATE_LINKER "" CACHE STRING "Use alternate linker. Leave empty for system default; alternatives are 'gold', 'lld', 'bfd', 'mold'") +if(NOT "${USE_ALTERNATE_LINKER}" STREQUAL "") + set_alternate_linker(${USE_ALTERNATE_LINKER}) +endif() + if (_compiler_launcher) set(CMAKE_CXX_COMPILER_LAUNCHER "${_compiler_launcher}" CACHE STRING "CXX compiler launcher") message(STATUS "Using ${_compiler_launcher} for speeding up build") diff --git a/mocks/include/mocks/Helix.hpp b/mocks/include/mocks/Helix.hpp index e7fe4ef4980..45b1017ee0a 100644 --- a/mocks/include/mocks/Helix.hpp +++ b/mocks/include/mocks/Helix.hpp @@ -105,12 +105,14 @@ class Helix : public IHelix (override)); MOCK_METHOD(void, blockUser, - (QString targetUserId, std::function successCallback, + (QString targetUserId, const QObject *caller, + std::function successCallback, HelixFailureCallback failureCallback), (override)); MOCK_METHOD(void, unblockUser, - (QString targetUserId, std::function successCallback, + (QString targetUserId, const QObject *caller, + std::function successCallback, HelixFailureCallback failureCallback), (override)); diff --git a/src/controllers/commands/CommandController.cpp b/src/controllers/commands/CommandController.cpp index 3956ec9c873..bd282224c5c 100644 --- a/src/controllers/commands/CommandController.cpp +++ b/src/controllers/commands/CommandController.cpp @@ -650,7 +650,7 @@ void CommandController::initialize(Settings &, Paths &paths) target, [currentUser, channel, target](const HelixUser &targetUser) { getApp()->accounts->twitch.getCurrent()->blockUser( - targetUser.id, + targetUser.id, nullptr, [channel, target, targetUser] { channel->addMessage(makeSystemMessage( QString("You successfully blocked user %1") @@ -703,7 +703,7 @@ void CommandController::initialize(Settings &, Paths &paths) target, [currentUser, channel, target](const auto &targetUser) { getApp()->accounts->twitch.getCurrent()->unblockUser( - targetUser.id, + targetUser.id, nullptr, [channel, target, targetUser] { channel->addMessage(makeSystemMessage( QString("You successfully unblocked user %1") diff --git a/src/providers/twitch/TwitchAccount.cpp b/src/providers/twitch/TwitchAccount.cpp index 789ed8339f0..0830fa15b90 100644 --- a/src/providers/twitch/TwitchAccount.cpp +++ b/src/providers/twitch/TwitchAccount.cpp @@ -121,11 +121,12 @@ void TwitchAccount::loadBlocks() }); } -void TwitchAccount::blockUser(QString userId, std::function onSuccess, +void TwitchAccount::blockUser(QString userId, const QObject *caller, + std::function onSuccess, std::function onFailure) { getHelix()->blockUser( - userId, + userId, caller, [this, userId, onSuccess] { TwitchUser blockedUser; blockedUser.id = userId; @@ -141,11 +142,12 @@ void TwitchAccount::blockUser(QString userId, std::function onSuccess, std::move(onFailure)); } -void TwitchAccount::unblockUser(QString userId, std::function onSuccess, +void TwitchAccount::unblockUser(QString userId, const QObject *caller, + std::function onSuccess, std::function onFailure) { getHelix()->unblockUser( - userId, + userId, caller, [this, userId, onSuccess] { TwitchUser ignoredUser; ignoredUser.id = userId; diff --git a/src/providers/twitch/TwitchAccount.hpp b/src/providers/twitch/TwitchAccount.hpp index 7e4b69d2ebe..154d307f53f 100644 --- a/src/providers/twitch/TwitchAccount.hpp +++ b/src/providers/twitch/TwitchAccount.hpp @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -71,9 +72,11 @@ class TwitchAccount : public Account bool isAnon() const; void loadBlocks(); - void blockUser(QString userId, std::function onSuccess, + void blockUser(QString userId, const QObject *caller, + std::function onSuccess, std::function onFailure); - void unblockUser(QString userId, std::function onSuccess, + void unblockUser(QString userId, const QObject *caller, + std::function onSuccess, std::function onFailure); SharedAccessGuard> accessBlockedUserIds() const; diff --git a/src/providers/twitch/api/Helix.cpp b/src/providers/twitch/api/Helix.cpp index 59329ae599b..b22e20669a0 100644 --- a/src/providers/twitch/api/Helix.cpp +++ b/src/providers/twitch/api/Helix.cpp @@ -541,7 +541,7 @@ void Helix::loadBlocks(QString userId, .execute(); } -void Helix::blockUser(QString targetUserId, +void Helix::blockUser(QString targetUserId, const QObject *caller, std::function successCallback, HelixFailureCallback failureCallback) { @@ -549,6 +549,7 @@ void Helix::blockUser(QString targetUserId, urlQuery.addQueryItem("target_user_id", targetUserId); this->makePut("users/blocks", urlQuery) + .caller(caller) .onSuccess([successCallback](auto /*result*/) -> Outcome { successCallback(); return Success; @@ -560,7 +561,7 @@ void Helix::blockUser(QString targetUserId, .execute(); } -void Helix::unblockUser(QString targetUserId, +void Helix::unblockUser(QString targetUserId, const QObject *caller, std::function successCallback, HelixFailureCallback failureCallback) { @@ -568,6 +569,7 @@ void Helix::unblockUser(QString targetUserId, urlQuery.addQueryItem("target_user_id", targetUserId); this->makeDelete("users/blocks", urlQuery) + .caller(caller) .onSuccess([successCallback](auto /*result*/) -> Outcome { successCallback(); return Success; diff --git a/src/providers/twitch/api/Helix.hpp b/src/providers/twitch/api/Helix.hpp index 3a370a41e31..79ff59d6c17 100644 --- a/src/providers/twitch/api/Helix.hpp +++ b/src/providers/twitch/api/Helix.hpp @@ -805,12 +805,12 @@ class IHelix HelixFailureCallback failureCallback) = 0; // https://dev.twitch.tv/docs/api/reference#block-user - virtual void blockUser(QString targetUserId, + virtual void blockUser(QString targetUserId, const QObject *caller, std::function successCallback, HelixFailureCallback failureCallback) = 0; // https://dev.twitch.tv/docs/api/reference#unblock-user - virtual void unblockUser(QString targetUserId, + virtual void unblockUser(QString targetUserId, const QObject *caller, std::function successCallback, HelixFailureCallback failureCallback) = 0; @@ -1118,11 +1118,12 @@ class Helix final : public IHelix HelixFailureCallback failureCallback) final; // https://dev.twitch.tv/docs/api/reference#block-user - void blockUser(QString targetUserId, std::function successCallback, + void blockUser(QString targetUserId, const QObject *caller, + std::function successCallback, HelixFailureCallback failureCallback) final; // https://dev.twitch.tv/docs/api/reference#unblock-user - void unblockUser(QString targetUserId, + void unblockUser(QString targetUserId, const QObject *caller, std::function successCallback, HelixFailureCallback failureCallback) final; diff --git a/src/widgets/dialogs/UserInfoPopup.cpp b/src/widgets/dialogs/UserInfoPopup.cpp index e6978100222..e5955884281 100644 --- a/src/widgets/dialogs/UserInfoPopup.cpp +++ b/src/widgets/dialogs/UserInfoPopup.cpp @@ -36,6 +36,7 @@ #include #include #include +#include const QString TEXT_FOLLOWERS("Followers: %1"); const QString TEXT_CREATED("Created: %1"); @@ -593,7 +594,7 @@ void UserInfoPopup::installEvents() this->ui_.block->setEnabled(false); getApp()->accounts->twitch.getCurrent()->unblockUser( - this->userId_, + this->userId_, this, [this, reenableBlockCheckbox, currentUser] { this->channel_->addMessage(makeSystemMessage( QString("You successfully unblocked user %1") @@ -620,7 +621,7 @@ void UserInfoPopup::installEvents() this->ui_.block->setEnabled(false); getApp()->accounts->twitch.getCurrent()->blockUser( - this->userId_, + this->userId_, this, [this, reenableBlockCheckbox, currentUser] { this->channel_->addMessage(makeSystemMessage( QString("You successfully blocked user %1") From 22b290cb2d1832fdff54e0f060e4e249ed063fcb Mon Sep 17 00:00:00 2001 From: nerix Date: Sat, 1 Jul 2023 14:59:59 +0200 Subject: [PATCH 3/3] Improve network error messages (#4704) --- CHANGELOG.md | 1 + src/common/NetworkPrivate.cpp | 13 +- src/common/NetworkResult.cpp | 28 +- src/common/NetworkResult.hpp | 30 ++- src/providers/IvrApi.cpp | 4 +- src/providers/RecentMessagesApi.cpp | 9 +- src/providers/bttv/BttvEmotes.cpp | 20 +- src/providers/ffz/FfzEmotes.cpp | 21 +- src/providers/seventv/SeventvEmotes.cpp | 29 +-- src/providers/twitch/api/Helix.cpp | 325 +++++++++++++++++------- src/singletons/Updates.cpp | 8 +- src/util/NuulsUploader.cpp | 2 +- tests/CMakeLists.txt | 1 + tests/src/NetworkRequest.cpp | 8 +- tests/src/NetworkResult.cpp | 48 ++++ 15 files changed, 378 insertions(+), 169 deletions(-) create mode 100644 tests/src/NetworkResult.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index d5b6f3b4e14..0e2c14ccb93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Bugfix: Fix visual glitches with smooth scrolling. (#4501) - Bugfix: Fixed pings firing for the "Your username" highlight when not signed in. (#4698) - Bugfix: Fixed partially broken filters on Qt 6 builds. (#4702) +- Bugfix: Fixed some network errors having `0` as their HTTP status. (#4704) - Bugfix: Fixed crash that could occurr when closing the usercard too quickly after blocking or unblocking a user. (#4711) - Dev: Added command to set Qt's logging filter/rules at runtime (`/c2-set-logging-rules`). (#4637) - Dev: Added the ability to see & load custom themes from the Themes directory. No stable promises are made of this feature, changes might be made that breaks custom themes without notice. (#4570) diff --git a/src/common/NetworkPrivate.cpp b/src/common/NetworkPrivate.cpp index 9af3018837b..661b2eccf47 100644 --- a/src/common/NetworkPrivate.cpp +++ b/src/common/NetworkPrivate.cpp @@ -155,7 +155,8 @@ void loadUncached(std::shared_ptr &&data) { postToThread([data] { data->onError_(NetworkResult( - {}, NetworkResult::timedoutStatus)); + NetworkResult::NetworkError::TimeoutError, {}, + {})); }); } @@ -218,8 +219,9 @@ void loadUncached(std::shared_ptr &&data) QString(data->payload_)); } // TODO: Should this always be run on the GUI thread? - postToThread([data, code = status.toInt(), reply] { - data->onError_(NetworkResult(reply->readAll(), code)); + postToThread([data, status, reply] { + data->onError_(NetworkResult(reply->error(), status, + reply->readAll())); }); } @@ -238,7 +240,7 @@ void loadUncached(std::shared_ptr &&data) auto status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); - NetworkResult result(bytes, status.toInt()); + NetworkResult result(reply->error(), status, bytes); DebugCount::increase("http request success"); // log("starting {}", data->request_.url().toString()); @@ -337,7 +339,8 @@ void loadCached(std::shared_ptr &&data) // XXX: check if bytes is empty? QByteArray bytes = cachedFile.readAll(); - NetworkResult result(bytes, 200); + NetworkResult result(NetworkResult::NetworkError::NoError, QVariant(200), + bytes); qCDebug(chatterinoHTTP) << QString("%1 [CACHED] 200 %2") diff --git a/src/common/NetworkResult.cpp b/src/common/NetworkResult.cpp index 0a5295fdbdd..ae614bac0fe 100644 --- a/src/common/NetworkResult.cpp +++ b/src/common/NetworkResult.cpp @@ -3,15 +3,21 @@ #include "common/QLogging.hpp" #include +#include #include #include namespace chatterino { -NetworkResult::NetworkResult(const QByteArray &data, int status) - : data_(data) - , status_(status) +NetworkResult::NetworkResult(NetworkError error, const QVariant &httpStatusCode, + QByteArray data) + : data_(std::move(data)) + , error_(error) { + if (httpStatusCode.isValid()) + { + this->status_ = httpStatusCode.toInt(); + } } QJsonObject NetworkResult::parseJson() const @@ -59,9 +65,21 @@ const QByteArray &NetworkResult::getData() const return this->data_; } -int NetworkResult::status() const +QString NetworkResult::formatError() const { - return this->status_; + if (this->status_) + { + return QString::number(*this->status_); + } + + const auto *name = + QMetaEnum::fromType().valueToKey( + this->error_); + if (name == nullptr) + { + return QStringLiteral("unknown error (%1)").arg(this->error_); + } + return name; } } // namespace chatterino diff --git a/src/common/NetworkResult.hpp b/src/common/NetworkResult.hpp index 64baed5ea8d..9f0ada784cf 100644 --- a/src/common/NetworkResult.hpp +++ b/src/common/NetworkResult.hpp @@ -2,14 +2,20 @@ #include #include +#include #include +#include + namespace chatterino { class NetworkResult { public: - NetworkResult(const QByteArray &data, int status); + using NetworkError = QNetworkReply::NetworkError; + + NetworkResult(NetworkError error, const QVariant &httpStatusCode, + QByteArray data); /// Parses the result as json and returns the root as an object. /// Returns empty object if parsing failed. @@ -20,13 +26,29 @@ class NetworkResult /// Parses the result as json and returns the document. rapidjson::Document parseRapidJson() const; const QByteArray &getData() const; - int status() const; - static constexpr int timedoutStatus = -2; + /// The error code of the reply. + /// In case of a successful reply, this will be NoError (0) + NetworkError error() const + { + return this->error_; + } + + /// The HTTP status code if a response was received. + std::optional status() const + { + return this->status_; + } + + /// Formats the error. + /// If a reply is received, returns the HTTP status otherwise, the network error. + QString formatError() const; private: QByteArray data_; - int status_; + + NetworkError error_; + std::optional status_; }; } // namespace chatterino diff --git a/src/providers/IvrApi.cpp b/src/providers/IvrApi.cpp index 26b4088e5eb..868a9ff082b 100644 --- a/src/providers/IvrApi.cpp +++ b/src/providers/IvrApi.cpp @@ -27,7 +27,7 @@ void IvrApi::getSubage(QString userName, QString channelName, }) .onError([failureCallback](auto result) { qCWarning(chatterinoIvr) - << "Failed IVR API Call!" << result.status() + << "Failed IVR API Call!" << result.formatError() << QString(result.getData()); failureCallback(); }) @@ -51,7 +51,7 @@ void IvrApi::getBulkEmoteSets(QString emoteSetList, }) .onError([failureCallback](auto result) { qCWarning(chatterinoIvr) - << "Failed IVR API Call!" << result.status() + << "Failed IVR API Call!" << result.formatError() << QString(result.getData()); failureCallback(); }) diff --git a/src/providers/RecentMessagesApi.cpp b/src/providers/RecentMessagesApi.cpp index 9fd524f684d..05ee6e8c8c5 100644 --- a/src/providers/RecentMessagesApi.cpp +++ b/src/providers/RecentMessagesApi.cpp @@ -217,17 +217,20 @@ void RecentMessagesApi::loadRecentMessages(const QString &channelName, return Success; }) - .onError([channelPtr, onError](NetworkResult result) { + .onError([channelPtr, onError](const NetworkResult &result) { auto shared = channelPtr.lock(); if (!shared) + { return; + } qCDebug(chatterinoRecentMessages) << "Failed to load recent messages for" << shared->getName(); shared->addMessage(makeSystemMessage( - QString("Message history service unavailable (Error %1)") - .arg(result.status()))); + QStringLiteral( + "Message history service unavailable (Error: %1)") + .arg(result.formatError()))); onError(); }) diff --git a/src/providers/bttv/BttvEmotes.cpp b/src/providers/bttv/BttvEmotes.cpp index b1b6fb2e772..42bf51cba0c 100644 --- a/src/providers/bttv/BttvEmotes.cpp +++ b/src/providers/bttv/BttvEmotes.cpp @@ -259,23 +259,17 @@ void BttvEmotes::loadChannel(std::weak_ptr channel, shared->addMessage( makeSystemMessage(CHANNEL_HAS_NO_EMOTES)); } - else if (result.status() == NetworkResult::timedoutStatus) - { - // TODO: Auto retry in case of a timeout, with a delay - qCWarning(chatterinoBttv) - << "Fetching BTTV emotes for channel" << channelId - << "failed due to timeout"; - shared->addMessage(makeSystemMessage( - "Failed to fetch BetterTTV channel emotes. (timed out)")); - } else { + // TODO: Auto retry in case of a timeout, with a delay + auto errorString = result.formatError(); qCWarning(chatterinoBttv) << "Error fetching BTTV emotes for channel" << channelId - << ", error" << result.status(); - shared->addMessage( - makeSystemMessage("Failed to fetch BetterTTV channel " - "emotes. (unknown error)")); + << ", error" << errorString; + shared->addMessage(makeSystemMessage( + QStringLiteral("Failed to fetch BetterTTV channel " + "emotes. (Error: %1)") + .arg(errorString))); } }) .execute(); diff --git a/src/providers/ffz/FfzEmotes.cpp b/src/providers/ffz/FfzEmotes.cpp index 73be0b4dda4..0a28cc51b6e 100644 --- a/src/providers/ffz/FfzEmotes.cpp +++ b/src/providers/ffz/FfzEmotes.cpp @@ -273,24 +273,17 @@ void FfzEmotes::loadChannel( makeSystemMessage(CHANNEL_HAS_NO_EMOTES)); } } - else if (result.status() == NetworkResult::timedoutStatus) - { - // TODO: Auto retry in case of a timeout, with a delay - qCWarning(chatterinoFfzemotes) - << "Fetching FFZ emotes for channel" << channelID - << "failed due to timeout"; - shared->addMessage( - makeSystemMessage("Failed to fetch FrankerFaceZ channel " - "emotes. (timed out)")); - } else { + // TODO: Auto retry in case of a timeout, with a delay + auto errorString = result.formatError(); qCWarning(chatterinoFfzemotes) << "Error fetching FFZ emotes for channel" << channelID - << ", error" << result.status(); - shared->addMessage( - makeSystemMessage("Failed to fetch FrankerFaceZ channel " - "emotes. (unknown error)")); + << ", error" << errorString; + shared->addMessage(makeSystemMessage( + QStringLiteral("Failed to fetch FrankerFaceZ channel " + "emotes. (Error: %1)") + .arg(errorString))); } }) .execute(); diff --git a/src/providers/seventv/SeventvEmotes.cpp b/src/providers/seventv/SeventvEmotes.cpp index 321e43a6519..a384189fac3 100644 --- a/src/providers/seventv/SeventvEmotes.cpp +++ b/src/providers/seventv/SeventvEmotes.cpp @@ -386,23 +386,17 @@ void SeventvEmotes::loadChannelEmotes( makeSystemMessage(CHANNEL_HAS_NO_EMOTES)); } } - else if (result.status() == NetworkResult::timedoutStatus) - { - // TODO: Auto retry in case of a timeout, with a delay - qCWarning(chatterinoSeventv) - << "Fetching 7TV emotes for channel" << channelId - << "failed due to timeout"; - shared->addMessage(makeSystemMessage( - "Failed to fetch 7TV channel emotes. (timed out)")); - } else { + // TODO: Auto retry in case of a timeout, with a delay + auto errorString = result.formatError(); qCWarning(chatterinoSeventv) << "Error fetching 7TV emotes for channel" << channelId - << ", error" << result.status(); - shared->addMessage( - makeSystemMessage("Failed to fetch 7TV channel " - "emotes. (unknown error)")); + << ", error" << errorString; + shared->addMessage(makeSystemMessage( + QStringLiteral("Failed to fetch 7TV channel " + "emotes. (Error: %1)") + .arg(errorString))); } }) .execute(); @@ -502,14 +496,7 @@ void SeventvEmotes::getEmoteSet( }) .onError([emoteSetId, callback = std::move(errorCallback)]( const NetworkResult &result) { - if (result.status() == NetworkResult::timedoutStatus) - { - callback("timed out"); - } - else - { - callback(QString("status: %1").arg(result.status())); - } + callback(result.formatError()); }) .execute(); } diff --git a/src/providers/twitch/api/Helix.cpp b/src/providers/twitch/api/Helix.cpp index b22e20669a0..04cf34f5d08 100644 --- a/src/providers/twitch/api/Helix.cpp +++ b/src/providers/twitch/api/Helix.cpp @@ -390,7 +390,7 @@ void Helix::createClip(QString channelId, return Success; }) .onError([failureCallback](auto result) { - switch (result.status()) + switch (result.status().value_or(0)) { case 503: { // Channel has disabled clip-creation, or channel has made cliops only creatable by followers and the user is not a follower (or subscriber) @@ -406,7 +406,7 @@ void Helix::createClip(QString channelId, default: { qCDebug(chatterinoTwitch) - << "Failed to create a clip: " << result.status() + << "Failed to create a clip: " << result.formatError() << result.getData(); failureCallback(HelixClipError::Unknown); } @@ -477,7 +477,7 @@ void Helix::createStreamMarker( return Success; }) .onError([failureCallback](NetworkResult result) { - switch (result.status()) + switch (result.status().value_or(0)) { case 403: { // User isn't a Channel Editor, so he can't create markers @@ -495,7 +495,7 @@ void Helix::createStreamMarker( default: { qCDebug(chatterinoTwitch) << "Failed to create a stream marker: " - << result.status() << result.getData(); + << result.formatError() << result.getData(); failureCallback(HelixStreamMarkerError::Unknown); } break; @@ -638,7 +638,7 @@ void Helix::manageAutoModMessages( return Success; }) .onError([failureCallback, msgID, action](NetworkResult result) { - switch (result.status()) + switch (result.status().value_or(0)) { case 400: { // Message was already processed @@ -670,7 +670,7 @@ void Helix::manageAutoModMessages( default: { qCDebug(chatterinoTwitch) << "Failed to manage automod message: " << action - << msgID << result.status() << result.getData(); + << msgID << result.formatError() << result.getData(); failureCallback(HelixAutoModMessageError::Unknown); } break; @@ -712,7 +712,7 @@ void Helix::getCheermotes( .onError([broadcasterId, failureCallback](NetworkResult result) { qCDebug(chatterinoTwitch) << "Failed to get cheermotes(broadcaster_id=" << broadcasterId - << "): " << result.status() << result.getData(); + << "): " << result.formatError() << result.getData(); failureCallback(); }) .execute(); @@ -806,17 +806,24 @@ void Helix::updateUserChatColor( { qCWarning(chatterinoTwitch) << "Success result for updating chat color was" - << result.status() << "but we only expected it to be 204"; + << result.formatError() + << "but we only expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.startsWith("invalid color", @@ -849,7 +856,7 @@ void Helix::updateUserChatColor( default: { qCDebug(chatterinoTwitch) << "Unhandled error changing user color:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -882,17 +889,24 @@ void Helix::deleteChatMessages( { qCWarning(chatterinoTwitch) << "Success result for deleting chat messages was" - << result.status() << "but we only expected it to be 204"; + << result.formatError() + << "but we only expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 404: { // A 404 on this endpoint means message id is invalid or unable to be deleted. @@ -934,7 +948,7 @@ void Helix::deleteChatMessages( default: { qCDebug(chatterinoTwitch) << "Unhandled error deleting chat messages:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -960,17 +974,24 @@ void Helix::addChannelModerator( { qCWarning(chatterinoTwitch) << "Success result for adding a moderator was" - << result.status() << "but we only expected it to be 204"; + << result.formatError() + << "but we only expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 401: { if (message.startsWith("Missing scope", @@ -1022,7 +1043,7 @@ void Helix::addChannelModerator( default: { qCDebug(chatterinoTwitch) << "Unhandled error adding channel moderator:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -1048,17 +1069,24 @@ void Helix::removeChannelModerator( { qCWarning(chatterinoTwitch) << "Success result for unmodding user was" - << result.status() << "but we only expected it to be 204"; + << result.formatError() + << "but we only expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.compare("user is not a mod", @@ -1100,8 +1128,8 @@ void Helix::removeChannelModerator( default: { qCDebug(chatterinoTwitch) - << "Unhandled error unmodding user:" << result.status() - << result.getData() << obj; + << "Unhandled error unmodding user:" + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -1135,17 +1163,24 @@ void Helix::sendChatAnnouncement( { qCWarning(chatterinoTwitch) << "Success result for sending an announcement was" - << result.status() << "but we only expected it to be 204"; + << result.formatError() + << "but we only expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { // These errors are generally well formatted, so we just forward them. @@ -1178,7 +1213,7 @@ void Helix::sendChatAnnouncement( default: { qCDebug(chatterinoTwitch) << "Unhandled error sending an announcement:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -1204,17 +1239,24 @@ void Helix::addChannelVIP( { qCWarning(chatterinoTwitch) << "Success result for adding channel VIP was" - << result.status() << "but we only expected it to be 204"; + << result.formatError() + << "but we only expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: case 409: @@ -1256,7 +1298,7 @@ void Helix::addChannelVIP( default: { qCDebug(chatterinoTwitch) << "Unhandled error adding channel VIP:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -1282,17 +1324,24 @@ void Helix::removeChannelVIP( { qCWarning(chatterinoTwitch) << "Success result for removing channel VIP was" - << result.status() << "but we only expected it to be 204"; + << result.formatError() + << "but we only expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: case 409: @@ -1333,7 +1382,7 @@ void Helix::removeChannelVIP( default: { qCDebug(chatterinoTwitch) << "Unhandled error removing channel VIP:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -1371,17 +1420,24 @@ void Helix::unbanUser( { qCWarning(chatterinoTwitch) << "Success result for unbanning user was" - << result.status() << "but we only expected it to be 204"; + << result.formatError() + << "but we only expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.startsWith("The user in the user_id query " @@ -1437,8 +1493,8 @@ void Helix::unbanUser( default: { qCDebug(chatterinoTwitch) - << "Unhandled error unbanning user:" << result.status() - << result.getData() << obj; + << "Unhandled error unbanning user:" + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -1476,11 +1532,17 @@ void Helix::startRaid( successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.compare("The IDs in from_broadcaster_id and " @@ -1531,7 +1593,7 @@ void Helix::startRaid( default: { qCDebug(chatterinoTwitch) << "Unhandled error while starting a raid:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -1556,17 +1618,24 @@ void Helix::cancelRaid( { qCWarning(chatterinoTwitch) << "Success result for canceling the raid was" - << result.status() << "but we only expected it to be 204"; + << result.formatError() + << "but we only expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 401: { if (message.startsWith("Missing scope", @@ -1603,7 +1672,7 @@ void Helix::cancelRaid( default: { qCDebug(chatterinoTwitch) << "Unhandled error while canceling the raid:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -1717,18 +1786,24 @@ void Helix::updateChatSettings( { qCWarning(chatterinoTwitch) << "Success result for updating chat settings was" - << result.status() << "but we expected it to be 200"; + << result.formatError() << "but we expected it to be 200"; } auto response = result.parseJson(); successCallback(HelixChatSettings( response.value("data").toArray().first().toObject())); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.contains("must be in the range")) @@ -1775,7 +1850,7 @@ void Helix::updateChatSettings( default: { qCDebug(chatterinoTwitch) << "Unhandled error updating chat settings:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -1840,18 +1915,24 @@ void Helix::fetchChatters( { qCWarning(chatterinoTwitch) << "Success result for getting chatters was " - << result.status() << "but we expected it to be 200"; + << result.formatError() << "but we expected it to be 200"; } auto response = result.parseJson(); successCallback(HelixChatters(response)); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { failureCallback(Error::Forwarded, message); @@ -1882,7 +1963,7 @@ void Helix::fetchChatters( default: { qCDebug(chatterinoTwitch) - << "Unhandled error data:" << result.status() + << "Unhandled error data:" << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } @@ -1949,18 +2030,24 @@ void Helix::fetchModerators( { qCWarning(chatterinoTwitch) << "Success result for getting moderators was " - << result.status() << "but we expected it to be 200"; + << result.formatError() << "but we expected it to be 200"; } auto response = result.parseJson(); successCallback(HelixModerators(response)); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { failureCallback(Error::Forwarded, message); @@ -1991,7 +2078,7 @@ void Helix::fetchModerators( default: { qCDebug(chatterinoTwitch) - << "Unhandled error data:" << result.status() + << "Unhandled error data:" << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } @@ -2035,17 +2122,23 @@ void Helix::banUser(QString broadcasterID, QString moderatorID, QString userID, { qCWarning(chatterinoTwitch) << "Success result for banning a user was" - << result.status() << "but we expected it to be 200"; + << result.formatError() << "but we expected it to be 200"; } // we don't care about the response successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.startsWith("The user specified in the user_id " @@ -2099,8 +2192,8 @@ void Helix::banUser(QString broadcasterID, QString moderatorID, QString userID, default: { qCDebug(chatterinoTwitch) - << "Unhandled error banning user:" << result.status() - << result.getData() << obj; + << "Unhandled error banning user:" + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -2132,17 +2225,23 @@ void Helix::sendWhisper( { qCWarning(chatterinoTwitch) << "Success result for sending a whisper was" - << result.status() << "but we expected it to be 204"; + << result.formatError() << "but we expected it to be 204"; } // we don't care about the response successCallback(); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.startsWith("A user cannot whisper themself", @@ -2203,8 +2302,8 @@ void Helix::sendWhisper( default: { qCDebug(chatterinoTwitch) - << "Unhandled error banning user:" << result.status() - << result.getData() << obj; + << "Unhandled error banning user:" + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -2274,8 +2373,8 @@ void Helix::getChannelVIPs( if (result.status() != 200) { qCWarning(chatterinoTwitch) - << "Success result for getting VIPs was" << result.status() - << "but we expected it to be 200"; + << "Success result for getting VIPs was" + << result.formatError() << "but we expected it to be 200"; } auto response = result.parseJson(); @@ -2289,11 +2388,17 @@ void Helix::getChannelVIPs( successCallback(channelVips); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { failureCallback(Error::Forwarded, message); @@ -2333,8 +2438,8 @@ void Helix::getChannelVIPs( default: { qCDebug(chatterinoTwitch) - << "Unhandled error listing VIPs:" << result.status() - << result.getData() << obj; + << "Unhandled error listing VIPs:" + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -2370,11 +2475,17 @@ void Helix::startCommercial( successCallback(HelixStartCommercialResponse(obj)); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.startsWith("Missing scope", @@ -2429,7 +2540,7 @@ void Helix::startCommercial( default: { qCDebug(chatterinoTwitch) << "Unhandled error starting commercial:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -2452,18 +2563,24 @@ void Helix::getGlobalBadges( { qCWarning(chatterinoTwitch) << "Success result for getting global badges was " - << result.status() << "but we expected it to be 200"; + << result.formatError() << "but we expected it to be 200"; } auto response = result.parseJson(); successCallback(HelixGlobalBadges(response)); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 401: { failureCallback(Error::Forwarded, message); @@ -2473,7 +2590,7 @@ void Helix::getGlobalBadges( default: { qCWarning(chatterinoTwitch) << "Helix global badges, unhandled error data:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -2499,18 +2616,24 @@ void Helix::getChannelBadges( { qCWarning(chatterinoTwitch) << "Success result for getting badges was " - << result.status() << "but we expected it to be 200"; + << result.formatError() << "but we expected it to be 200"; } auto response = result.parseJson(); successCallback(HelixChannelBadges(response)); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + auto obj = result.parseJson(); auto message = obj.value("message").toString(); - switch (result.status()) + switch (*result.status()) { case 400: case 401: { @@ -2521,7 +2644,7 @@ void Helix::getChannelBadges( default: { qCWarning(chatterinoTwitch) << "Helix channel badges, unhandled error data:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -2552,7 +2675,7 @@ void Helix::updateShieldMode( { qCWarning(chatterinoTwitch) << "Success result for updating shield mode was " - << result.status() << "but we expected it to be 200"; + << result.formatError() << "but we expected it to be 200"; } const auto response = result.parseJson(); @@ -2560,11 +2683,17 @@ void Helix::updateShieldMode( HelixShieldModeStatus(response["data"][0].toObject())); return Success; }) - .onError([failureCallback](auto result) { + .onError([failureCallback](const auto &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + const auto obj = result.parseJson(); auto message = obj["message"].toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.startsWith("Missing scope", @@ -2590,7 +2719,7 @@ void Helix::updateShieldMode( default: { qCWarning(chatterinoTwitch) << "Helix shield mode, unhandled error data:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } break; @@ -2619,17 +2748,23 @@ void Helix::sendShoutout( { qCWarning(chatterinoTwitch) << "Success result for sending shoutout was " - << result.status() << "but we expected it to be 204"; + << result.formatError() << "but we expected it to be 204"; } successCallback(); return Success; }) - .onError([failureCallback](NetworkResult result) -> void { + .onError([failureCallback](const NetworkResult &result) -> void { + if (!result.status()) + { + failureCallback(Error::Unknown, result.formatError()); + return; + } + const auto obj = result.parseJson(); auto message = obj["message"].toString(); - switch (result.status()) + switch (*result.status()) { case 400: { if (message.startsWith("The broadcaster may not give " @@ -2692,7 +2827,7 @@ void Helix::sendShoutout( default: { qCWarning(chatterinoTwitch) << "Helix send shoutout, unhandled error data:" - << result.status() << result.getData() << obj; + << result.formatError() << result.getData() << obj; failureCallback(Error::Unknown, message); } } diff --git a/src/singletons/Updates.cpp b/src/singletons/Updates.cpp index c54d63e7b7d..b25e52819bc 100644 --- a/src/singletons/Updates.cpp +++ b/src/singletons/Updates.cpp @@ -128,8 +128,8 @@ void Updates::installUpdates() auto *box = new QMessageBox( QMessageBox::Information, "Chatterino Update", QStringLiteral("The update couldn't be downloaded " - "(HTTP status %1).") - .arg(result.status())); + "(Error: %1).") + .arg(result.formatError())); box->setAttribute(Qt::WA_DeleteOnClose); box->exec(); return Failure; @@ -189,8 +189,8 @@ void Updates::installUpdates() auto *box = new QMessageBox( QMessageBox::Information, "Chatterino Update", QStringLiteral("The update couldn't be downloaded " - "(HTTP status %1).") - .arg(result.status())); + "(Error: %1).") + .arg(result.formatError())); box->setAttribute(Qt::WA_DeleteOnClose); box->exec(); return Failure; diff --git a/src/util/NuulsUploader.cpp b/src/util/NuulsUploader.cpp index 79f7b3794e7..8627ef8fddd 100644 --- a/src/util/NuulsUploader.cpp +++ b/src/util/NuulsUploader.cpp @@ -208,7 +208,7 @@ void uploadImageToNuuls(RawImageData imageData, ChannelPtr channel, .onError([channel](NetworkResult result) -> bool { auto errorMessage = QString("An error happened while uploading your image: %1") - .arg(result.status()); + .arg(result.formatError()); // Try to read more information from the result body auto obj = result.parseJson(); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 09a9828bc2b..06c254ba9ac 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -8,6 +8,7 @@ set(test_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/AccessGuard.cpp ${CMAKE_CURRENT_LIST_DIR}/src/NetworkCommon.cpp ${CMAKE_CURRENT_LIST_DIR}/src/NetworkRequest.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/NetworkResult.cpp ${CMAKE_CURRENT_LIST_DIR}/src/ChatterSet.cpp ${CMAKE_CURRENT_LIST_DIR}/src/HighlightPhrase.cpp ${CMAKE_CURRENT_LIST_DIR}/src/Emojis.cpp diff --git a/tests/src/NetworkRequest.cpp b/tests/src/NetworkRequest.cpp index 2bcb5da9a8b..048ed1c91f7 100644 --- a/tests/src/NetworkRequest.cpp +++ b/tests/src/NetworkRequest.cpp @@ -210,7 +210,9 @@ TEST(NetworkRequest, TimeoutTimingOut) .onError([&waiter, url](const NetworkResult &result) { qDebug() << QTime::currentTime().toString() << "timeout request finish error"; - EXPECT_EQ(result.status(), NetworkResult::timedoutStatus); + EXPECT_EQ(result.error(), + NetworkResult::NetworkError::TimeoutError); + EXPECT_EQ(result.status(), std::nullopt); waiter.requestDone(); }) @@ -267,7 +269,9 @@ TEST(NetworkRequest, FinallyCallbackOnTimeout) }) .onError([&](const NetworkResult &result) { onErrorCalled = true; - EXPECT_EQ(result.status(), NetworkResult::timedoutStatus); + EXPECT_EQ(result.error(), + NetworkResult::NetworkError::TimeoutError); + EXPECT_EQ(result.status(), std::nullopt); }) .finally([&] { finallyCalled = true; diff --git a/tests/src/NetworkResult.cpp b/tests/src/NetworkResult.cpp new file mode 100644 index 00000000000..6bafc3a83bb --- /dev/null +++ b/tests/src/NetworkResult.cpp @@ -0,0 +1,48 @@ +#include "common/NetworkResult.hpp" + +#include + +using namespace chatterino; + +using Error = NetworkResult::NetworkError; + +namespace { + +void checkResult(const NetworkResult &res, Error error, + std::optional status, const QString &formatted) +{ + ASSERT_EQ(res.error(), error); + ASSERT_EQ(res.status(), status); + ASSERT_EQ(res.formatError(), formatted); +} + +} // namespace + +TEST(NetworkResult, NoError) +{ + checkResult({Error::NoError, 200, {}}, Error::NoError, 200, "200"); + checkResult({Error::NoError, 202, {}}, Error::NoError, 202, "202"); + + // no status code + checkResult({Error::NoError, {}, {}}, Error::NoError, std::nullopt, + "NoError"); +} + +TEST(NetworkResult, Errors) +{ + checkResult({Error::TimeoutError, {}, {}}, Error::TimeoutError, + std::nullopt, "TimeoutError"); + checkResult({Error::RemoteHostClosedError, {}, {}}, + Error::RemoteHostClosedError, std::nullopt, + "RemoteHostClosedError"); + + // status code takes precedence + checkResult({Error::TimeoutError, 400, {}}, Error::TimeoutError, 400, + "400"); +} + +TEST(NetworkResult, InvalidError) +{ + checkResult({static_cast(-1), {}, {}}, static_cast(-1), + std::nullopt, "unknown error (-1)"); +}