feat(extractor): polite HTTP client with Retry-After + restart-loop fix#317
Merged
SimplicityGuy merged 2 commits intomainfrom Apr 29, 2026
Merged
feat(extractor): polite HTTP client with Retry-After + restart-loop fix#317SimplicityGuy merged 2 commits intomainfrom
SimplicityGuy merged 2 commits intomainfrom
Conversation
Discogs and MusicBrainz both rate-limit aggressively. The extractor was issuing raw `reqwest::get` calls with no User-Agent and no 429 handling. On a 429, the response body became the "HTML" we scraped → no year directories → exit 1 → docker `restart: on-failure` fired ~50s later and slid the limiter window forward. This change introduces `polite_http::PoliteClient`: * Sends `User-Agent: discogsography-extractor/<version>` on every request * Enforces a configurable minimum gap between requests (5s Discogs, 2s MB) * Honors `Retry-After` on 429 / 503, sleeping in-process up to a cap (2h Discogs, 30m MB) so cooldowns don't trigger restart-loop flapping * Falls back to growing default backoff when `Retry-After` is absent Both downloaders are converted to use the polite client at all 6 call sites. Retry counts go 3→5 and base delays 2s→30s for download attempts. Belt-and-suspenders: on terminal failure, `main.rs` sleeps `FAILURE_COOLDOWN_SECS` (default 600s) before exiting so docker can't flap us through a residual rate-limit window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…cooldown Codecov flagged 18 uncovered lines on PR #317. This adds: * extracted `parse_failure_cooldown` + `apply_failure_cooldown` helpers in main.rs so the env-var → duration mapping and the actual sleep are unit-testable without invoking `process::exit` (covers the 13 lines on the previously-untestable Err branch) * discogs_downloader: tests for scrape main-page returning 500 (error surfaced) and one of two year directories returning 500 (warn-and- continue, other year still succeeds) * musicbrainz_downloader: tests for index 500 and SHA256SUMS 404 paths Lib tests 480 → 486 (+6); bin tests 479 → 484 (+5). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
E2E Coverage (webkit)
|
Contributor
E2E Coverage (firefox)
|
Contributor
E2E Coverage (chromium)
|
Contributor
E2E Coverage (webkit - iPhone 15)
|
Contributor
E2E Coverage (webkit - iPad Pro 11)
|
4 tasks
SimplicityGuy
added a commit
that referenced
this pull request
Apr 29, 2026
…ane (#318) PR #317 raised both downloaders' transport-error retry budget from 3 attempts at 2s base to 5 attempts at 30s base. The intent was rate-limit politeness, but rate-limit handling actually lives in `polite_http` and never reaches this loop — only post-connect transport errors do (partial reads, flush/sync errors, mid-stream drops). Side effect: integration tests in `tests/http_integration_test.rs` see `cfg(not(test))` (the `#[cfg(test)]` override only applies when the *library* itself is being unit-tested, not when external test binaries link against it). Failing-download tests therefore paid the full production backoff: 30+60+120+240 = 450s sleeping per test. That blew up `http_integration_test` from ~50s on main to 460s locally, and made `test-extractor` time out under `cargo-llvm-cov` in CI on PR #317. Restoring the original 3-retry / 2s-base — those are the right numbers for transport-error retry. The polite client owns the rate-limit backoff path with its own server-driven Retry-After handling and per- provider caps (2h Discogs, 30m MusicBrainz). Wall time: `cargo test --test http_integration_test` 460s → 75s sequentially, 20s parallel. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Discogs and MusicBrainz both rate-limit aggressively (Discogs has been observed returning a 36-minute
Retry-After). The extractor was issuing rawreqwest::getcalls with no User-Agent and no 429 handling, so:process::exit(1)restart: on-failurefired ~50s later, hammered the limiter again, and slid the cooldown window forwardThis adds
polite_http::PoliteClient, used at every upstream HTTP call site:User-Agent: discogsography-extractor/<version> (+https://github.com/...)on every requestRetry-Afteron 429 / 503, sleeping in-process up to a configurable cap (2h Discogs, 30m MusicBrainz) — so the cooldown rides out within the same binary instead of triggering a restart loopRetry-Afteris absentBelt-and-suspenders:
main.rsnow sleepsFAILURE_COOLDOWN_SECS(default 600s, override via env) before exiting on terminal failure, so docker can't flap us through a residual rate-limit window even if the polite client can't recover.Files changed
extractor/src/polite_http.rs(new, 300 lines + 6 unit tests)extractor/src/discogs_downloader.rs— wired client at 3 call sites, bumped retry knobsextractor/src/musicbrainz_downloader.rs— wired client at 3 call sites, bumped retry knobsextractor/src/main.rs—mod polite_http, post-failure cooldownextractor/src/lib.rs—pub mod polite_httpexportTest plan
cargo test— 480 lib tests + 50 integration tests pass (was 474+50 baseline → +6 new polite_http tests)cargo clippy --lib --all-features -- -D warningscleancargo fmtclean on changed filespolite_httptests cover: gap enforcement, 429 retry, Retry-After cap, max retry exhaustion, missing-header fallback, concurrent serializationextractor_di_test::test_run_musicbrainz_loop_periodic_check_ok_truestill passes (had to drop clienttimeoutin favor ofconnect_timeoutbecausetokio::test(start_paused = true)advances virtual time past wall-clock timeouts)Operator notes
FAILURE_COOLDOWN_SECS=0in the extractor environment.🤖 Generated with Claude Code