diff --git a/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/InsecureRedirectGuard.java b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/InsecureRedirectGuard.java index 06b29da0c..19753aa46 100644 --- a/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/InsecureRedirectGuard.java +++ b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/InsecureRedirectGuard.java @@ -18,8 +18,6 @@ */ package org.eclipse.aether.transport.jetty; -import java.io.IOException; - import org.eclipse.aether.ConfigurationProperties; import org.eclipse.jetty.client.Request; @@ -39,7 +37,7 @@ final class InsecureRedirectGuard implements Request.QueuedListener { @Override public void onQueued(Request request) { if (!"https".equalsIgnoreCase(request.getScheme())) { - request.abort(new IOException("Insecure redirect to '" + request.getURI() + request.abort(new RedirectRefusedException("Insecure redirect to '" + request.getURI() + "' not followed: protocol downgrade from https to " + request.getScheme() + " would expose credentials and artifact content; set the configuration property " + ConfigurationProperties.HTTP_FOLLOW_INSECURE_REDIRECT diff --git a/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyRetryHandler.java b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyRetryHandler.java new file mode 100644 index 000000000..799f64d6d --- /dev/null +++ b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyRetryHandler.java @@ -0,0 +1,158 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.eclipse.aether.transport.jetty; + +import javax.net.ssl.SSLException; + +import java.io.IOException; +import java.io.InterruptedIOException; +import java.net.ConnectException; +import java.net.NoRouteToHostException; +import java.net.UnknownHostException; +import java.util.Set; +import java.util.concurrent.TimeoutException; + +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.repository.RemoteRepository; +import org.eclipse.aether.transfer.TransferCancelledException; +import org.eclipse.aether.util.connector.transport.http.HttpTransporterUtils; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Decides whether a failed request is retried and how long to wait before doing so, based on the + * {@code aether.connector.http.retryHandler.*} configuration. The Jetty HTTP client has no retry hook of its own, + * therefore {@link JettyTransporter} drives the retries and consults this class. + */ +final class JettyRetryHandler { + + private static final Logger LOGGER = LoggerFactory.getLogger(JettyRetryHandler.class); + + /** + * Classes of IOExceptions that should not be retried (because they are permanent failures). + * Same as in + * Apache HttpClient's DefaultHttpRequestRetryHandler. + */ + private static final Set> NON_RETRIABLE_IO_EXCEPTIONS = Set.of( + InterruptedIOException.class, + UnknownHostException.class, + ConnectException.class, + NoRouteToHostException.class, + SSLException.class); + + /** + * The delay before a retry together with the reason for choosing it. + */ + record Delay(long millis, String reason) {} + + private final int retryCount; + + private final long retryInterval; + + private final long retryIntervalMax; + + private final Set retriableStatusCodes; + + JettyRetryHandler(RepositorySystemSession session, RemoteRepository repository) { + this.retryCount = HttpTransporterUtils.getHttpRetryHandlerCount(session, repository); + this.retryInterval = HttpTransporterUtils.getHttpRetryHandlerInterval(session, repository); + this.retryIntervalMax = HttpTransporterUtils.getHttpRetryHandlerIntervalMax(session, repository); + this.retriableStatusCodes = HttpTransporterUtils.getHttpServiceUnavailableCodes(session, repository); + } + + /** + * The maximum number of retries, i.e. a request is attempted at most {@code getRetryCount() + 1} times. + */ + int getRetryCount() { + return retryCount; + } + + /** + * Tells whether a request that failed with the given exception should be sent again. + * + * @param attempt the 0-based number of the attempt that failed + */ + boolean shouldRetry(int attempt, Throwable failure) { + return attempt < retryCount && isRetriable(failure); + } + + /** + * Tells whether a request that got a response with the given status code should be sent again. + * + * @param attempt the 0-based number of the attempt that got the response + */ + boolean shouldRetry(int attempt, int statusCode) { + return attempt < retryCount && retriableStatusCodes.contains(statusCode); + } + + private static boolean isRetriable(Throwable failure) { + if (failure instanceof TimeoutException) { + return true; + } + if (!(failure instanceof IOException)) { + return false; + } + for (Throwable t = failure; t != null; t = t.getCause()) { + if (t instanceof TransferCancelledException || t instanceof RedirectRefusedException) { + return false; + } + } + return NON_RETRIABLE_IO_EXCEPTIONS.stream().noneMatch(clazz -> clazz.isInstance(failure)); + } + + /** + * Computes the delay before the next attempt. A {@code Retry-After} response header is obeyed, otherwise the + * delay grows linearly with the number of attempts. It never exceeds the configured maximum. + * + * @param responseHeaders the headers of the response that triggered the retry + * @param attempt the 0-based number of the attempt that got the response + */ + Delay getDelay(HttpFields responseHeaders, int attempt) { + long delay = -1L; + String reason = ""; + String retryAfter = responseHeaders.get(HttpHeader.RETRY_AFTER); + if (retryAfter != null) { + try { + if (retryAfter.contains(":")) { // is date when to retry + delay = Math.max( + responseHeaders.getDateField(HttpHeader.RETRY_AFTER) - System.currentTimeMillis(), 0L); + reason = "Retry-After header with date value " + retryAfter; + } else { + delay = Long.parseLong(retryAfter.trim()) * 1000L; // is in seconds + reason = "Retry-After header with seconds value " + retryAfter; + } + } catch (IllegalArgumentException e) { // includes NumberFormatException + LOGGER.warn("Invalid Retry-After header value {}", retryAfter); + delay = -1L; + } + } + if (delay < 0L) { + long executionCount = attempt + 1L; + delay = executionCount * retryInterval; + reason = "retryInterval " + retryInterval + " ms multiplied by execution count " + executionCount; + } + if (delay > retryIntervalMax) { + delay = retryIntervalMax; + reason += ", capped at max value"; + } + return new Delay(delay, reason); + } +} diff --git a/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyTransporter.java b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyTransporter.java index 835f9db94..b51e85a2d 100644 --- a/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyTransporter.java +++ b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyTransporter.java @@ -36,6 +36,7 @@ import java.util.TreeSet; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; @@ -85,6 +86,8 @@ import org.eclipse.jetty.quic.quiche.client.QuicheClientQuicConfiguration; import org.eclipse.jetty.quic.quiche.client.QuicheTransport; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static org.eclipse.aether.spi.connector.transport.http.HttpConstants.ACCEPT_ENCODING; import static org.eclipse.aether.spi.connector.transport.http.HttpConstants.CONTENT_LENGTH; @@ -103,6 +106,8 @@ * @since 2.0.0 */ final class JettyTransporter extends AbstractTransporter implements HttpTransporter { + private static final Logger LOGGER = LoggerFactory.getLogger(JettyTransporter.class); + private static final long MODIFICATION_THRESHOLD = 60L * 1000L; private final RepositorySystemSession session; @@ -121,6 +126,8 @@ final class JettyTransporter extends AbstractTransporter implements HttpTranspor private final int requestTimeout; + private final JettyRetryHandler retryHandler; + private final Map headers; private final boolean preemptiveAuth; @@ -165,6 +172,7 @@ final class JettyTransporter extends AbstractTransporter implements HttpTranspor this.connectTimeout = HttpTransporterUtils.getHttpConnectTimeout(session, repository); this.requestTimeout = HttpTransporterUtils.getHttpRequestTimeout(session, repository); + this.retryHandler = new JettyRetryHandler(session, repository); this.preemptiveAuth = HttpTransporterUtils.isHttpPreemptiveAuth(session, repository); this.preemptivePutAuth = HttpTransporterUtils.isHttpPreemptivePutAuth(session, repository); this.sendRfc9457Accept = HttpTransporterUtils.isHttpSendRfc9457Accept(session, repository); @@ -189,25 +197,73 @@ private URI resolve(TransportTask task) { return baseUri.resolve(task.getLocation()); } + /** + * Logs that the request is sent again right away, after the given attempt failed without a (retriable) response. + */ + private void logImmediateRetry(Request request, int attempt, Throwable failure) { + HttpTransporter.logImmediateRetry( + LOGGER, request.getMethod(), request.getURI(), attempt, retryHandler.getRetryCount() + 1, failure); + } + + /** + * Logs and waits until the request is sent again, after the given attempt got a retriable response. + */ + private void waitBeforeRetry(Request request, int attempt, Response response) throws InterruptedException { + JettyRetryHandler.Delay delay = retryHandler.getDelay(response.getHeaders(), attempt); + HttpTransporter.logRetry( + LOGGER, + request.getMethod(), + request.getURI(), + attempt, + retryHandler.getRetryCount() + 1, + "status " + response.getStatus(), + delay.millis(), + delay.reason()); + if (delay.millis() > 0L) { + Thread.sleep(delay.millis()); + } + } + @Override protected void implPeek(PeekTask task) throws Exception { - Request request = client.newRequest(resolve(task)).method("HEAD"); - request.headers(m -> headers.forEach(m::add)); - if (preemptiveAuth) { - mayApplyPreemptiveAuth(request); - } - // capture raw response headers as described in https://github.com/jetty/jetty.project/discussions/14404 - Map rawResponseHeaders = new HashMap<>(); - request.onResponseHeader((r, field) -> { - rawResponseHeaders.put(field.getLowerCaseName(), field); - return true; // continue processing - }); - Response response = request.send(); - Map transportProperties = - createTransportProperties(request, rawResponseHeaders); - task.getListener().transportPropertiesAvailable(transportProperties); - if (response.getStatus() >= MULTIPLE_CHOICES) { - throw new HttpTransporterException(response.getStatus()); + for (int attempt = 0; ; attempt++) { + Request request = client.newRequest(resolve(task)).method("HEAD"); + request.headers(m -> headers.forEach(m::add)); + if (preemptiveAuth) { + mayApplyPreemptiveAuth(request); + } + // capture raw response headers as described in https://github.com/jetty/jetty.project/discussions/14404 + Map rawResponseHeaders = new HashMap<>(); + request.onResponseHeader((r, field) -> { + rawResponseHeaders.put(field.getLowerCaseName(), field); + return true; // continue processing + }); + Response response; + try { + response = request.send(); + } catch (ExecutionException | TimeoutException e) { + Throwable failure = e instanceof ExecutionException ? e.getCause() : e; + if (retryHandler.shouldRetry(attempt, failure)) { + logImmediateRetry(request, attempt, failure); + continue; + } + if (failure instanceof Exception) { + throw (Exception) failure; + } else { + throw new RuntimeException(failure); + } + } + if (retryHandler.shouldRetry(attempt, response.getStatus())) { + waitBeforeRetry(request, attempt, response); + continue; + } + Map transportProperties = + createTransportProperties(request, rawResponseHeaders); + task.getListener().transportPropertiesAvailable(transportProperties); + if (response.getStatus() >= MULTIPLE_CHOICES) { + throw new HttpTransporterException(response.getStatus()); + } + return; } } @@ -217,6 +273,7 @@ protected void implGet(GetTask task) throws Exception { Response response; InputStreamResponseListener listener; + int attempt = 0; while (true) { Request request = client.newRequest(resolve(task)).method("GET"); request.headers(m -> headers.forEach(m::add)); @@ -248,14 +305,27 @@ protected void implGet(GetTask task) throws Exception { request.send(listener); try { response = listener.get(requestTimeout, TimeUnit.MILLISECONDS); - } catch (ExecutionException e) { - Throwable t = e.getCause(); + } catch (ExecutionException | TimeoutException e) { + Throwable t = e instanceof ExecutionException ? e.getCause() : e; + if (retryHandler.shouldRetry(attempt, t)) { + request.abort(t); // no-op if the request already failed + listener.close(); + logImmediateRetry(request, attempt, t); + attempt++; + continue; + } if (t instanceof Exception) { throw (Exception) t; } else { throw new RuntimeException(t); } } + if (retryHandler.shouldRetry(attempt, response.getStatus())) { + listener.close(); // discards the response body + waitBeforeRetry(request, attempt, response); + attempt++; + continue; + } Map transportProperties = createTransportProperties(request, rawResponseHeaders); task.getListener().transportPropertiesAvailable(transportProperties); @@ -360,6 +430,19 @@ private static Function headerGetter(Response response) { @Override protected void implPut(PutTask task) throws Exception { + int attempt = 0; + while (!put(task, attempt)) { + attempt++; + } + } + + /** + * Sends the PUT request once. + * + * @param attempt the 0-based number of this attempt + * @return {@code true} if the upload is complete, {@code false} if it failed in a way that is worth another attempt + */ + private boolean put(PutTask task, int attempt) throws Exception { Request request = client.newRequest(resolve(task)).method("PUT"); request.headers(m -> headers.forEach(m::add)); if (sendRfc9457Accept) { @@ -432,18 +515,26 @@ protected void implPut(PutTask task) throws Exception { } } } - // transportPropertiesAvailable is called after transportStarted/transportProgressed - // to honour the TransportListener contract: "for remote put tasks this is called - // after transportStarted(long, long)". Placed before the status check so it is - // reachable on both success (2xx) and error (3xx/4xx/5xx) responses. - task.getListener().transportPropertiesAvailable(createTransportProperties(request, rawResponseHeaders)); - if (response.getStatus() >= MULTIPLE_CHOICES) { - JettyRFC9457Reporter.INSTANCE.generateException(listener, (statusCode, reasonPhrase) -> { - throw new HttpTransporterException(statusCode); - }); + if (!retryHandler.shouldRetry(attempt, response.getStatus())) { + // transportPropertiesAvailable is called after transportStarted/transportProgressed + // to honour the TransportListener contract: "for remote put tasks this is called + // after transportStarted(long, long)". Placed before the status check so it is + // reachable on both success (2xx) and error (3xx/4xx/5xx) responses. + task.getListener().transportPropertiesAvailable(createTransportProperties(request, rawResponseHeaders)); + if (response.getStatus() >= MULTIPLE_CHOICES) { + JettyRFC9457Reporter.INSTANCE.generateException(listener, (statusCode, reasonPhrase) -> { + throw new HttpTransporterException(statusCode); + }); + } + return true; + } + } catch (ExecutionException | TimeoutException e) { + Throwable t = e instanceof ExecutionException ? e.getCause() : e; + if (retryHandler.shouldRetry(attempt, t)) { + request.abort(t); // no-op if the request already failed + logImmediateRetry(request, attempt, t); + return false; } - } catch (ExecutionException e) { - Throwable t = e.getCause(); if (t instanceof IOException ioex) { if (ioex.getCause() instanceof TransferCancelledException) { throw (TransferCancelledException) ioex.getCause(); @@ -456,6 +547,9 @@ protected void implPut(PutTask task) throws Exception { throw new RuntimeException(t); } } + // the listener is closed by now, which discards the response body + waitBeforeRetry(request, attempt, response); + return false; } @Override diff --git a/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/RedirectRefusedException.java b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/RedirectRefusedException.java new file mode 100644 index 000000000..125797bc8 --- /dev/null +++ b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/RedirectRefusedException.java @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.eclipse.aether.transport.jetty; + +import java.io.IOException; + +/** + * Signals that a redirect was refused on purpose. Sending the request again would just lead to the same redirect, + * therefore this failure is never retried. + */ +final class RedirectRefusedException extends IOException { + private static final long serialVersionUID = 1L; + + RedirectRefusedException(String message) { + super(message); + } +} diff --git a/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/SameAuthorityRedirectGuard.java b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/SameAuthorityRedirectGuard.java index bd85c66c6..e94e34494 100644 --- a/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/SameAuthorityRedirectGuard.java +++ b/maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/SameAuthorityRedirectGuard.java @@ -18,7 +18,6 @@ */ package org.eclipse.aether.transport.jetty; -import java.io.IOException; import java.net.URI; import org.eclipse.aether.util.connector.transport.TransporterUtils; @@ -35,7 +34,8 @@ final class SameAuthorityRedirectGuard implements Request.QueuedListener { public void onQueued(Request request) { URI target = request.getURI(); if (target == null || !TransporterUtils.isSameAuthority(repositoryUri, target)) { - request.abort(new IOException("Refusing to follow redirect to different authority: " + target)); + request.abort( + new RedirectRefusedException("Refusing to follow redirect to different authority: " + target)); } } } diff --git a/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyRetryHandlerTest.java b/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyRetryHandlerTest.java new file mode 100644 index 000000000..fe90824ec --- /dev/null +++ b/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyRetryHandlerTest.java @@ -0,0 +1,179 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.eclipse.aether.transport.jetty; + +import javax.net.ssl.SSLHandshakeException; + +import java.io.EOFException; +import java.io.IOException; +import java.io.InterruptedIOException; +import java.net.ConnectException; +import java.net.UnknownHostException; +import java.nio.channels.ClosedChannelException; +import java.time.Instant; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.util.concurrent.TimeoutException; + +import org.eclipse.aether.ConfigurationProperties; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.internal.test.util.TestUtils; +import org.eclipse.aether.repository.RemoteRepository; +import org.eclipse.aether.transfer.TransferCancelledException; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class JettyRetryHandlerTest { + private final DefaultRepositorySystemSession session = TestUtils.newSession(); + + private final RemoteRepository repository = + new RemoteRepository.Builder("test", "default", "http://localhost/repo").build(); + + private JettyRetryHandler newHandler() { + return new JettyRetryHandler(session, repository); + } + + private static HttpFields retryAfter(String value) { + return HttpFields.build().put(HttpHeader.RETRY_AFTER, value).asImmutable(); + } + + @Test + void retriesAreLimitedByRetryCount() { + session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 2); + JettyRetryHandler handler = newHandler(); + assertEquals(2, handler.getRetryCount()); + assertTrue(handler.shouldRetry(0, 503)); + assertTrue(handler.shouldRetry(1, 503)); + assertFalse(handler.shouldRetry(2, 503)); + assertTrue(handler.shouldRetry(1, new EOFException())); + assertFalse(handler.shouldRetry(2, new EOFException())); + } + + @Test + void noRetriesIfDisabled() { + session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 0); + JettyRetryHandler handler = newHandler(); + assertFalse(handler.shouldRetry(0, 503)); + assertFalse(handler.shouldRetry(0, new EOFException())); + } + + @Test + void onlyConfiguredStatusCodesAreRetried() { + JettyRetryHandler handler = newHandler(); + assertTrue(handler.shouldRetry(0, 429)); + assertTrue(handler.shouldRetry(0, 503)); + assertFalse(handler.shouldRetry(0, 404)); + assertFalse(handler.shouldRetry(0, 500)); + + session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE, "500,502"); + handler = newHandler(); + assertTrue(handler.shouldRetry(0, 500)); + assertTrue(handler.shouldRetry(0, 502)); + assertFalse(handler.shouldRetry(0, 503)); + } + + @Test + void transientFailuresAreRetried() { + JettyRetryHandler handler = newHandler(); + assertTrue(handler.shouldRetry(0, new EOFException())); + assertTrue(handler.shouldRetry(0, new ClosedChannelException())); + assertTrue(handler.shouldRetry(0, new IOException("some failure"))); + assertTrue(handler.shouldRetry(0, new TimeoutException("Idle timeout expired"))); + } + + @Test + void permanentFailuresAreNotRetried() { + JettyRetryHandler handler = newHandler(); + assertFalse(handler.shouldRetry(0, new UnknownHostException())); + assertFalse(handler.shouldRetry(0, new ConnectException())); + assertFalse(handler.shouldRetry(0, new InterruptedIOException())); + assertFalse(handler.shouldRetry(0, new SSLHandshakeException("bad certificate"))); + assertFalse(handler.shouldRetry(0, new IllegalStateException())); + } + + @Test + void cancelledTransferIsNotRetried() { + JettyRetryHandler handler = newHandler(); + assertFalse(handler.shouldRetry(0, new IOException(new TransferCancelledException()))); + } + + @Test + void refusedRedirectIsNotRetried() { + JettyRetryHandler handler = newHandler(); + assertFalse(handler.shouldRetry(0, new RedirectRefusedException("refused"))); + assertFalse(handler.shouldRetry(0, new IOException(new RedirectRefusedException("refused")))); + } + + @Test + void delayGrowsLinearlyWithAttempts() { + session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL, 100L); + JettyRetryHandler handler = newHandler(); + assertEquals(100L, handler.getDelay(HttpFields.build(), 0).millis()); + assertEquals(200L, handler.getDelay(HttpFields.build(), 1).millis()); + assertEquals(300L, handler.getDelay(HttpFields.build(), 2).millis()); + assertEquals( + "retryInterval 100 ms multiplied by execution count 2", + handler.getDelay(HttpFields.build(), 1).reason()); + } + + @Test + void retryAfterSecondsIsObeyed() { + session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL, 5000L); + JettyRetryHandler.Delay delay = newHandler().getDelay(retryAfter("2"), 0); + assertEquals(2000L, delay.millis()); + assertEquals("Retry-After header with seconds value 2", delay.reason()); + } + + @Test + void retryAfterDateIsObeyed() { + String date = DateTimeFormatter.RFC_1123_DATE_TIME.format( + Instant.now().plusSeconds(60).atOffset(ZoneOffset.UTC)); + JettyRetryHandler.Delay delay = newHandler().getDelay(retryAfter(date), 0); + assertTrue(delay.millis() > 50_000L && delay.millis() <= 60_000L, "Unexpected delay " + delay.millis()); + assertEquals("Retry-After header with date value " + date, delay.reason()); + } + + @Test + void retryAfterDateInThePastMeansNoDelay() { + String date = DateTimeFormatter.RFC_1123_DATE_TIME.format( + Instant.now().minusSeconds(60).atOffset(ZoneOffset.UTC)); + assertEquals(0L, newHandler().getDelay(retryAfter(date), 0).millis()); + } + + @Test + void invalidRetryAfterFallsBackToRetryInterval() { + session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL, 100L); + JettyRetryHandler handler = newHandler(); + assertEquals(100L, handler.getDelay(retryAfter("soon"), 0).millis()); + assertEquals(100L, handler.getDelay(retryAfter("Not a date: really"), 0).millis()); + } + + @Test + void delayIsCappedAtMaximum() { + session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL_MAX, 1500L); + JettyRetryHandler.Delay delay = newHandler().getDelay(retryAfter("3600"), 0); + assertEquals(1500L, delay.millis()); + assertEquals("Retry-After header with seconds value 3600, capped at max value", delay.reason()); + } +} diff --git a/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyTransporterTest.java b/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyTransporterTest.java index a37742759..d459d93ca 100644 --- a/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyTransporterTest.java +++ b/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyTransporterTest.java @@ -18,14 +18,23 @@ */ package org.eclipse.aether.transport.jetty; +import java.io.File; +import java.net.URI; import java.util.stream.Stream; import org.eclipse.aether.ConfigurationProperties; +import org.eclipse.aether.internal.test.util.TestFileUtils; import org.eclipse.aether.internal.test.util.http.HttpTransporterTest; +import org.eclipse.aether.spi.connector.transport.GetTask; +import org.eclipse.aether.spi.connector.transport.PutTask; +import org.eclipse.aether.spi.connector.transport.http.HttpTransporterException; import org.eclipse.aether.spi.io.PathProcessorSupport; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + /** * Jetty transporter UT. */ @@ -36,11 +45,6 @@ protected Stream supportedCompressionAlgorithms() { return Stream.of("gzip", "deflate", "br", "zstd"); } - @Override - protected boolean supportsRetryLogging() { - return false; // Jetty does not yet support configurable retries - } - @Override @Disabled @Test @@ -56,31 +60,6 @@ protected void testPut_ProxyUnauthenticated() {} @Test protected void testPut_Unauthenticated() {} - @Override - @Disabled - @Test - protected void testRetryHandler_defaultCount_positive() {} - - @Override - @Disabled - @Test - protected void testRetryHandler_explicitCount_positive() {} - - @Override - @Disabled - @Test - protected void testRetryHandler_tooManyRequests_explicitCount_positive() {} - - @Override - @Disabled - @Test - protected void testRetryHandler_tooManyRequests_withRetryAfterHeader_explicitCount_positive() {} - - @Override - @Disabled - @Test - protected void testRetryHandler_tooManyRequests_explicitCount_negative() {} - @Override @Disabled @Test @@ -113,6 +92,72 @@ protected void testGet_HTTP3() throws Exception { super.testGet_HTTP3(); } + // the retry tests of the base class only cover HEAD requests + + private void configureQuickRetries(int retryCount) throws Exception { + session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, retryCount); + session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL, 1); + newTransporter(httpServer.getHttpUrl()); + } + + @Test + void testGet_RetriesAfterServiceUnavailable() throws Exception { + configureQuickRetries(2); + httpServer.setServerErrorsBeforeWorks(2, 503); + GetTask task = new GetTask(URI.create("repo/file.txt")); + transporter.get(task); + assertEquals("test", task.getDataString()); + assertEquals(3, httpServer.getLogEntries().size()); + } + + @Test + void testGet_RetriesAfterClosedConnection() throws Exception { + configureQuickRetries(2); + httpServer.setConnectionsToClose(2); + GetTask task = new GetTask(URI.create("repo/file.txt")); + transporter.get(task); + assertEquals("test", task.getDataString()); + } + + @Test + void testGet_GivesUpAfterRetryCount() throws Exception { + configureQuickRetries(2); + httpServer.setServerErrorsBeforeWorks(3, 503); + HttpTransporterException e = assertThrows( + HttpTransporterException.class, () -> transporter.get(new GetTask(URI.create("repo/file.txt")))); + assertEquals(503, e.getStatusCode()); + assertEquals(3, httpServer.getLogEntries().size()); + } + + @Test + void testPut_RetriesAfterServiceUnavailable() throws Exception { + configureQuickRetries(2); + httpServer.setServerErrorsBeforeWorks(2, 503); + transporter.put(new PutTask(URI.create("repo/file.txt")).setDataString("upload")); + assertEquals("upload", TestFileUtils.readString(new File(repoDir, "file.txt"))); + assertEquals(3, httpServer.getLogEntries().size()); + } + + @Test + void testPut_RetriesAfterClosedConnection() throws Exception { + configureQuickRetries(2); + httpServer.setConnectionsToClose(2); + // the file is not checked: the server handlers of the two closed connections may still delete it + transporter.put(new PutTask(URI.create("repo/file.txt")).setDataString("upload")); + } + + @Test + void testPut_GivesUpAfterRetryCount() throws Exception { + configureQuickRetries(1); + httpServer.setServerErrorsBeforeWorks(2, 503); + HttpTransporterException e = assertThrows( + HttpTransporterException.class, + () -> transporter.put(new PutTask(URI.create("repo/file.txt")).setDataString("upload"))); + assertEquals(503, e.getStatusCode()); + assertEquals(2, httpServer.getLogEntries().size()); + assertEquals("test", TestFileUtils.readString(new File(repoDir, "file.txt"))); + } + public JettyTransporterTest() { super(() -> new JettyTransporterFactory(standardChecksumExtractor(), new PathProcessorSupport())); } diff --git a/src/site/markdown/transporter-known-issues.md b/src/site/markdown/transporter-known-issues.md index a6173b5bb..b0d5106fc 100644 --- a/src/site/markdown/transporter-known-issues.md +++ b/src/site/markdown/transporter-known-issues.md @@ -66,7 +66,6 @@ you need to add `org.apache.maven.resolver.transport:transport-http-jetty` artif This situation is probably improving with Jetty 13 (which is supposed to be shipping with a native Java QUIC implementation). * When HTTP/3 is configured there is [no fallback to lower versions](https://github.com/jetty/jetty.project/issues/15423). The request will just time out. -* Does not support configurable retries. ## The `url` (Consumer Only) Transporter