fix(metadata): retry throttled lookups instead of recording them as no match - #883
Open
krejko wants to merge 1 commit into
Open
fix(metadata): retry throttled lookups instead of recording them as no match#883krejko wants to merge 1 commit into
krejko wants to merge 1 commit into
Conversation
…o match Bulk library import issues one metadata lookup per unmatched book - over a thousand on a large library - but nothing in the pipeline handled being throttled, so a rate-limited stretch silently produced books that looked like they simply were not on Audible. Polly's HandleTransientHttpError covers 5xx and 408 but deliberately not 429, which is the only status a rate limiter returns. The Audible and Audnexus clients therefore had exponential backoff that could never fire for the case it was needed. Adds an explicit TooManyRequests clause and prefers the server's Retry-After hint over the computed backoff when it is longer, capped at 60s so a mistaken header cannot stall the pipeline. OpenLibraryService was registered with AddScoped and a bare HttpClient, which resolves the default unnamed client and carries no policy at all, so the fallback had no retry of any kind. It is now a typed client sharing the same policy. The import store's catch block marked a failed lookup hasSearched: true with no match, making a throttled request indistinguishable from a genuine miss and excluding the row from re-runs. Failures are tracked separately, the row stays unprocessed so the next run retries it, and the count is surfaced in the footer. The footer also claimed a "rate limit: 150/window" that corresponds to no limiter in the codebase; the ApiConfiguration.RateLimitPerMinute setting it appears to reference is never read by any production code. Reworded to what is actually true. Each fix was confirmed to fail its test when reverted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bulk library import issues one metadata lookup per unmatched book — over a thousand on a large library — and nothing handled being throttled.
HandleTransientHttpErrorcovers 5xx and 408 but not 429, the only status a rate limiter returns, so the Audible and Audnexus clients had exponential backoff that could never fire for the case it existed for. Adds aTooManyRequestsclause and honoursRetry-Afterwhen it exceeds the computed backoff, capped at 60s.OpenLibraryServicehad no policy at all — registered viaAddScopedwith a bareHttpClient, which resolves the default unnamed client. Now a typed client sharing the policy.The import store recorded failures as misses — the catch block set
hasSearched: truewith no match, making a 429 indistinguishable from "not on Audible" and excluding the row from re-runs. Failures are now tracked separately and stay retryable, with a count in the footer.Also reworded the footer's "rate limit: 150/window", which matches no limiter in the codebase —
ApiConfiguration.RateLimitPerMinuteis never read by any production code.Tests were each confirmed to fail with the corresponding fix reverted.