gempir / go-twitch-irc

go irc client for twitch.tv
MIT License
357 stars 59 forks source link

Implement method to respect rateLimits #148

Closed deefdragon closed 2 years ago

deefdragon commented 3 years ago

Creating this to not bog #128 down discussing implementation.

The cleanest implementation I can think of on this is to put an optional middleware channel between the Join/Say and c.send. If a rate limit rate is not set, it will skip this and simply use c.send. If one is set, the value gets sent to the middleware.

Whenever a new message is sent to the receiver, it gets added to a stdlib list, and based on a ticker, is then output to c.send. Only downside is that I don't want to leave a ticker running all the time, so that may require some shenanigans based on connect/disconnect and if messages are ready to be sent to enable/disable/reset.

Thoughts @gempir

gempir commented 3 years ago

Yeah sounds like a good implementation. The Ticker should only run when the middleware is active and the chat connection is active.

deefdragon commented 3 years ago

A few things. First: What do we want the default ratelimits to be set at? I'm thinking unknown bots level, but I can see an argument for the default being to ignore them. Id prefer only people who at least perused the docs/reamed ignore them, but I can also see the need to preserve the "current value" of no limits.

Second: Going to do a PR for code-review purposes. Still a number of things I need to add (mod or not detection for example), but I could use some input and sanity checks. I feel like I need mutexes somewhere or something as well, but I am spacing out after having stared at this code for long enough.

Third: I'm running into a rather strange problem. Any chance if there is a known inconsistency for TestRejoinOnReconnect? If I extend the timeouts to 30 seconds, I get this ~50% of the time. (if the timeout is less than that, I get the the test failed timeout) the other 50% it just passes.

panic: Fail in goroutine after TestCanConnectAndAuthenticate has completed

goroutine 365 [running]:
testing.(*common).Fail(0xc0003b0900)
        /usr/local/go/src/testing/testing.go:688 +0x125
testing.(*common).Errorf(0xc0003b0900, 0x712d71, 0x31, 0xc0000f5f98, 0x2, 0x2)
        /usr/local/go/src/testing/testing.go:794 +0x93
github.com/gempir/go-twitch-irc/v2.assertErrorsEqual(...)
        /home/deef/workspace/src/go-twitch-irc/helpers_test.go:109
github.com/gempir/go-twitch-irc/v2.connectAndEnsureGoodDisconnect.func1(0xc000660540, 0xc0003b0900, 0xc0009fbbc0)
        /home/deef/workspace/src/go-twitch-irc/client_test.go:100 +0xca
created by github.com/gempir/go-twitch-irc/v2.connectAndEnsureGoodDisconnect
        /home/deef/workspace/src/go-twitch-irc/client_test.go:98 +0x71
exit status 2
FAIL    github.com/gempir/go-twitch-irc/v2      20.612s
gempir commented 3 years ago

First:

Off by default. And then having "presets" for Users, Known Bots and Verified Bots. Not sure if the Ratelimits differ for anonymous vs users, so maybe only 2 Presets.

Second: Keep in mind to always compile your code with the -race to see any race conditions early on.

Third: I think I remember some inconsistencies with some tests yes, not sure if that one, we never quite figured out how to 100% solve it but it stopped being super frequent. Definitely not 50% of the time.

gempir commented 3 years ago

160 will handle join limits

deefdragon commented 3 years ago

Apologies for lack of anything on this. Got nerd-sniped to examining how the go x library does rate limiting, and finding it lacking, followed by work being... work etc. etc.

If someone else wants to take this with send, feel free as I don't think I will be able to do it any time soon, and again sorry.

zneix commented 3 years ago

Thought I'd mention it here... As of recent silent rate-limit documentation udpate: as a verified bot, the 100 messages per 30 seconds in 1 channel still applies to the bot's account, even with the sitewide limit of 7500 messages per 30 seconds. However, after lots of testing with others I found out that /ban and /timeout messages don't count towards that 100/30s limit - you are still only limited with 7500/30s limit. That, and also the fact that "The channel limits above also apply" (from rate-limit documentation) should be taken into account while implementing ratelimitting for PRIVMSG messages.

gempir commented 2 years ago

Join ratelimiting is mostly implemented now, will release beta 3.0 soon-ish and if if we are happy with it a stable release after.

The message ratelimiting will be tracked in #175