diff --git a/core/src/main/java/feign/SynchronousMethodHandler.java b/core/src/main/java/feign/SynchronousMethodHandler.java index 244fe90f5d..64b81d43b4 100644 --- a/core/src/main/java/feign/SynchronousMethodHandler.java +++ b/core/src/main/java/feign/SynchronousMethodHandler.java @@ -76,10 +76,17 @@ private Object runWithRetry(Invocation invocation, Options options) throws Throw } catch (RetryableException th) { Throwable cause = th.getCause(); if (methodHandlerConfiguration.getPropagationPolicy() == UNWRAP && cause != null) { - throw cause; - } else { - throw th; + if (cause instanceof RuntimeException || cause instanceof Error) { + throw cause; + } + for (Class exceptionType : + methodHandlerConfiguration.getMetadata().method().getExceptionTypes()) { + if (exceptionType.isAssignableFrom(cause.getClass())) { + throw cause; + } + } } + throw th; } if (methodHandlerConfiguration.getLogLevel() != Logger.Level.NONE) { methodHandlerConfiguration diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java index 748bfe1ad5..47a348bfaf 100755 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -50,6 +50,7 @@ import feign.querymap.FieldQueryMapEncoder; import java.io.IOException; import java.lang.reflect.Type; +import java.net.ProtocolException; import java.net.URI; import java.time.Clock; import java.time.Instant; @@ -631,6 +632,60 @@ void throwsFeignExceptionWithoutBody() { } } + @Test + void doesNotUnwrapUndeclaredCheckedCauseWhenPropagationPolicyIsUnwrap() { + TestInterface api = + Feign.builder() + .exceptionPropagationPolicy(UNWRAP) + .retryer(new DefaultRetryer(1, 1, 1)) + .client( + (_, _) -> { + throw new ProtocolException("missing Location header for redirect"); + }) + .target(TestInterface.class, "http://localhost:" + server.getPort()); + + RetryableException exception = assertThrows(RetryableException.class, () -> api.post()); + + assertThat(exception.getMessage()).contains("missing Location header for redirect"); + assertThat(exception.getCause()).isInstanceOf(ProtocolException.class); + } + + @Test + void unwrapDeclaredCheckedCauseWhenPropagationPolicyIsUnwrap() { + TestInterface api = + Feign.builder() + .exceptionPropagationPolicy(UNWRAP) + .retryer(new DefaultRetryer(1, 1, 1)) + .client( + (_, _) -> { + throw new ProtocolException("missing Location header for redirect"); + }) + .target(TestInterface.class, "http://localhost:" + server.getPort()); + + ProtocolException exception = + assertThrows(ProtocolException.class, () -> api.postThrowsProtocolException()); + + assertThat(exception.getMessage()).contains("missing Location header for redirect"); + } + + @Test + void unwrapCheckedCauseAssignableToDeclaredTypeWhenPropagationPolicyIsUnwrap() { + TestInterface api = + Feign.builder() + .exceptionPropagationPolicy(UNWRAP) + .retryer(new DefaultRetryer(1, 1, 1)) + .client( + (_, _) -> { + throw new ProtocolException("missing Location header for redirect"); + }) + .target(TestInterface.class, "http://localhost:" + server.getPort()); + + IOException exception = assertThrows(IOException.class, () -> api.postThrowsIOException()); + + assertThat(exception).isInstanceOf(ProtocolException.class); + assertThat(exception.getMessage()).contains("missing Location header for redirect"); + } + @Test void ensureRetryerClonesItself() throws Exception { server.enqueue(new MockResponse().setResponseCode(503).setBody("foo 1")); @@ -1234,6 +1289,12 @@ interface TestInterface { @RequestLine("POST /") String post() throws TestInterfaceException; + @RequestLine("POST /") + String postThrowsProtocolException() throws ProtocolException; + + @RequestLine("POST /") + String postThrowsIOException() throws IOException; + @RequestLine("POST /") @Body( "%7B\"customer_name\": \"{customer_name}\", \"user_name\": \"{user_name}\", \"password\":" diff --git a/httpclient/src/test/java/feign/httpclient/ApacheHttpClientTest.java b/httpclient/src/test/java/feign/httpclient/ApacheHttpClientTest.java index 3506aae5be..4ab3601425 100644 --- a/httpclient/src/test/java/feign/httpclient/ApacheHttpClientTest.java +++ b/httpclient/src/test/java/feign/httpclient/ApacheHttpClientTest.java @@ -23,6 +23,8 @@ import feign.Feign.Builder; import feign.FeignException; import feign.Request.Options; +import feign.RetryableException; +import feign.Retryer; import feign.client.AbstractClientTest; import feign.jaxrs.JAXRSContract; import java.nio.charset.StandardCharsets; @@ -32,6 +34,8 @@ import javax.ws.rs.QueryParam; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.RecordedRequest; +import org.apache.http.ProtocolException; +import org.apache.http.client.ClientProtocolException; import org.apache.http.impl.client.HttpClientBuilder; import org.junit.jupiter.api.Test; @@ -43,6 +47,25 @@ public Builder newBuilder() { return Feign.builder().client(new ApacheHttpClient()); } + @Test + void redirectWithoutLocationHeaderKeepsRetryableExceptionWhenPropagationPolicyIsUnwrap() { + JaxRsTestInterface api = + Feign.builder() + .contract(new JAXRSContract()) + .exceptionPropagationPolicy(feign.ExceptionPropagationPolicy.UNWRAP) + .retryer(Retryer.NEVER_RETRY) + .client(new ApacheHttpClient(HttpClientBuilder.create().build())) + .target(JaxRsTestInterface.class, "http://localhost:" + server.getPort()); + + server.enqueue(new MockResponse().setResponseCode(303)); + + RetryableException exception = + assertThrows(RetryableException.class, () -> api.withoutBody("foo")); + + assertThat(exception.getCause()).isInstanceOf(ClientProtocolException.class); + assertThat(exception.getCause()).hasCauseInstanceOf(ProtocolException.class); + } + @Test void queryParamsAreRespectedWhenBodyIsEmpty() throws InterruptedException { final JaxRsTestInterface testInterface = buildTestInterface();