Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved error messaging for Update Channel API #5429

Merged
merged 15 commits into from
Jun 9, 2024
Merged

Improved error messaging for Update Channel API #5429

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
c60a514
Remove unused accessStreamStatus call
JakeRYW May 29, 2024
7641f87
Add error message for channel titles exceeding 140 characters
JakeRYW May 29, 2024
a0d97f0
Adding updated error messages for Update Channel API
JakeRYW Jun 3, 2024
f7e250a
Merge branch 'Chatterino:master' into feature/add-settitle-validation
JakeRYW Jun 3, 2024
ec59fff
Refactoring error message into function
JakeRYW Jun 3, 2024
c2cc573
Updating UpdateChannel Mock
JakeRYW Jun 3, 2024
879a633
Removing 409 status since it's a typo from Twitch Docs
JakeRYW Jun 3, 2024
cda1c66
Update formatting
JakeRYW Jun 3, 2024
e6d9b96
Adding changelog entry
JakeRYW Jun 3, 2024
4e5d031
Merge branch 'master' into feature/add-settitle-validation
JakeRYW Jun 7, 2024
5ed0c9e
fix: formatting
pajlada Jun 8, 2024
6997f26
fix: updateChannel mock missing a right-paren
pajlada Jun 8, 2024
f258c00
fix: missing break after case 500 in Helix::updateChannel
pajlada Jun 8, 2024
6f10bac
nit: update HelixUpdateChannelError comments
pajlada Jun 8, 2024
a02c0f0
Merge branch 'master' into feature/add-settitle-validation
pajlada Jun 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 81 additions & 9 deletions src/controllers/commands/builtin/twitch/UpdateChannel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ QString setTitle(const CommandContext &ctx)
return "";
}

auto status = ctx.twitchChannel->accessStreamStatus();
auto title = ctx.words.mid(1).join(" ");

