diff --git a/src/main/java/org/kohsuke/github/GHRateLimit.java b/src/main/java/org/kohsuke/github/GHRateLimit.java index 06af2831bf..51faa4c10e 100644 --- a/src/main/java/org/kohsuke/github/GHRateLimit.java +++ b/src/main/java/org/kohsuke/github/GHRateLimit.java @@ -494,7 +494,7 @@ && getRemaining() <= other.getRemaining())) { return this; } else if (!(other instanceof UnknownLimitRecord)) { // If the above is not the case that means other has a later reset - // or the same resent and fewer requests remaining. + // or the same reset and fewer requests remaining. // If the other record is not an unknown record, the other is more recent return other; } else if (this.isExpired() && !other.isExpired()) { diff --git a/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java b/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java index 9ef2fd9ea8..30f3193c2f 100644 --- a/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java +++ b/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.io.InterruptedIOException; +import java.time.Duration; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import java.time.temporal.ChronoUnit; @@ -23,6 +24,11 @@ */ public abstract class GitHubAbuseLimitHandler extends GitHubConnectorResponseErrorHandler { + /** + * On a wait, even if the response suggests a very short wait, wait for a minimum duration. + */ + private static final int MINIMUM_ABUSE_RETRY_MILLIS = 1000; + /** * Create default GitHubAbuseLimitHandler instance */ @@ -143,7 +149,7 @@ public void onError(GitHubConnectorResponse connectorResponse) throws IOExceptio }; // If "Retry-After" missing, wait for unambiguously over one minute per GitHub guidance - static long DEFAULT_WAIT_MILLIS = 61 * 1000; + static long DEFAULT_WAIT_MILLIS = Duration.ofSeconds(61).toMillis(); /* * Exposed for testability. Given an http response, find the retry-after header field and parse it as either a @@ -156,11 +162,23 @@ static long parseWaitTime(GitHubConnectorResponse connectorResponse) { } try { - return Math.max(1000, Long.parseLong(v) * 1000); + return Math.max(MINIMUM_ABUSE_RETRY_MILLIS, Duration.ofSeconds(Long.parseLong(v)).toMillis()); } catch (NumberFormatException nfe) { // The retry-after header could be a number in seconds, or an http-date + // We know it was a date if we got a number format exception :) + + // Don't use ZonedDateTime.now(), because the local and remote server times may not be in sync + // Instead, we can take advantage of the Date field in the response to see what time the remote server + // thinks it is + String dateField = connectorResponse.header("Date"); + ZonedDateTime now; + if (dateField != null) { + now = ZonedDateTime.parse(dateField, DateTimeFormatter.RFC_1123_DATE_TIME); + } else { + now = ZonedDateTime.now(); + } ZonedDateTime zdt = ZonedDateTime.parse(v, DateTimeFormatter.RFC_1123_DATE_TIME); - return ChronoUnit.MILLIS.between(ZonedDateTime.now(), zdt); + return Math.max(MINIMUM_ABUSE_RETRY_MILLIS, ChronoUnit.MILLIS.between(now, zdt)); } } diff --git a/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java b/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java index cc2420b234..96dbad4c14 100644 --- a/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java +++ b/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java @@ -481,9 +481,8 @@ public void onError(@NotNull GitHubConnectorResponse connectorResponse) throws I "You have exceeded a secondary rate limit. Please wait a few minutes before you try again"); long waitTime = parseWaitTime(connectorResponse); - // The exact value here will depend on when the test is run assertThat(waitTime, Matchers.lessThan(GitHubAbuseLimitHandler.DEFAULT_WAIT_MILLIS)); - assertThat(waitTime, Matchers.greaterThan(3 * 1000l)); + assertThat(waitTime, equalTo(8 * 1000l)); GitHubAbuseLimitHandler.WAIT.onError(connectorResponse); }