Better Retry and Provider Limit handling

[omgbeez]

User description

• It'll attempt a retry, and respect Retry-After headers if present.
• Remove ACTIVE_LIMIT error checks in TorBox, these conditions can never be reached as TorBox doesn't queue whin this happens, and TorBoxNET throws an exception for the error.

---

PR Type

Enhancement, Bug fix

---

Description

• Replace Polly HTTP retry policy with standard resilience handler

• Implement Retry-After header support for respecting server delays

• Remove unreachable ACTIVE_LIMIT error handling in TorBox client

• Simplify retry logic with exponential backoff and jitter

---

Diagram Walkthrough

flowchart LR
  A["Old Polly Policy"] -->|"Replace with"| B["Standard Resilience Handler"]
  B -->|"Supports"| C["Retry-After Headers"]
  B -->|"Includes"| D["Exponential Backoff + Jitter"]
  E["TorBox Client"] -->|"Remove"| F["Unreachable ACTIVE_LIMIT Checks"]

Loading

File Walkthrough

	Relevant files
Enhancement	DiConfig.cs Migrate to standard resilience handler with Retry-After support server/RdtClient.Service/DiConfig.cs Remove unused System.Net and Polly.Extensions.Http imports Replace custom Polly retry policy with AddStandardResilienceHandler Add Retry-After header parsing in custom delay generator Configure exponential backoff with jitter and 10-second timeout +30/-8
Bug fix	TorBoxTorrentClient.cs Remove unreachable ACTIVE_LIMIT error handling                      server/RdtClient.Service/Services/TorrentClients/TorBoxTorrentClient.cs Remove ACTIVE_LIMIT error handling from AddMagnet method Remove ACTIVE_LIMIT error handling from AddFile method Simplify return logic to directly use API response data +0/-13

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Null/failed response: The new code assumes result.Data and result.Data.Hash are always non-null and returns result.Data!.Hash! without checking result.Error or null values, risking unhandled exceptions and loss of actionable error context. Referred Code public async Task<String> AddMagnet(String magnetLink) { var user = await GetClient().User.GetAsync(true); var result = await GetClient().Torrents.AddMagnetAsync(magnetLink, user.Data?.Settings?.SeedTorrents ?? 3, false); return result.Data!.Hash!; } public async Task<String> AddFile(Byte[] bytes) { var user = await GetClient().User.GetAsync(true); var result = await GetClient().Torrents.AddFileAsync(bytes, user.Data?.Settings?.SeedTorrents ?? 3); return result.Data!.Hash!; } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Unbounded Retry-After: The Retry-After header (external input) is used to compute retry delay without an upper bound, enabling a server/proxy to force excessively long delays (potential client-side DoS) unless capped. Referred Code .AddStandardResilienceHandler(options => { options.AttemptTimeout.Timeout = TimeSpan.FromSeconds(10); options.Retry.MaxRetryAttempts = 5; options.Retry.BackoffType = DelayBackoffType.Exponential; options.Retry.UseJitter = true; options.Retry.Delay = TimeSpan.FromSeconds(2); options.Retry.DelayGenerator = args => { // Check if we have a result and if it contains the Retry-After header if (args.Outcome.Result is { } response && response.Headers.RetryAfter is { } retryAfter) { // The header can be either a specific date or a delay in seconds if (retryAfter.Delta.HasValue) { return ValueTask.FromResult<TimeSpan?>(retryAfter.Delta.Value); } if (retryAfter.Date.HasValue) { var delay = retryAfter.Date.Value - DateTimeOffset.UtcNow; ... (clipped 8 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Exception exposure risk: The use of null-forgiving operators on result.Data!.Hash! may surface raw exceptions to callers depending on middleware/exception mapping, which should be verified to ensure user-facing errors remain generic. Referred Code public async Task<String> AddMagnet(String magnetLink) { var user = await GetClient().User.GetAsync(true); var result = await GetClient().Torrents.AddMagnetAsync(magnetLink, user.Data?.Settings?.SeedTorrents ?? 3, false); return result.Data!.Hash!; } public async Task<String> AddFile(Byte[] bytes) { var user = await GetClient().User.GetAsync(true); var result = await GetClient().Torrents.AddFileAsync(bytes, user.Data?.Settings?.SeedTorrents ?? 3); return result.Data!.Hash!; } Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Handle 429 status for retries Suggestion Impact: The commit changed the HTTP client resilience setup to a custom AddResilienceHandler with an explicit retry ShouldHandle predicate that includes HttpStatusCode.TooManyRequests (429). This satisfies the core intent of retrying on 429, though it does not implement the suggested DelayGenerator-based Retry-After handling (it was removed in favor of RateLimitHeaders.Polly proactive throttling). code diff: services.AddHttpClient(RD_CLIENT) - .AddStandardResilienceHandler(options => - { - options.AttemptTimeout.Timeout = TimeSpan.FromSeconds(10); - options.Retry.MaxRetryAttempts = 5; - options.Retry.BackoffType = DelayBackoffType.Exponential; - options.Retry.UseJitter = true; - options.Retry.Delay = TimeSpan.FromSeconds(2); - options.Retry.DelayGenerator = args => + .AddResilienceHandler("rd_client_handler", builder => { - // Check if we have a result and if it contains the Retry-After header - if (args.Outcome.Result is { } response && response.Headers.RetryAfter is { } retryAfter) + builder.AddRateLimitHeaders(options => options.EnableProactiveThrottling = true); + builder.AddRetry(new() { - // The header can be either a specific date or a delay in seconds - if (retryAfter.Delta.HasValue) + ShouldHandle = args => args.Outcome switch { - return ValueTask.FromResult<TimeSpan?>(retryAfter.Delta.Value); - } - - if (retryAfter.Date.HasValue) - { - var delay = retryAfter.Date.Value - DateTimeOffset.UtcNow; - - return ValueTask.FromResult<TimeSpan?>(delay > TimeSpan.Zero ? delay : TimeSpan.Zero); - } - } - - // Return null to let Polly use the default BackoffType/Delay specified above - return ValueTask.FromResult<TimeSpan?>(null); - }; - }); + { Exception: HttpRequestException } => PredicateResult.True(), + { Result.StatusCode: HttpStatusCode.RequestTimeout } => PredicateResult.True(), + { Result.StatusCode: >= HttpStatusCode.InternalServerError } => PredicateResult.True(), + { Result.StatusCode: HttpStatusCode.TooManyRequests } => PredicateResult.True(), + _ => PredicateResult.False() + }, + MaxRetryAttempts = 5, + BackoffType = DelayBackoffType.Exponential, + Delay = TimeSpan.FromSeconds(2), + UseJitter = true + }); Update the resilience handler to retry on 429 Too Many Requests status codes, ensuring the custom DelayGenerator for Retry-After headers is triggered correctly. server/RdtClient.Service/DiConfig.cs [68-98] services.AddHttpClient(RD_CLIENT) .AddStandardResilienceHandler(options => { options.AttemptTimeout.Timeout = TimeSpan.FromSeconds(10); options.Retry.MaxRetryAttempts = 5; + options.Retry.ShouldHandle = args => + { + if (args.Outcome.Result?.StatusCode == System.Net.HttpStatusCode.TooManyRequests) + { + return ValueTask.FromResult(true); + } + + // In addition to 429, we want to retry the default transient errors. + return Polly.Extensions.Http.HttpResiliencePredicates.IsTransient(args.Outcome); + }; options.Retry.BackoffType = DelayBackoffType.Exponential; options.Retry.UseJitter = true; options.Retry.Delay = TimeSpan.FromSeconds(2); options.Retry.DelayGenerator = args => { // Check if we have a result and if it contains the Retry-After header if (args.Outcome.Result is { } response && response.Headers.RetryAfter is { } retryAfter) { // The header can be either a specific date or a delay in seconds if (retryAfter.Delta.HasValue) { return ValueTask.FromResult<TimeSpan?>(retryAfter.Delta.Value); } if (retryAfter.Date.HasValue) { var delay = retryAfter.Date.Value - DateTimeOffset.UtcNow; return ValueTask.FromResult<TimeSpan?>(delay > TimeSpan.Zero ? delay : TimeSpan.Zero); } } // Return null to let Polly use the default BackoffType/Delay specified above return ValueTask.FromResult<TimeSpan?>(null); }; }); [Suggestion processed] Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies that the new resilience handler will not retry on 429 Too Many Requests status codes, which was handled by the old policy, thus rendering the new DelayGenerator logic for Retry-After headers ineffective.	High
Update

[omgbeez]

Qodo is incorrect on these two points:

Generic: Robust Error Handling and Edge Case Management - These values are guaranteed to be set. If not, the only handling will be by exception anyway, there is no recovery.

Generic: Security-First Input Validation and Data Handling - The Retry-After is not actually unbounded, and the http-request timeout still applies even if there is a large value. Also, this is explicitly handled in the 429 Handler for TorBox and rethrows a RateLimitException, as it's expected.

I have extensively tested the rate limiting with TorBox, it's handling well and this PR is ready to go.