diff --git a/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java b/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java index cd37d6b3a..33887cad0 100644 --- a/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java +++ b/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java @@ -4,13 +4,14 @@ 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; +import static java.net.HttpURLConnection.HTTP_FORBIDDEN; + // TODO: Auto-generated Javadoc /** * Pluggable strategy to determine what to do when the API rate limit is reached. @@ -56,7 +57,7 @@ private boolean isTooManyRequests(GitHubConnectorResponse connectorResponse) { * @return true if the status code is HTTP_FORBIDDEN */ private boolean isForbidden(GitHubConnectorResponse connectorResponse) { - return connectorResponse.statusCode() == HttpURLConnection.HTTP_FORBIDDEN; + return connectorResponse.statusCode() == HTTP_FORBIDDEN; } /** diff --git a/src/main/java/org/kohsuke/github/GitHubClient.java b/src/main/java/org/kohsuke/github/GitHubClient.java index 91012fdd9..38552b573 100644 --- a/src/main/java/org/kohsuke/github/GitHubClient.java +++ b/src/main/java/org/kohsuke/github/GitHubClient.java @@ -22,11 +22,15 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; -import javax.net.ssl.SSLHandshakeException; import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY; import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE; -import static java.net.HttpURLConnection.*; +import static java.net.HttpURLConnection.HTTP_ACCEPTED; +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.net.HttpURLConnection.HTTP_MOVED_PERM; +import static java.net.HttpURLConnection.HTTP_MOVED_TEMP; +import static java.net.HttpURLConnection.HTTP_NOT_MODIFIED; +import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED; import static java.util.logging.Level.*; import static org.apache.commons.lang3.StringUtils.defaultString; @@ -440,13 +444,6 @@ public GitHubResponse sendRequest(GitHubRequest request, @CheckForNull Bo if (retries > 0 && e.connectorRequest != null) { connectorRequest = e.connectorRequest; } - } catch (SocketException | SocketTimeoutException | SSLHandshakeException e) { - // These transient errors thrown by HttpURLConnection - if (retries > 0) { - logRetryConnectionError(e, connectorRequest.url(), retries); - continue; - } - throw interpretApiError(e, connectorRequest, connectorResponse); } catch (IOException e) { throw interpretApiError(e, connectorRequest, connectorResponse); } finally { @@ -656,10 +653,10 @@ private static GitHubResponse createResponse(@Nonnull GitHubConnectorResp } private static boolean shouldIgnoreBody(@Nonnull GitHubConnectorResponse connectorResponse) { - if (connectorResponse.statusCode() == HttpURLConnection.HTTP_NOT_MODIFIED) { + if (connectorResponse.statusCode() == HTTP_NOT_MODIFIED) { // special case handling for 304 unmodified, as the content will be "" return true; - } else if (connectorResponse.statusCode() == HttpURLConnection.HTTP_ACCEPTED) { + } else if (connectorResponse.statusCode() == HTTP_ACCEPTED) { // Response code 202 means data is being generated or an action that can require some time is triggered. // This happens in specific cases: diff --git a/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java b/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java index 5426cdac7..24ea47f5f 100644 --- a/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java +++ b/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java @@ -5,10 +5,11 @@ import java.io.IOException; import java.io.InterruptedIOException; -import java.net.HttpURLConnection; import javax.annotation.Nonnull; +import static java.net.HttpURLConnection.HTTP_FORBIDDEN; + // TODO: Auto-generated Javadoc /** * Pluggable strategy to determine what to do when the API rate limit is reached. @@ -31,7 +32,7 @@ public abstract class GitHubRateLimitHandler extends GitHubConnectorResponseErro */ @Override boolean isError(@NotNull GitHubConnectorResponse connectorResponse) throws IOException { - return connectorResponse.statusCode() == HttpURLConnection.HTTP_FORBIDDEN + return connectorResponse.statusCode() == HTTP_FORBIDDEN && "0".equals(connectorResponse.header("X-RateLimit-Remaining")); } diff --git a/src/main/java/org/kohsuke/github/GitHubResponse.java b/src/main/java/org/kohsuke/github/GitHubResponse.java index 3ec2e2b9f..e356952a9 100644 --- a/src/main/java/org/kohsuke/github/GitHubResponse.java +++ b/src/main/java/org/kohsuke/github/GitHubResponse.java @@ -10,7 +10,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.lang.reflect.Array; -import java.net.HttpURLConnection; import java.nio.charset.StandardCharsets; import java.util.*; import java.util.logging.Level; @@ -19,6 +18,9 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import static java.net.HttpURLConnection.HTTP_NO_CONTENT; + + // TODO: Auto-generated Javadoc /** * A GitHubResponse @@ -86,7 +88,7 @@ class GitHubResponse { @CheckForNull static T parseBody(GitHubConnectorResponse connectorResponse, Class type) throws IOException { - if (connectorResponse.statusCode() == HttpURLConnection.HTTP_NO_CONTENT) { + if (connectorResponse.statusCode() == HTTP_NO_CONTENT) { if (type != null && type.isArray()) { // no content for array should be empty array return type.cast(Array.newInstance(type.getComponentType(), 0)); diff --git a/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java b/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java index 12a68ee6c..cc2420b23 100644 --- a/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java +++ b/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java @@ -9,7 +9,6 @@ import java.io.IOException; import java.io.InputStream; -import java.net.HttpURLConnection; import java.nio.charset.StandardCharsets; import java.util.Map; @@ -67,14 +66,11 @@ protected WireMockConfiguration getWireMockOptions() { public void testHandler_Fail() throws Exception { // Customized response that templates the date to keep things working snapshotNotAllowed(); - final HttpURLConnection[] savedConnection = new HttpURLConnection[1]; gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()) .withAbuseLimitHandler(new GitHubAbuseLimitHandler() { @Override public void onError(@NotNull GitHubConnectorResponse connectorResponse) throws IOException { - savedConnection[0] = null; - HttpURLConnection uc = null; // Verify // assertThat(GitHubClient.parseInstant(connectorResponse.header("Date")).toEpochMilli(), // Matchers.greaterThanOrEqualTo(new Date().getTime() - 10000)); diff --git a/src/test/java/org/kohsuke/github/RequesterRetryTest.java b/src/test/java/org/kohsuke/github/RequesterRetryTest.java index 3d8a09a2c..37ab6aed0 100644 --- a/src/test/java/org/kohsuke/github/RequesterRetryTest.java +++ b/src/test/java/org/kohsuke/github/RequesterRetryTest.java @@ -15,10 +15,8 @@ import org.kohsuke.github.connector.GitHubConnectorResponse; import org.kohsuke.github.extras.HttpClientGitHubConnector; import org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector; -import org.kohsuke.github.internal.DefaultGitHubConnector; import java.io.*; -import java.net.HttpURLConnection; import java.net.SocketException; import java.net.SocketTimeoutException; import java.net.URL; @@ -34,7 +32,6 @@ import javax.annotation.Nonnull; import javax.net.ssl.SSLHandshakeException; -import static com.github.tomakehurst.wiremock.client.WireMock.*; import static org.hamcrest.Matchers.*; // TODO: Auto-generated Javadoc @@ -51,7 +48,7 @@ public class RequesterRetryTest extends AbstractGitHubWireMockTest { private static StreamHandler customLogHandler; /** The connection. */ - HttpURLConnection connection; + //HttpURLConnection connection; /** The base request count. */ int baseRequestCount; @@ -148,79 +145,79 @@ public void testGitHubIsApiUrlValid() throws Exception { assertThat(capturedLog, not(containsString("leaked"))); } - /** - * Test socket connection and retry. - * - * @throws Exception - * the exception - */ - // Issue #539 - @Test - public void testSocketConnectionAndRetry() throws Exception { - // Only implemented for HttpURLConnection connectors - Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class))); - - // CONNECTION_RESET_BY_PEER errors result in two requests each - // to get this failure for "3" tries we have to do 6 queries. - this.mockGitHub.apiServer() - .stubFor(get(urlMatching(".+/branches/test/timeout")) - .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); - - this.gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()).build(); - - GHRepository repo = getRepository(); - baseRequestCount = this.mockGitHub.getRequestCount(); - try { - repo.getBranch("test/timeout"); - fail(); - } catch (Exception e) { - assertThat(e, instanceOf(HttpException.class)); - } - - String capturedLog = getTestCapturedLog(); - assertThat(capturedLog, containsString("(2 retries remaining)")); - assertThat(capturedLog, containsString("(1 retries remaining)")); - - assertThat(this.mockGitHub.getRequestCount(), equalTo(baseRequestCount + 6)); - } - - /** - * Test socket connection and retry status code. - * - * @throws Exception - * the exception - */ - // Issue #539 - @Test - public void testSocketConnectionAndRetry_StatusCode() throws Exception { - // Only implemented for HttpURLConnection connectors - Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class))); - - // CONNECTION_RESET_BY_PEER errors result in two requests each - // to get this failure for "3" tries we have to do 6 queries. - this.mockGitHub.apiServer() - .stubFor(get(urlMatching(".+/branches/test/timeout")) - .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); - - this.gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()).build(); - - baseRequestCount = this.mockGitHub.getRequestCount(); - try { - // status code is a different code path that should also be covered by this. - gitHub.createRequest() - .withUrlPath("/repos/hub4j-test-org/github-api/branches/test/timeout") - .fetchHttpStatusCode(); - fail(); - } catch (Exception e) { - assertThat(e, instanceOf(HttpException.class)); - } - - String capturedLog = getTestCapturedLog(); - assertThat(capturedLog, containsString("(2 retries remaining)")); - assertThat(capturedLog, containsString("(1 retries remaining)")); - - assertThat(this.mockGitHub.getRequestCount(), equalTo(baseRequestCount + 6)); - } + // /** + // * Test socket connection and retry. + // * + // * @throws Exception + // * the exception + // */ + // // Issue #539 + // @Test + // public void testSocketConnectionAndRetry() throws Exception { + // // Only implemented for HttpURLConnection connectors + // Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class))); + + // // CONNECTION_RESET_BY_PEER errors result in two requests each + // // to get this failure for "3" tries we have to do 6 queries. + // this.mockGitHub.apiServer() + // .stubFor(get(urlMatching(".+/branches/test/timeout")) + // .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); + + // this.gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()).build(); + + // GHRepository repo = getRepository(); + // baseRequestCount = this.mockGitHub.getRequestCount(); + // try { + // repo.getBranch("test/timeout"); + // fail(); + // } catch (Exception e) { + // assertThat(e, instanceOf(HttpException.class)); + // } + + // String capturedLog = getTestCapturedLog(); + // assertThat(capturedLog, containsString("(2 retries remaining)")); + // assertThat(capturedLog, containsString("(1 retries remaining)")); + + // assertThat(this.mockGitHub.getRequestCount(), equalTo(baseRequestCount + 6)); + // } + + // /** + // * Test socket connection and retry status code. + // * + // * @throws Exception + // * the exception + // */ + // // Issue #539 + // @Test + // public void testSocketConnectionAndRetry_StatusCode() throws Exception { + // // Only implemented for HttpURLConnection connectors + // Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class))); + + // // CONNECTION_RESET_BY_PEER errors result in two requests each + // // to get this failure for "3" tries we have to do 6 queries. + // this.mockGitHub.apiServer() + // .stubFor(get(urlMatching(".+/branches/test/timeout")) + // .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); + + // this.gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()).build(); + + // baseRequestCount = this.mockGitHub.getRequestCount(); + // try { + // // status code is a different code path that should also be covered by this. + // gitHub.createRequest() + // .withUrlPath("/repos/hub4j-test-org/github-api/branches/test/timeout") + // .fetchHttpStatusCode(); + // fail(); + // } catch (Exception e) { + // assertThat(e, instanceOf(HttpException.class)); + // } + + // String capturedLog = getTestCapturedLog(); + // assertThat(capturedLog, containsString("(2 retries remaining)")); + // assertThat(capturedLog, containsString("(1 retries remaining)")); + + // assertThat(this.mockGitHub.getRequestCount(), equalTo(baseRequestCount + 6)); + // } /** * Test socket connection and retry success. @@ -228,58 +225,58 @@ public void testSocketConnectionAndRetry_StatusCode() throws Exception { * @throws Exception * the exception */ - @Test - public void testSocketConnectionAndRetry_Success() throws Exception { - // Only implemented for HttpURLConnection connectors - Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class))); - - // CONNECTION_RESET_BY_PEER errors result in two requests each - // to get this failure for "3" tries we have to do 6 queries. - // If there are only 5 errors we succeed. - this.mockGitHub.apiServer() - .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) - .inScenario("Retry") - .whenScenarioStateIs(Scenario.STARTED) - .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) - .setNewScenarioState("Retry-1"); - this.mockGitHub.apiServer() - .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) - .inScenario("Retry") - .whenScenarioStateIs("Retry-1") - .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) - .setNewScenarioState("Retry-2"); - this.mockGitHub.apiServer() - .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) - .inScenario("Retry") - .whenScenarioStateIs("Retry-2") - .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) - .setNewScenarioState("Retry-3"); - this.mockGitHub.apiServer() - .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) - .inScenario("Retry") - .whenScenarioStateIs("Retry-3") - .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) - .setNewScenarioState("Retry-4"); - this.mockGitHub.apiServer() - .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) - .atPriority(0) - .inScenario("Retry") - .whenScenarioStateIs("Retry-4") - .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) - .setNewScenarioState("Retry-5"); - - this.gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()).build(); - - GHRepository repo = getRepository(); - baseRequestCount = this.mockGitHub.getRequestCount(); - GHBranch branch = repo.getBranch("test/timeout"); - assertThat(branch, notNullValue()); - String capturedLog = getTestCapturedLog(); - assertThat(capturedLog, containsString("(2 retries remaining)")); - assertThat(capturedLog, containsString("(1 retries remaining)")); - - assertThat(this.mockGitHub.getRequestCount(), equalTo(baseRequestCount + 6)); - } + // @Test + // public void testSocketConnectionAndRetry_Success() throws Exception { + // // Only implemented for HttpURLConnection connectors + // Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class))); + + // // CONNECTION_RESET_BY_PEER errors result in two requests each + // // to get this failure for "3" tries we have to do 6 queries. + // // If there are only 5 errors we succeed. + // this.mockGitHub.apiServer() + // .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) + // .inScenario("Retry") + // .whenScenarioStateIs(Scenario.STARTED) + // .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) + // .setNewScenarioState("Retry-1"); + // this.mockGitHub.apiServer() + // .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) + // .inScenario("Retry") + // .whenScenarioStateIs("Retry-1") + // .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) + // .setNewScenarioState("Retry-2"); + // this.mockGitHub.apiServer() + // .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) + // .inScenario("Retry") + // .whenScenarioStateIs("Retry-2") + // .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) + // .setNewScenarioState("Retry-3"); + // this.mockGitHub.apiServer() + // .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) + // .inScenario("Retry") + // .whenScenarioStateIs("Retry-3") + // .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) + // .setNewScenarioState("Retry-4"); + // this.mockGitHub.apiServer() + // .stubFor(get(urlMatching(".+/branches/test/timeout")).atPriority(0) + // .atPriority(0) + // .inScenario("Retry") + // .whenScenarioStateIs("Retry-4") + // .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))) + // .setNewScenarioState("Retry-5"); + + // this.gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()).build(); + + // GHRepository repo = getRepository(); + // baseRequestCount = this.mockGitHub.getRequestCount(); + // GHBranch branch = repo.getBranch("test/timeout"); + // assertThat(branch, notNullValue()); + // String capturedLog = getTestCapturedLog(); + // assertThat(capturedLog, containsString("(2 retries remaining)")); + // assertThat(capturedLog, containsString("(1 retries remaining)")); + + // assertThat(this.mockGitHub.getRequestCount(), equalTo(baseRequestCount + 6)); + // } /** * Test response code failure exceptions. diff --git a/src/test/resources/no-reflect-and-serialization-list b/src/test/resources/no-reflect-and-serialization-list index 8709dfa33..705a18d5e 100644 --- a/src/test/resources/no-reflect-and-serialization-list +++ b/src/test/resources/no-reflect-and-serialization-list @@ -69,18 +69,6 @@ org.kohsuke.github.extras.authorization.JwtBuilderUtil$ReflectionBuilderImpl org.kohsuke.github.extras.authorization.JWTTokenProvider org.kohsuke.github.extras.HttpClientGitHubConnector org.kohsuke.github.extras.HttpClientGitHubConnector$HttpClientGitHubConnectorResponse -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$1 -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$BufferedRequestBody -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$DelegatingHttpsURLConnection -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$OkHttpsURLConnection -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$OkHttpURLConnection -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$OkHttpURLConnection$NetworkInterceptor -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$OutputStreamRequestBody$1 -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$OutputStreamRequestBody -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$ResponseBodyInputStream -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$StreamedRequestBody -org.kohsuke.github.extras.okhttp3.ObsoleteUrlFactory$UnexpectedException org.kohsuke.github.extras.okhttp3.OkHttpConnector org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector$OkHttpGitHubConnectorResponse @@ -91,7 +79,5 @@ org.kohsuke.github.function.InputStreamFunction org.kohsuke.github.function.SupplierThrows org.kohsuke.github.internal.DefaultGitHubConnector org.kohsuke.github.internal.EnumUtils -org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter -org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter$HttpURLConnectionGitHubConnectorResponse org.kohsuke.github.internal.Previews org.kohsuke.github.EnterpriseManagedSupport \ No newline at end of file