Lavalink icon indicating copy to clipboard operation
Lavalink copied to clipboard

[Feature Request] Implement 429 handling for URL queries

Open GnomedDev opened this issue 2 years ago • 8 comments

In my bot, I am feeding gTTS URLs to lavalink manually as is supported. However recently I discovered that the 429 IP block proxy is only used for Youtube queries. It would be very much appreciated if this could be implemented for all queries.

The API I am using does not require auth, and seems to ban via IP, so shouldn't be a problem. An example URL is

https://translate.google.com/translate_tts?ie=UTF-8&total=1&idx=0&client=tw-ob&tl=en&q=hello&textlen=5

however this shouldn't be important as all URLs should be supported.

If the URL queried does not support IPv6 there should be a warning put into the log, letting the developer know that rate limit avoidance will not work.

GnomedDev avatar Jan 30 '22 15:01 GnomedDev

The choice for IPv6 rotation to only apply to YouTube is deliberate. It could easily be changed with the new Lavalink plugin system though.

If the URL queried does not support IPv6 there should be a warning put into the log, letting the developer know that rate limit avoidance will not work.

I believe it already does that

freyacodes avatar Jan 30 '22 16:01 freyacodes

If the decision is deliberate, what is the reasoning? By the warning, I meant with the new system, as a response to "what if the URL doesn't support IPv6"

GnomedDev avatar Jan 30 '22 17:01 GnomedDev

If the decision is deliberate, what is the reasoning?

Well first of all, that's how it's written in Lavaplayer. E.g. the classes are named after YouTube. I don't believe there's actually anything stopping you from applying it to a generic audio source though.

Secondly, it's a little bit risky to assume that every domain with an AAAA record supports IPv6.

By the warning, I meant with the new system, as a response to "what if the URL doesn't support IPv6"

Yes but it doesn't seem like a new system is required

freyacodes avatar Jan 30 '22 17:01 freyacodes

Well first of all, that's how it's written in Lavaplayer. E.g. the classes are named after YouTube. I don't believe there's actually anything stopping you from applying it to a generic audio source though.

so why can't this be changed, should I open an issue on lavaplayer?

little bit risky to assume that every domain with an AAAA record supports IPv6.

why would it be risky to check? Then, if the URL does not support IPv6, just to warn in logs and continue on as is now

GnomedDev avatar Jan 30 '22 18:01 GnomedDev

so why can't this be changed, should I open an issue on lavaplayer?

What would you change if it's ready to use? At most I would change the log statements mentioning YouTube.

why would it be risky to check? Then, if the URL does not support IPv6, just to warn in logs and continue on as is now

We already fall back to v4 if there's no v6 record for the domain. That's no guarantee that IPv6 actually works though, and when requests fail v4 probably won't either. Might be a little overly-cautious. I wouldn't put a v4 retry in such case.

freyacodes avatar Jan 30 '22 19:01 freyacodes

Wait, does the rate limit IPv6 stuff work with other websites or not. If it's "ready to use" why doesn't it?

GnomedDev avatar Jan 30 '22 19:01 GnomedDev

In theory yes. But it wasn't intended to and Lavalink only uses it for YouTube

freyacodes avatar Jan 30 '22 19:01 freyacodes

Okay. So this feature request is to make it supported and usable for other websites than YouTube.

GnomedDev avatar Jan 30 '22 20:01 GnomedDev