Skip to content

New: Updated Rarbg request limits#5347

Merged
markus101 merged 5 commits intodevelopfrom
rarbg-limits
Apr 14, 2023
Merged

New: Updated Rarbg request limits#5347
markus101 merged 5 commits intodevelopfrom
rarbg-limits

Conversation

@markus101
Copy link
Copy Markdown
Member

Database Migration

NO

Description

Updates Rarbg to handle additional rate limit cases.

Todos

  • Tests

Issues Fixed or Closed by this PR

@markus101 markus101 requested a review from Taloth January 5, 2023 07:20

var jsonResponse = new HttpResponse<RarbgResponse>(indexerResponse.HttpResponse);

if (jsonResponse.Resource.rate_limit == 1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this for now. RarBG has a bug where rate_limit = 1 on almost every request

Ref - Prowlarr/Prowlarr#1277
Ref - Prowlarr/Prowlarr@5cc044a

@bakerboy448
Copy link
Copy Markdown
Contributor

bakerboy448 commented Jan 5, 2023

would it be prudent to remove the defunct cloudflare /captcha logic and associated settings as well?

    I can merge it, it just won't help anyone since CF Captcha changed... I basically should remove the whole feature, but never bothered. :smile:

Originally posted by @Taloth in #4821 (comment)

var jsonResponse = new HttpResponse<RarbgResponse>(indexerResponse.HttpResponse);

// TODO: Uncomment when RARBG doesn't return it for most valid requests
// if (jsonResponse.Resource.rate_limit == 1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// if (jsonResponse.Resource.rate_limit == 1)
if (jsonResponse.Resource.rate_limit is 1 && jsonResponse.Resource.torrent_results == null)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uncomment & only throw if there are no results to workaround false positive


if (jsonResponse.Resource.torrent_results == null)
{
if (jsonResponse.Resource.rate_limit == 1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest skip handling jsonResponse.Resource.error_code == 5 above and handle here instead
5 indicates a rate limit

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m basing the changes on the GHI, if we need to make other changes later we can.

@bakerboy448
Copy link
Copy Markdown
Contributor

Based on comments / users with jackett's testing - it probably makes sense to comment out the the rate_limit = 1 handling and land this otherwise as is.

x-ref Jackett/Jackett#14000 (comment)

@bakerboy448
Copy link
Copy Markdown
Contributor

FYI some prowlarr rework to catch the http error before it tries to parse so the html isn't tried to be parsed

Prowlarr/Prowlarr#1458

@mynameisbogdan
Copy link
Copy Markdown
Contributor

FYI some prowlarr rework to catch the http error before it tries to parse so the html isn't tried to be parsed

Prowlarr/Prowlarr#1458

Sonarr doesn't seem to be affected by this, since it doesn't retry with a new token if the current one is expired.

@bakerboy448 bakerboy448 mentioned this pull request Mar 23, 2023
1 task
case HttpStatusCode.TooManyRequests:
throw new RequestLimitReachedException("Indexer API limit reached", TimeSpan.FromMinutes(2));
case (HttpStatusCode)520:
throw new RequestLimitReachedException("Indexer API returned unknown error", TimeSpan.FromMinutes(3));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: 520 is known - internal servers behind cloudflare are rate limited/overwhelmed

when rated limited web returns http code 444 to cloudflare which returns 520 on cloudflare , there is no way to send 429 to cloudflare without it breaking the request properly and retrying it

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless it throws a 520 for another reason…

@markus101 markus101 changed the title Updated Rarbg request limits New: Updated Rarbg request limits Apr 14, 2023
@markus101 markus101 merged commit 47cf8e6 into develop Apr 14, 2023
@markus101 markus101 deleted the rarbg-limits branch April 14, 2023 02:54
@yardenshoham
Copy link
Copy Markdown

When will this be released?

@bakerboy448
Copy link
Copy Markdown
Contributor

Please use one of the support channels: forums, subreddit, discord , or IRC for support/questions.

no eta on when v4 beta would be released

also a moot point as rarbg is defunct

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TorrentAPI new limits/changes

4 participants