search

roogue / osu-collector-dl

Automatic Downloader for Osu!Collector
MIT License
38 stars 3 forks source link

[ BUG ] Ratelimits #18

Closed Calemy closed 2 days ago

Calemy commented 3 weeks ago

Describe A clear and concise description of what the bug is.

To Reproduce download a bunch of maps

418 (blocked ip from catboy.best)

Expected behavior respect ratelimits

Screenshots grafik

Additional context Please handle 429 so your users don't get fucked over and end up being blocked off the mirror thank you very much

roogue commented 3 weeks ago

Uhrm, interesting status code.

I am not sure about the limitations of catboy.best mirror as it's not explicitly specified on their website. The only way is to tweak around the concurrency config to find the sweet spot. However you do reminded me to make a better queue system, as the current system handle request rate by download speed. Anyway, I suppose this is not a bug but rather a suggestion. Thank you for reporting.

Philenst commented 3 weeks ago

For there being nothing on the site is fair, I will address that.

However Mino rate-limit works in 3 stages, we return 200 and obviously the download, then we return a 429, which is the standard for "Too many requests", if you keep requesting beyond that 429 (15req/min burst), we're going to ban that IP, and start returning 418.

If you don't want to handle the 429, we also provide a X-Ratelimit-Remaining header in the download, which you can also utilize for this purpose, this is returned no matter if the user is getting a 200 or a 429.

Our IP ban list from your service at this point is fairly long, (~30 IPs) and we'd not like it to get longer as this issue is transparent and likely not understood by the end users who utilize your application, therefore this ban is unfair on them.

roogue commented 3 weeks ago

Hi Philenst,

I apologize for the unintended misuse of your service by my program. This was not my intention, and I deeply regret any inconvenience caused.

Initially, I used chimu.moe, one of the largest osu! mirror service provider websites, which did not have strict rate limitations as per what I can remember. Consequently, I focused on overall functionality rather than rate limitation handling. After chimu.moe was discontinued, the original site redirected users to your service. I assumed it was managed by the same development team under a different brand, leading me to use your service similarly.

I understand the unfairness to end users and appreciate your transparency on this issue. Unfortunately, I cannot simply "terminate the service", as it runs locally on users' machines. Additionally, my program lacks an "announcement" or "notice" function to broadcast this issue. For now, the best I can do is update the README and any other available information to prevent new users from encountering this problem.

Regarding the implementation of rate limitation handling, I regret that I cannot address this immediately due to ongoing academic commitments and exams. I will seek a collaborator who can submit a pull request to resolve this issue as a matter of urgency.

Thank you for your understanding, and I sincerely apologize for any inconvenience caused.

Philenst commented 3 weeks ago

Thank you for your understanding on the matter. After all we're trying to provide a service that everyone can use free of charge, just as you provide this application.

I can understand the frustration with the current strict rate limits, upon reviewing this, I'm in agreement that they are a bit too strict, therefore we will work on improving these (Planning to allow up to 120 maps/min depending on our download source)

I understand the struggle of other commitments, especially academic, I've been there and thankfully done that. Therefore I'm wishing you the best in this exam season.

roogue commented 3 weeks ago

Understood. I will try to make the necessary changes to your information after addressing my current academic commitments, likely after the middle of this month. Thanks a lot for your understanding :)

roogue commented 2 days ago

Close as completed, released v2.7.5

Philenst commented 1 day ago

Thank you for the swift work on this @roogue, I've cleared out the ban list therefore all previously banned users will be able to utilise Mino once again.

roogue commented 1 day ago

Thank you, @Philenst. I apologize for any inconvenience caused previously. The current program allows up to 50 requests/min and 5 burst requests by default (429 is handled too), which has been tested and runs smoothly within this limit. Thank you once again for your free and API key-less service. If there are any issues or areas of dissatisfaction, please feel free to open a new issue. I am happy to discuss further.