getHelix()->updateChannel(
ctx.twitchChannel->roomId(), "", "", title,
[channel{ctx.channel}, title](const auto &result) {
Expand All @@ -40,10 +40,46 @@ QString setTitle(const CommandContext &ctx)
channel->addMessage(
makeSystemMessage(QString("Updated title to %1").arg(title)));
},
[channel{ctx.channel}] {
channel->addMessage(
makeSystemMessage("Title update failed! Are you "
"missing the required scope?"));
[channel{ctx.channel}](auto error, auto message) {
QString errorMessage = QString("Failed to set title - ");

using Error = HelixUpdateChannelError;

switch(error)
{
case Error::UserMissingScope: {
errorMessage += "Missing required scope. "
"Re-login with your "
"account and try again.";
}
break;

case Error::UserNotAuthorized: {
errorMessage += "You must be the broadcaster "
"to set the title.";
}
break;

case Error::Ratelimited: {
errorMessage +=
"You are being ratelimited by Twitch. Try "
"again in a few seconds.";
}
break;

case Error::Forwarded: {
errorMessage += message;
}
break;

case Error::Unknown:
default: {
errorMessage += "An unknown error has occured.";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should include the message - like "Failed to set title - An unknown error has occurred ({message})" (there's also a typo in occured → occurred, which is also present in the error handling of manageAutoModMessages and the comment for secondsSinceDisconnect).

}
break;
}

channel->addMessage(makeSystemMessage(errorMessage));
});

return "";
Expand Down Expand Up @@ -105,10 +141,46 @@ QString setGame(const CommandContext &ctx)
channel->addMessage(makeSystemMessage(
QString("Updated game to %1").arg(matchedGame.name)));
},
[channel] {
channel->addMessage(
makeSystemMessage("Game update failed! Are you "
"missing the required scope?"));
[channel](auto error, auto message) {
QString errorMessage = QString("Failed to set game - ");

using Error = HelixUpdateChannelError;

switch(error)
{
case Error::UserMissingScope: {
errorMessage += "Missing required scope. "
"Re-login with your "
"account and try again.";
}
break;

case Error::UserNotAuthorized: {
errorMessage += "You must be the broadcaster "
"to set the game.";
}
break;

case Error::Ratelimited: {
errorMessage +=
"You are being ratelimited by Twitch. Try "
"again in a few seconds.";
}
break;

case Error::Forwarded: {
errorMessage += message;
}
break;

case Error::Unknown:
default: {
errorMessage += "An unknown error has occured.";
}
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is almost identical to the code above and should be refactored into a function (in an anonymous namespace above chatterino::commands).


channel->addMessage(makeSystemMessage(errorMessage));
});
},
[channel{ctx.channel}] {
Expand Down
61 changes: 59 additions & 2 deletions src/providers/twitch/api/Helix.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,8 +616,11 @@ void Helix::unblockUser(QString targetUserId, const QObject *caller,
void Helix::updateChannel(QString broadcasterId, QString gameId,
QString language, QString title,
std::function<void(NetworkResult)> successCallback,
HelixFailureCallback failureCallback)
FailureCallback<HelixUpdateChannelError, QString> failureCallback)
{

using Error = HelixUpdateChannelError;

QUrlQuery urlQuery;
auto obj = QJsonObject();
if (!gameId.isEmpty())
Expand Down Expand Up @@ -646,7 +649,61 @@ void Helix::updateChannel(QString broadcasterId, QString gameId,
successCallback(result);
})
.onError([failureCallback](NetworkResult result) {
failureCallback();
if (!result.status())
{
failureCallback(Error::Unknown, result.formatError());
return;
}

auto obj = result.parseJson();
auto message = obj.value("message").toString();

switch(*result.status())
{
case 401: {
if (message.startsWith("Missing scope",
Qt::CaseInsensitive))
{
failureCallback(Error::UserMissingScope, message);
}
else if (message.compare(
"The ID in broadcaster_id must match the user "
"ID found in the request's OAuth token.",
Qt::CaseInsensitive) == 0)
{
failureCallback(Error::UserNotAuthorized, message);
}
else
{
failureCallback(Error::Forwarded, message);
}
}
break;

case 400:
case 403: {
failureCallback(Error::Forwarded, message);
}
break;

case 409:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong. Shouldn't 409 forward the message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the Modify Channel Information docs, 409 for this endpoint will give a Too Many Requests response when trying to see the Branded Content flag too frequently, which wouldn't be hit by /settitle or /settgame of course but I just tried to handle all errors currently provided in the docs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right. I read Too Many Requests and thought it would be 429. Probably should say 409 Conflict in the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it looks like it's just a typo in the docs, I'll remove the 409, and leave it to just 429.

case 429: {
failureCallback(Error::Ratelimited, message);
}
break;

case 500: {
failureCallback(Error::Unknown, message);
}

default: {
qCDebug(chatterinoTwitch)
<< "Helix update channel, unhandled error data:" << result.formatError()
<< result.getData() << obj;
failureCallback(Error::Unknown, message);
}
break;
}
})
.execute();
}
Expand Down
16 changes: 13 additions & 3 deletions src/providers/twitch/api/Helix.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,16 @@ enum class HelixUpdateChatSettingsError { // update chat settings
Forwarded,
}; // update chat settings

enum class HelixUpdateChannelError { // /settitle, /setgame
pajlada marked this conversation as resolved.
Show resolved Hide resolved
pajlada marked this conversation as resolved.
Show resolved Hide resolved
pajlada marked this conversation as resolved.
Show resolved Hide resolved
Unknown,
UserMissingScope,
UserNotAuthorized,
Ratelimited,

// The error message is forwarded directly from the Twitch API
Forwarded,
}; // /settitle, /setgame

enum class HelixBanUserError { // /timeout, /ban
Unknown,
UserMissingScope,
Expand Down Expand Up @@ -862,7 +872,7 @@ class IHelix
virtual void updateChannel(
QString broadcasterId, QString gameId, QString language, QString title,
std::function<void(NetworkResult)> successCallback,
HelixFailureCallback failureCallback) = 0;
FailureCallback<HelixUpdateChannelError, QString> failureCallback) = 0;

// https://dev.twitch.tv/docs/api/reference#manage-held-automod-messages
virtual void manageAutoModMessages(
Expand Down Expand Up @@ -1183,8 +1193,8 @@ class Helix final : public IHelix
void updateChannel(QString broadcasterId, QString gameId, QString language,
QString title,
std::function<void(NetworkResult)> successCallback,
HelixFailureCallback failureCallback) final;

FailureCallback<HelixUpdateChannelError, QString> failureCallback) final;
// https://dev.twitch.tv/docs/api/reference#manage-held-automod-messages
void manageAutoModMessages(
QString userID, QString msgID, QString action,
Expand Down
Loading