do not unwrap checked and undeclared exceptions

Author: gitshabh

core/src/main/java/feign/SynchronousMethodHandler.java

@@ -24,6 +24,7 @@
 import feign.interceptor.Invocation;
 import feign.interceptor.MethodInterceptor;
 import java.io.IOException;
+import java.lang.reflect.Method;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Stream;
@@ -75,7 +76,12 @@ private Object runWithRetry(Invocation invocation, Options options) throws Throw
           retryer.continueOrPropagate(e);
         } catch (RetryableException th) {
           Throwable cause = th.getCause();
-          if (methodHandlerConfiguration.getPropagationPolicy() == UNWRAP && cause != null) {
+          if (methodHandlerConfiguration.getPropagationPolicy() == UNWRAP
+              && cause != null
+              && (cause instanceof RuntimeException
+                  || cause instanceof Error
+                  || isDeclaredCheckedException(
+                      cause, methodHandlerConfiguration.getMetadata().method()))) {
             throw cause;
           } else {
             throw th;
@@ -160,6 +166,16 @@ Options findOptions(Object[] argv) {
                 .getMethodOptions(methodHandlerConfiguration.getMetadata().method().getName()));
   }
 
+  private static boolean isDeclaredCheckedException(Throwable cause, Method method) {
+    Class<?>[] exceptionTypes = method.getExceptionTypes();
+    for (Class<?> exType : exceptionTypes) {
+      if (exType.isAssignableFrom(cause.getClass())) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   static class Factory implements MethodHandler.Factory<Object> {
 
     private final Client client;

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;
@@ -708,6 +709,39 @@ void throwsRetryableExceptionIfNoUnderlyingCause() throws Exception {
     assertThat(exception.getMessage()).contains(message);
   }
 
+  @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");
+  }
+
+  @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 whenReturnTypeIsResponseNoErrorHandling() {
     Map<String, Collection<String>> headers = new LinkedHashMap<>();
@@ -1234,6 +1268,9 @@ interface TestInterface {
     @RequestLine("POST /")
     String post() throws TestInterfaceException;
 
+    @RequestLine("POST /")
+    String postThrowsProtocolException() throws ProtocolException;
+
     @RequestLine("POST /")
     @Body(
         "%7B\"customer_name\": \"{customer_name}\", \"user_name\": \"{user_name}\", \"password\":"

httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java

@@ -81,8 +81,12 @@ public Response execute(Request request, Request.Options options) throws IOExcep
     } catch (URISyntaxException e) {
       throw new IOException("URL '" + request.url() + "' couldn't be parsed into a URI", e);
     }
-    HttpResponse httpResponse = client.execute(httpUriRequest);
-    return toFeignResponse(httpResponse, request);
+    try {
+      HttpResponse httpResponse = client.execute(httpUriRequest);
+      return toFeignResponse(httpResponse, request);
+    } catch (Exception e) {
+      throw new IOException(e);
+    }
   }
 
   HttpUriRequest toHttpUriRequest(Request request, Request.Options options)

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;
@@ -89,6 +91,21 @@ void notFollowRedirectIsRespected() throws InterruptedException {
     assertThat(server.takeRequest().getPath()).isEqualTo("/withOptions");
   }
 
+  @Test
+  void redirectWithoutLocationHeader() {
+    JaxRsTestInterface api =
+        Feign.builder()
+            .contract(new JAXRSContract())
+            .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.getMessage()).contains("org.apache.http.client.ClientProtocolException");
+  }
+
   private JaxRsTestInterface buildTestInterface() {
     return Feign.builder()
         .contract(new JAXRSContract())