fix(bref): retry on HTTP 429 rate-limit with exponential backoff (#344) - #417
Open
cissel wants to merge 1 commit into
Open
fix(bref): retry on HTTP 429 rate-limit with exponential backoff (#344)#417cissel wants to merge 1 commit into
cissel wants to merge 1 commit into
Conversation
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
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.
Pull Request
Table of Contents generated with DocToc
Summary
Baseball Reference rate-limits scrapers with HTTP 429 (Too Many Requests). The four Bref functions — bref_standings_on_date(), bref_daily_batter(), bref_daily_pitcher(), and bref_team_results() — used xml2::read_html() directly, which has no retry. When a 429 was returned, the tryCatch swallowed it into a misleading "Invalid arguments or no data available" message, the function returned NULL, and the user saw Error: object 'x' not found. This adds a bref_read_html() helper using httr2::req_retry() with exponential backoff (up to 3 tries) for 429/503, mirroring the retry pattern already established for NCAA scrapers (request_with_proxy() in the same file).
Type of Change
feat-- New feature (new endpoint wrapper, new exported function)fix-- Bug fix (non-breaking change that fixes an issue)docs-- Documentation only (roxygen, README, vignettes, NEWS)test-- Adding or updating testsrefactor-- Code change that neither fixes a bug nor adds a featurechore-- Tooling, build, or maintenance changeperf-- Performance improvementRelated Issues
Closes #344
Background & Context
Data source: Baseball Reference (baseball-reference.com).
Multiple users report HTTP 429 when calling bref_standings_on_date() sequentially for all six divisions (#344). The error is silently swallowed — the tryCatch error handler prints a generic "Invalid arguments or no data available" message and the function returns NULL, producing the confusing object 'x' not found downstream. The existing Sys.sleep(5) was placed after the scrape (not before a retry), so it couldn't help.
The package already has a retry pattern for NCAA scrapers in R/utils.R (request_with_proxy() uses httr2::req_retry with is_transient = 429/503). This PR extends the same pattern to Baseball Reference.
Changes Made
Submission Checklist
CLAUDE.md(native pipe
|>, return-value initialized beforetryCatch,climessaging,
any_of()column-drift guards).devtools::document()and committed the regeneratedman/andNAMESPACE(no hand edits).@param,@return,@export,@family,runnable
@examples).IP-ban caveat (NCAA endpoints exercised sparingly / against cached data).
devtools::check()passes with no new errors, warnings, or notes.NEWS.md, and reflected user-visible changes incran-comments.mdand_pkgdown.ymlwhere applicable.trailers.
Testing
devtools::test()passes locally.assertions.
Execution time before:
Execution time after:
Screenshots / Output
Reviewer Checklist
tryCatch; error path returns anempty value with a
climessage rather than erroring.man/andNAMESPACEare regenerated and consistent with the source.NEWS.md/cran-comments.md/_pkgdown.ymlare consistent for anyAPI-surface change.
Additional Notes