diff --git a/src/main/java/org/kohsuke/github/AbuseLimitHandler.java b/src/main/java/org/kohsuke/github/AbuseLimitHandler.java index 288a1542bd..dff1d992d3 100644 --- a/src/main/java/org/kohsuke/github/AbuseLimitHandler.java +++ b/src/main/java/org/kohsuke/github/AbuseLimitHandler.java @@ -3,11 +3,7 @@ import org.kohsuke.github.connector.GitHubConnectorResponse; import java.io.IOException; -import java.io.InterruptedIOException; import java.net.HttpURLConnection; -import java.time.ZonedDateTime; -import java.time.format.DateTimeFormatter; -import java.time.temporal.ChronoUnit; import javax.annotation.Nonnull; @@ -88,13 +84,8 @@ public void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws I public static final AbuseLimitHandler WAIT = new AbuseLimitHandler() { @Override public void onError(IOException e, HttpURLConnection uc) throws IOException { - try { - Thread.sleep(parseWaitTime(uc)); - } catch (InterruptedException ex) { - throw (InterruptedIOException) new InterruptedIOException().initCause(e); - } + sleep(parseWaitTime(uc)); } - }; /** @@ -116,19 +107,6 @@ public void onError(IOException e, HttpURLConnection uc) throws IOException { * number or a date (the spec allows both). If no header is found, wait for a reasonably amount of time. */ long parseWaitTime(HttpURLConnection uc) { - String v = uc.getHeaderField("Retry-After"); - if (v == null) { - // can't tell, wait for unambiguously over one minute per GitHub guidance - return DEFAULT_WAIT_MILLIS; - } - - try { - return Math.max(1000, Long.parseLong(v) * 1000); - } catch (NumberFormatException nfe) { - // The retry-after header could be a number in seconds, or an http-date - ZonedDateTime zdt = ZonedDateTime.parse(v, DateTimeFormatter.RFC_1123_DATE_TIME); - return ChronoUnit.MILLIS.between(ZonedDateTime.now(), zdt); - } + return parseWaitTime(uc.getHeaderField("Retry-After"), null, DEFAULT_WAIT_MILLIS, 1000); } - } diff --git a/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java b/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java index c7dc33f8a9..a495b90cb3 100644 --- a/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java +++ b/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java @@ -3,12 +3,8 @@ import org.kohsuke.github.connector.GitHubConnectorResponse; import java.io.IOException; -import java.io.InterruptedIOException; import java.net.HttpURLConnection; import java.time.Duration; -import java.time.ZonedDateTime; -import java.time.format.DateTimeFormatter; -import java.time.temporal.ChronoUnit; import javax.annotation.Nonnull; @@ -125,11 +121,7 @@ private boolean hasHeader(GitHubConnectorResponse connectorResponse, String head public static final GitHubAbuseLimitHandler WAIT = new GitHubAbuseLimitHandler() { @Override public void onError(GitHubConnectorResponse connectorResponse) throws IOException { - try { - Thread.sleep(parseWaitTime(connectorResponse)); - } catch (InterruptedException ex) { - throw (InterruptedIOException) new InterruptedIOException().initCause(ex); - } + sleep(parseWaitTime(connectorResponse)); } }; @@ -155,30 +147,10 @@ public void onError(GitHubConnectorResponse connectorResponse) throws IOExceptio * number or a date (the spec allows both). If no header is found, wait for a reasonably amount of time. */ static long parseWaitTime(GitHubConnectorResponse connectorResponse) { - String v = connectorResponse.header("Retry-After"); - if (v == null) { - return DEFAULT_WAIT_MILLIS; - } - - try { - 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 Math.max(MINIMUM_ABUSE_RETRY_MILLIS, ChronoUnit.MILLIS.between(now, zdt)); - } + return parseWaitTime(connectorResponse.header("Retry-After"), + connectorResponse.header("Date"), + DEFAULT_WAIT_MILLIS, + MINIMUM_ABUSE_RETRY_MILLIS); } } diff --git a/src/main/java/org/kohsuke/github/GitHubConnectorResponseErrorHandler.java b/src/main/java/org/kohsuke/github/GitHubConnectorResponseErrorHandler.java index 0a7f7fa9fb..ebeb139181 100644 --- a/src/main/java/org/kohsuke/github/GitHubConnectorResponseErrorHandler.java +++ b/src/main/java/org/kohsuke/github/GitHubConnectorResponseErrorHandler.java @@ -7,7 +7,12 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStreamReader; +import java.io.InterruptedIOException; import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; +import java.time.temporal.ChronoUnit; import javax.annotation.Nonnull; @@ -111,4 +116,38 @@ private boolean isServiceDown(GitHubConnectorResponse connectorResponse) throws return false; } }; + + static void sleep(long waitMillis) throws IOException { + try { + Thread.sleep(waitMillis); + } catch (InterruptedException ex) { + throw (InterruptedIOException) new InterruptedIOException().initCause(ex); + } + } + + static long parseWaitTime(String waitHeader, String dateHeader, long defaultMillis, long minimumMillis) { + if (waitHeader == null) { + return defaultMillis; + } + + try { + return Math.max(minimumMillis, Duration.ofSeconds(Long.parseLong(waitHeader)).toMillis()); + } catch (NumberFormatException nfe) { + // The wait header could be a number in seconds, or an http-date + // We know it was a date if we got a number format exception :) + + // Try not to 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 = dateHeader; + ZonedDateTime now; + if (dateField != null) { + now = ZonedDateTime.parse(dateField, DateTimeFormatter.RFC_1123_DATE_TIME); + } else { + now = ZonedDateTime.now(); + } + ZonedDateTime zdt = ZonedDateTime.parse(waitHeader, DateTimeFormatter.RFC_1123_DATE_TIME); + return Math.max(minimumMillis, ChronoUnit.MILLIS.between(now, zdt)); + } + } } diff --git a/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java b/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java index c16cc77a41..654a7744a6 100644 --- a/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java +++ b/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java @@ -4,7 +4,6 @@ import org.kohsuke.github.connector.GitHubConnectorResponse; import java.io.IOException; -import java.io.InterruptedIOException; import java.net.HttpURLConnection; import java.time.Duration; import java.time.ZonedDateTime; @@ -72,11 +71,7 @@ boolean isError(@NotNull GitHubConnectorResponse connectorResponse) throws IOExc public static final GitHubRateLimitHandler WAIT = new GitHubRateLimitHandler() { @Override public void onError(GitHubConnectorResponse connectorResponse) throws IOException { - try { - Thread.sleep(parseWaitTime(connectorResponse)); - } catch (InterruptedException ex) { - throw (InterruptedIOException) new InterruptedIOException().initCause(ex); - } + sleep(parseWaitTime(connectorResponse)); } }; @@ -100,6 +95,10 @@ long parseWaitTime(GitHubConnectorResponse connectorResponse) { now = ZonedDateTime.now(); } return Math.max(MINIMUM_RATE_LIMIT_RETRY_MILLIS, (Long.parseLong(v) - now.toInstant().getEpochSecond()) * 1000); + // return parseWaitTime(connectorResponse.header("X-RateLimit-Reset"), + // connectorResponse.header("Date"), + // Duration.ofMinutes(1).toMillis(), + // MINIMUM_RATE_LIMIT_RETRY_MILLIS); } /**