lavalink-devs / Lavalink

Standalone audio sending node based on Lavaplayer.
https://lavalink.dev/
MIT License
1.55k stars 674 forks source link

[Feature Request] Implement 429 handling for URL queries #614

Open GnomedDev opened 2 years ago

GnomedDev commented 2 years ago

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.

freyacodes commented 2 years ago

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

GnomedDev commented 2 years ago

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"

freyacodes commented 2 years ago

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

GnomedDev commented 2 years ago

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

freyacodes commented 2 years ago

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.

GnomedDev commented 2 years ago

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

freyacodes commented 2 years ago

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

GnomedDev commented 2 years ago

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