Fix retry logic for HTTP 5xx errors and network failures #471
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.
Refactors error handling and retry logic to be more robust and consistent.
There were a number of issues related to this which are now addressed. Tried to simplify the flow and make the main data retrieval route more readable.
Key changes:
URLError,socket.error, andIncompleteRead(fixes Retry API requests which failed due to whatever reasons #110)retry-afterand rate limit headers per GitHub API requirementsmake_request_with_retry()wrapperFixes #140, #110, #138
#138 suggests adding a
--continue-on-errorbehavior, this is more part of a per-repo solution that relates to #244Retry behavior
Logging improvements
Clear visibility into retry behavior and failures:
warningHTTP 502, retrying in 2.1s (attempt 1/5)warningConnection error: <urlopen error ...>, retrying in 1.0s (attempt 1/5)warningJSONDecodeError reading response/Retrying in 1.0s (attempt 1/5)infoThrottling: 50 requests left, pausing 30serrorHTTP 502 failed after 5 attemptserrorConnection error failed after 5 attempts: <error>errorFailed to read response after 5 attempts for <url>infoHint: Authenticate to raise your GitHub rate limitDead code removed
_request_http_error()- was dead code,errorslist intentionally disabled in 2016 (commit 1e5a904) to fix Quits after resuming from rate limit sleep #29_request_url_error()- same as aboveerrorslist parameter threading - never usedParameter rename
The
retrieve_data()function parametersingle_requesthas been renamed topaginatedwith inverted logic:single_request=Truemeant "don't paginate"paginated=Falsemeans "don't paginate"This is more intuitive (positive framing). All internal call sites have been updated.
Tests
New test added to exercise retries. Live tested against a number of repos.