Skip to content

Commit 99e3265

Browse files
bitwisemanbitwiseman
authored
Merge pull request hub4j#680 from bitwiseman/issue/ssl
Handle SSLHandshakeException with connection retry
2 parents aeb5e5f + 1dde975 commit 99e3265

204 files changed

Lines changed: 8987 additions & 180 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/main/java/org/kohsuke/github/Requester.java‎

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import javax.annotation.CheckForNull;
6464
import javax.annotation.Nonnull;
6565
import javax.annotation.WillClose;
66+
import javax.net.ssl.SSLHandshakeException;
6667

6768
import static java.util.Arrays.asList;
6869
import static java.util.logging.Level.*;
@@ -110,7 +111,7 @@ private Entry(String key, Object value) {
110111
/**
111112
* If timeout issues let's retry after milliseconds.
112113
*/
113-
private static final int retryTimeoutMillis = 500;
114+
private static final int retryTimeoutMillis = 100;
114115

115116
Requester(GitHub root) {
116117
this.root = root;
@@ -531,7 +532,8 @@ private <T> T _fetchOrRetry(SupplierThrows<T, IOException> supplier, int retries
531532
// don't wrap exception in HttpException to preserve backward compatibility
532533
throw e;
533534
} catch (IOException e) {
534-
if (!retrySocketException(e, retries)) {
535+
536+
if (!retryConnectionError(e, retries)) {
535537
throw new HttpException(responseCode, responseMessage, uc.getURL(), e);
536538
}
537539
}
@@ -541,10 +543,13 @@ private <T> T _fetchOrRetry(SupplierThrows<T, IOException> supplier, int retries
541543

542544
}
543545

544-
private boolean retrySocketException(IOException e, int retries) throws IOException {
545-
if ((e instanceof SocketException || e instanceof SocketTimeoutException) && retries > 0) {
546+
private boolean retryConnectionError(IOException e, int retries) throws IOException {
547+
// There are a range of connection errors where we want to wait a moment and just automatically retry
548+
boolean connectionError = e instanceof SocketException || e instanceof SocketTimeoutException
549+
|| e instanceof SSLHandshakeException;
550+
if (connectionError && retries > 0) {
546551
LOGGER.log(INFO,
547-
"timed out accessing " + uc.getURL() + ". Sleeping " + Requester.retryTimeoutMillis
552+
"Error while connecting to " + uc.getURL() + ". Sleeping " + Requester.retryTimeoutMillis
548553
+ " milliseconds before retrying... ; will try " + retries + " more time(s)",
549554
e);
550555
try {
@@ -558,6 +563,33 @@ private boolean retrySocketException(IOException e, int retries) throws IOExcept
558563
return false;
559564
}
560565

566+
private boolean retryInvalidCached404Response(int responseCode, int retries) throws IOException {
567+
// WORKAROUND FOR ISSUE #669:
568+
// When the Requester detects a 404 response with an ETag (only happpens when the server's 304
569+
// is bogus and would cause cache corruption), try the query again with new request header
570+
// that forces the server to not return 304 and return new data instead.
571+
//
572+
// This solution is transparent to users of this library and automatically handles a
573+
// situation that was cause insidious and hard to debug bad responses in caching
574+
// scenarios. If GitHub ever fixes their issue and/or begins providing accurate ETags to
575+
// their 404 responses, this will result in at worst two requests being made for each 404
576+
// responses. However, only the second request will count against rate limit.
577+
if (responseCode == 404 && Objects.equals(uc.getRequestMethod(), "GET") && uc.getHeaderField("ETag") != null
578+
&& !Objects.equals(uc.getRequestProperty("Cache-Control"), "no-cache") && retries > 0) {
579+
LOGGER.log(FINE,
580+
"Encountered GitHub invalid cached 404 from " + uc.getURL()
581+
+ ". Retrying with \"Cache-Control\"=\"no-cache\"...");
582+
583+
uc = setupConnection(uc.getURL());
584+
// Setting "Cache-Control" to "no-cache" stops the cache from supplying
585+
// "If-Modified-Since" or "If-None-Match" values.
586+
// This makes GitHub give us current data (not incorrectly cached data)
587+
uc.setRequestProperty("Cache-Control", "no-cache");
588+
return true;
589+
}
590+
return false;
591+
}
592+
561593
private <T> T[] concatenatePages(Class<T[]> type, List<T[]> pages, int totalLength) {
562594

563595
T[] result = type.cast(Array.newInstance(type.getComponentType(), totalLength));
@@ -957,33 +989,6 @@ private <T> T parse(Class<T> type, T instance, int timeouts) throws IOException
957989
}
958990
}
959991

960-
private boolean retryInvalidCached404Response(int responseCode, int retries) throws IOException {
961-
// WORKAROUND FOR ISSUE #669:
962-
// When the Requester detects a 404 response with an ETag (only happpens when the server's 304
963-
// is bogus and would cause cache corruption), try the query again with new request header
964-
// that forces the server to not return 304 and return new data instead.
965-
//
966-
// This solution is transparent to users of this library and automatically handles a
967-
// situation that was cause insidious and hard to debug bad responses in caching
968-
// scenarios. If GitHub ever fixes their issue and/or begins providing accurate ETags to
969-
// their 404 responses, this will result in at worst two requests being made for each 404
970-
// responses. However, only the second request will count against rate limit.
971-
if (responseCode == 404 && Objects.equals(uc.getRequestMethod(), "GET") && uc.getHeaderField("ETag") != null
972-
&& !Objects.equals(uc.getRequestProperty("Cache-Control"), "no-cache") && retries > 0) {
973-
LOGGER.log(FINE,
974-
"Encountered GitHub invalid cached 404 from " + uc.getURL()
975-
+ ". Retrying with \"Cache-Control\"=\"no-cache\"...");
976-
977-
uc = setupConnection(uc.getURL());
978-
// Setting "Cache-Control" to "no-cache" stops the cache from supplying
979-
// "If-Modified-Since" or "If-None-Match" values.
980-
// This makes GitHub give us current data (not incorrectly cached data)
981-
uc.setRequestProperty("Cache-Control", "no-cache");
982-
return true;
983-
}
984-
return false;
985-
}
986-
987992
private <T> T setResponseHeaders(T readValue) {
988993
if (readValue instanceof GHObject[]) {
989994
for (GHObject ghObject : (GHObject[]) readValue) {

0 commit comments

Comments
 (0)