gempir/go-twitch-irc

Allow the rate limiter to be sharded across multiple connections

TroyKomodo opened this issue · 4 comments

TroyKomodo commented 
panic: runtime error: index out of range [56] with length 44

goroutine 561 [running]:
github.com/gempir/go-twitch-irc/v2.(*RateLimiter).Throttle(0xc000070260, 0x2c)
        /home/troy/go/pkg/mod/github.com/gempir/go-twitch-irc/v2@v2.8.2-beta.1/ratelimit.go:45 +0x3a5
github.com/gempir/go-twitch-irc/v2.(*RateLimiter).Throttle(0xc000070260, 0x2c)
        /home/troy/go/pkg/mod/github.com/gempir/go-twitch-irc/v2@v2.8.2-beta.1/ratelimit.go:58 +0x268
github.com/gempir/go-twitch-irc/v2.(*Client).writeMessage(0xc000444900, {0x7f11a02c7fc8, 0xc0002ffc00}, {0xc000560000, 0x1ed})
        /home/troy/go/pkg/mod/github.com/gempir/go-twitch-irc/v2@v2.8.2-beta.1/client.go:897 +0xbe
github.com/gempir/go-twitch-irc/v2.(*Client).startWriter(0xc000444900, {0x7f11a02c7fc8, 0xc0002ffc00}, 0x7)
        /home/troy/go/pkg/mod/github.com/gempir/go-twitch-irc/v2@v2.8.2-beta.1/client.go:889 +0xa5
created by github.com/gempir/go-twitch-irc/v2.(*Client).makeConnection
        /home/troy/go/pkg/mod/github.com/gempir/go-twitch-irc/v2@v2.8.2-beta.1/client.go:750 +0x2ba

limiter := twitch.CreateVerifiedRateLimiter()

limit := 250
clients := make([]*twitch.Client, limit)
for i := range clients {
    clients[i] = twitch.NewClient("----", "-----")
    clients[i].SetRateLimiter(limiter)
}

Perhaps I am misusing this?
Also ideally the actual ratelimiter should be an interface so we can implement custom logic for our specific needs.

gempir commented

Hmm. I didn't really consider using the same limiter for multiple clients. It's certainly not ideal because you'll be limited to that 1 ratelimit. So the quickfix here would be just moving the limiter into the loop.

I should still prevent a panic though. Maybe a mutex of sorts is needed

TroyKomodo commented

Yeah i modified it locally adding a mutex. And that worked as intended.
And is the ratelimit not per account or is it per connection?

gempir commented

You're right. It's account based, so you should be able to share Ratelimiter. I didn't think about this usecase. Thank you for the input. Will look at it in more detail when I have more time

TroyKomodo commented

👍