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.

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

Improve Twitch PubSub connection reliability #3643

Merged
merged 32 commits into from
May 7, 2022

Conversation

Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Apr 2, 2022

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

Improve connection stability

Additional changes

  • To simplify "json enum" to C++ enum, the magic_enum library has been added as a dependency
  • PubSubClient moved to its own file
  • PubSub Messages are mostly built as structs initially, and then finally serialized into a json string as needed
  • PubSub messages now all have their own file
  • Mentions of Pubsub have been changed to PubSub to be consistent with capitalization
  • TwitchAccountManager accountUpdated signal now uses boost2 so we can define a callback order (This fixed account-changing moderation action subscribe/unsubscribing)
  • We now unsubscribe from whispers when the Twitch account changes
  • use QJson instead of rapidjson where possible

Old description

This prevents a deadlock if a connection fails since addingClient will never be set to false.

The documentation for websocketpp says:

Either open or fail will be called for each connection. Never both. Connections that fail will never have a close handler called.

Since we don't listen for fails, addingClient will be true forever if the first connection fails, effectively resulting in a deadlock (the PubSub topic backlog will never decrease). This PR attempts to fix this bug.

There's a new bug however: If the connection will always be closed, then we will spam Twitch with requests. Maybe this should be throttled?

This prevents a deadlock if a connection fails since `addingClient` will never be set to `false`.
@Felanbird
Copy link
Collaborator


I've tested the current code for the past 4 days and had no issues, both redemptions and automod continue to work in comparison to before where they'd just ⚰️.
As well tested how the connection would handle a VPN being enabled and disabled (as this killed the connection very consistently before), and it seems to work as well.
also mod actions work too but I only tested them a few minutes ago cuz I was just thinking of the important ones most users see.

@Felanbird
Copy link
Collaborator

Felanbird commented Apr 23, 2022

Minor: Better handle Pubsub connections. This will help prevent certain functions from appearing to be broken [Automod/Channelpoint Redemptions/Mod Actions]. (#3643)

I do think this is a bugfix, but the concept of this fix belongs as high up as it can be in the changelog, I'd probably stick it at the top of the changelog (because apparently people actually do read the changelog, who knew.)

@pajlada pajlada changed the title fix: handle failed PubSub connection attempts Improve Twitch PubSub connection reliability Apr 30, 2022
@pajlada
Copy link
Member

pajlada commented May 7, 2022

image

@pajlada pajlada enabled auto-merge (squash) May 7, 2022 15:07
@pajlada pajlada merged commit f97780d into Chatterino:master May 7, 2022
@Nerixyz Nerixyz deleted the pubsub-connection-fail branch January 6, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants