✨ อัปเดตล่าสุด: Chatterino/chatterino/pull/

JakeRYW · 2024-05-29T02:48:10Z

Adding appropriate error handling for the /settitle and /setgame commands. Handles trying to set the title/game in unauthorized channels, missing scope, rate limits, and title being too long by providing Failed to set title - Title is too long. error message.

Also removed the status variable declaration from the setTitle function as it was not used.

Fixes #5332

Nerixyz · 2024-05-31T20:17:56Z

+    if(title.size() > 140)
+    {
+        ctx.channel->addMessage(
+            makeSystemMessage("Unable to set title: Title must be 140 characters or fewer."));
+        return "";
+    }


This shouldn't be a client-side check. #5332 mentions that the response we get from Twitch's Helix API may include a message field which we can use to report back. Currently, we're not doing anything with the response in case of an error (see Helix.cpp).

Nerixyz is correct - the reason we try using server-side errors wherever possible is to ensure compatibility with API changes (e.g. if Twitch changes their max title length to 120 or 160)

Removed the client side check, and added error handling for both the settitle and setgame since they are both utilizing the Update Channel API.

github-actions

clang-tidy made some suggestions

Nerixyz

This looks good, just a few things.

Make sure you run clang-format on the files you edited (currently, it's not formatted correctly)
The Helix mock declaration needs to be updated - since the type now contains a comma, it has to be enclosed in parentheses: (FailureCallback<HelixUpdateChannelError, QString> failureCallback).

Nerixyz · 2024-06-03T10:42:47Z

+                }
+                break;
+
+                case 409:


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

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.

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

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

Nerixyz · 2024-06-03T10:45:18Z

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


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).

Nerixyz · 2024-06-03T10:49:31Z

+                    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;
+                    }


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

Nerixyz

Looks good, now you just need a changelog entry!

Nerixyz

I already approved the changes (I'm not a collaborator, so my opinion doesn't matter). Changelog entry looks okay.

github-actions

clang-tidy made some suggestions

github-actions

clang-tidy made some suggestions

pajlada

Thank you for your contribution! As a first-time contributor, you can now add yourself to the contributors list that's shown inside the About page in Chatterino.

If you want this, you can open a new PR where you modify the resources/contributors.txt file and add yourself to the list. Make sure to read the comments at the top for instructions.

github-actions

clang-tidy made some suggestions

github-actions

clang-tidy made some suggestions

github-actions · 2024-06-09T10:50:48Z

-                 std::function<void(NetworkResult)> successCallback,
-                 HelixFailureCallback failureCallback),
-                (override));
+    MOCK_METHOD(


warning: invalid case style for function 'MOCK_METHOD' [readability-identifier-naming]

Suggested change

MOCK_METHOD(

mockMethod(

github-actions · 2024-06-09T10:50:48Z

+/// Error type for Helix::updateChannel
+///
+/// Used in the /settitle and /setgame commands
+enum class HelixUpdateChannelError {


warning: enum 'HelixUpdateChannelError' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]

enum class HelixUpdateChannelError { ^

JakeRYW added 2 commits May 28, 2024 22:20

Remove unused accessStreamStatus call

c60a514

Add error message for channel titles exceeding 140 characters

7641f87

Nerixyz suggested changes May 31, 2024

View reviewed changes

JakeRYW and others added 2 commits June 2, 2024 20:39

Adding updated error messages for Update Channel API

a0d97f0

Merge branch 'Chatterino:master' into feature/add-settitle-validation

f7e250a

JakeRYW changed the title ~~Add error message for channel titles exceeding 140 characters~~ Add updated error handling for Update Channel API Jun 3, 2024

JakeRYW requested review from Nerixyz and pajlada June 3, 2024 00:49

github-actions Bot reviewed Jun 3, 2024

View reviewed changes

Comment thread src/providers/twitch/api/Helix.hpp Outdated

Nerixyz suggested changes Jun 3, 2024

View reviewed changes

JakeRYW added 4 commits June 3, 2024 09:52

Refactoring error message into function

ec59fff

Updating UpdateChannel Mock

c2cc573

Removing 409 status since it's a typo from Twitch Docs

879a633

Update formatting

cda1c66

JakeRYW requested a review from Nerixyz June 3, 2024 14:04

Nerixyz approved these changes Jun 3, 2024

View reviewed changes

JakeRYW and others added 2 commits June 3, 2024 11:59

Adding changelog entry

e6d9b96

Merge branch 'master' into feature/add-settitle-validation

4e5d031

JakeRYW requested a review from Nerixyz June 7, 2024 13:26

Nerixyz approved these changes Jun 7, 2024

View reviewed changes

github-actions Bot reviewed Jun 7, 2024

View reviewed changes

Comment thread src/controllers/commands/builtin/twitch/UpdateChannel.cpp

Comment thread src/providers/twitch/api/Helix.hpp Outdated

pajlada added 3 commits June 8, 2024 11:37

fix: formatting

5ed0c9e

fix: updateChannel mock missing a right-paren

6997f26

fix: missing break after case 500 in Helix::updateChannel

f258c00

github-actions Bot reviewed Jun 8, 2024

View reviewed changes

Comment thread mocks/include/mocks/Helix.hpp

Comment thread src/providers/twitch/api/Helix.hpp Outdated

nit: update HelixUpdateChannelError comments

6f10bac

pajlada approved these changes Jun 8, 2024

View reviewed changes

pajlada changed the title ~~Add updated error handling for Update Channel API~~ Improved error messaging for Update Channel API Jun 8, 2024

pajlada enabled auto-merge (squash) June 8, 2024 09:51

github-actions Bot reviewed Jun 8, 2024

View reviewed changes

Comment thread mocks/include/mocks/Helix.hpp

Comment thread src/providers/twitch/api/Helix.hpp

Merge branch 'master' into feature/add-settitle-validation

a02c0f0

github-actions Bot reviewed Jun 9, 2024

View reviewed changes

pajlada merged commit 25284fc into Chatterino:master Jun 9, 2024

JakeRYW mentioned this pull request Jun 23, 2024

Adding JakeRYW to contributor list #5481

Merged

Nerixyz mentioned this pull request Aug 20, 2024

ci: show recent changes in nightly release #5553

Merged

8thony mentioned this pull request Jan 20, 2025

Handle other errors than unauthorized in /setgame and /settitle #3419

Closed

4 tasks done

babicekmarek pushed a commit to babicekmarek/streamally-chatterino that referenced this pull request Sep 14, 2026

Improved error messaging for Update Channel API (Chatterino#5429)

d808d6d

Conversation

JakeRYW commented May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Nerixyz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nerixyz left a comment

Choose a reason for hiding this comment

Uh oh!

Nerixyz left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pajlada left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 9, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 9, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JakeRYW commented May 29, 2024 •

edited

Loading