gempir/go-twitch-irc

Implement method to respect rateLimits

deefdragon opened this issue · 7 comments

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

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

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

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.

#160 will handle join limits

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

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.

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