fix(http): on_retry reports target URL on 429 under scraper_api (1.6.2) - #182
Merged
Conversation
The sync client's 429 retry branch passed the rewritten scraper_api provider endpoint to on_retry instead of target_url, unlike every sibling hook call (on_request, the 5xx and RequestError on_retry branches) and unlike the async client's 429 branch. A retry dashboard/metric then attributed a rate-limited retry to api.scraperapi.com rather than the site being scraped, violating the mldsveda#171 contract that hooks report the logical target URL. Regression test fails on the old code (hook saw the provider endpoint) and passes with the one-line fix. Bump to 1.6.2.
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.
Bug
The sync HTTP client's 429 (rate-limit) retry branch passed the rewritten
scraper_apiprovider endpoint to theon_retryhook instead of the logical target URL.Every other hook call in the same method already passes
target_url—on_request, and the 5xx andRequestErroron_retrybranches — and the async client's 429 branch does too. So this was a sync/async divergence: the #171 fix ("hooks report the logical target URL, not the provider endpoint") was applied everywhere except the sync 429 path.Impact
With
scraper_apiconfigured, a rate-limited retry firedon_retrywithapi.scraperapi.cominstead of the site being scraped, so a user's retry dashboard/metrics misattributed the retry.Fix
One line: pass
target_urlin the 429 branch, matching the siblings and the async client.Test
Added
test_on_retry_429_reports_target_url_not_scraper_api_endpoint(mirrors the existingon_requestscraper_api test). Regression-proven: it fails on the old code (hook received the provider endpoint) and passes with the fix.590 passed, ruff clean. Bumped to 1.6.2 across all four version sites + CHANGELOG.