Streamline fetch and retry process #694
Conversation
bitwiseman
force-pushed
the
task/fetch
branch
from
4ca8f06 to
1234c2e
Compare
| while (true) {// loop while API rate limit is hit | ||
| int responseCode = -1; | ||
| String responseMessage = null; | ||
|
|
There was a problem hiding this comment.
It is better to view this in split diff view. Two methods got squashed down and blended.
|
|
||
| do { | ||
| // if we fail to create a connection we do not retry and we do not wrap | ||
| uc = null; |
There was a problem hiding this comment.
This ensures that on retry we don't somehow accidentally use the connection from the previous loop.
| // This is where the request is sent and response is processing starts | ||
| responseCode = uc.getResponseCode(); | ||
| responseMessage = uc.getResponseMessage(); | ||
| noteRateLimit(tailApiUrl); |
There was a problem hiding this comment.
We note the rate limit change as soon as we have confidence that we have enough information to do so.
Better for race conditions.
| if (isInvalidCached404Response(responseCode)) { | ||
| continue; | ||
| } | ||
| if (!(isRateLimitResponse(responseCode) || isAbuseLimitResponse(responseCode))) { |
There was a problem hiding this comment.
If we have an error condition that we can detect early, skip calling the lambda.
| continue; | ||
| } | ||
|
|
||
| throw interpretApiError(e, responseCode, responseMessage, url, retries); |
There was a problem hiding this comment.
If the error is not transient, process it if needed and throw.
| */ | ||
| void handleApiError(IOException e) throws IOException { | ||
| int responseCode; | ||
| try { |
There was a problem hiding this comment.
This case is handled in the main _fetch method now. We pass in the best known values and don't have to guard against bad state here.
|
|
||
| if (!retryConnectionError(e, retries)) { | ||
| throw new HttpException(responseCode, responseMessage, uc.getURL(), e); | ||
| private void detectOTPRequired(int responseCode) throws GHIOException { |
There was a problem hiding this comment.
All of these were move from handleApiError.
Like other errors we've been waiting until later to catch, this one is detectable so whe should do that before the downstream exception needs to be thrown.
bitwiseman
force-pushed
the
task/fetch
branch
from
a0fb8d3 to
289282e
Compare
1 participant
Description
Streamlining and clean up of fetching. Not quite a refactoring but close.
Rate limit changed to be updated for each retry.
Rate and Abuse handlers are part of the retry process - they will not loop indefinitely. They will also be detected without requiring an exception. This means that
fetchHttpStatusCodeno longer simply returns forbidden status when rate limit is hit.