Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
GnomedDev opened this issue Jan 30, 2022 · 8 comments
Open

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

GnomedDev opened this issue Jan 30, 2022 · 8 comments

Comments

@GnomedDev
Copy link

GnomedDev commented Jan 30, 2022

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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

@freyacodes
Copy link
Member

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

@GnomedDev
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants