GH-845: Add Request to Feign Exception (#1039)

Fixes #845 This change allows Feign Exceptions to be created with the original request as an optional parameter. Changes include: * Request field added to FeignException * New constructors are defended from null in request argument * Tests to check null instead of request, null message updated
OpenFeign · Aug 26, 2019 · d7db804 · d7db804
1 parent 4c0f7dc
commit d7db804
Show file tree

Hide file tree

Showing 20 changed files with 269 additions and 151 deletions.
diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java
@@ -14,6 +14,7 @@
 package feign;
 
 import static feign.Util.UTF_8;
+import static feign.Util.checkNotNull;
 import static java.lang.String.format;
 import java.io.IOException;
 
@@ -22,30 +23,67 @@
  */
 public class FeignException extends RuntimeException {
 
+  private static final String EXCEPTION_MESSAGE_TEMPLATE_NULL_REQUEST = "request should not be null";
   private static final long serialVersionUID = 0;
   private int status;
   private byte[] content;
+  private Request request;
 
   protected FeignException(int status, String message, Throwable cause) {
     super(message, cause);
     this.status = status;
+    this.request = null;
   }
 
   protected FeignException(int status, String message, Throwable cause, byte[] content) {
     super(message, cause);
     this.status = status;
     this.content = content;
+    this.request = null;
   }
 
   protected FeignException(int status, String message) {
     super(message);
     this.status = status;
+    this.request = null;
   }
 
   protected FeignException(int status, String message, byte[] content) {
     super(message);
     this.status = status;
     this.content = content;
+    this.request = null;
+  }
+
+  protected FeignException(int status, String message, Request request, Throwable cause) {
+    super(message, cause);
+    this.status = status;
+    this.request = checkRequestNotNull(request);
+  }
+
+  protected FeignException(int status, String message, Request request, Throwable cause,
+      byte[] content) {
+    super(message, cause);
+    this.status = status;
+    this.content = content;
+    this.request = checkRequestNotNull(request);
+  }
+
+  protected FeignException(int status, String message, Request request) {
+    super(message);
+    this.status = status;
+    this.request = checkRequestNotNull(request);
+  }
+
+  protected FeignException(int status, String message, Request request, byte[] content) {
+    super(message);
+    this.status = status;
+    this.content = content;
+    this.request = checkRequestNotNull(request);
+  }
+
+  private Request checkRequestNotNull(Request request) {
+    return checkNotNull(request, EXCEPTION_MESSAGE_TEMPLATE_NULL_REQUEST);
   }
 
   public int status() {
@@ -56,6 +94,14 @@ public byte[] content() {
     return this.content;
   }
 
+  public Request request() {
+    return this.request;
+  }
+
+  public boolean hasRequest() {
+    return (this.request != null);
+  }
+
   public String contentUTF8() {
     if (content != null) {
       return new String(content, UTF_8);
@@ -68,6 +114,7 @@ static FeignException errorReading(Request request, Response response, IOExcepti
     return new FeignException(
         response.status(),
         format("%s reading %s %s", cause.getMessage(), request.httpMethod(), request.url()),
+        request,
         cause,
         request.requestBody().asBytes());
   }
@@ -83,70 +130,79 @@ public static FeignException errorStatus(String methodKey, Response response) {
     } catch (IOException ignored) { // NOPMD
     }
 
-    return errorStatus(response.status(), message, body);
+    return errorStatus(response.status(), message, response.request(), body);
   }
 
-  private static FeignException errorStatus(int status, String message, byte[] body) {
+  private static FeignException errorStatus(int status,
+                                            String message,
+                                            Request request,
+                                            byte[] body) {
     if (isClientError(status)) {
-      return clientErrorStatus(status, message, body);
+      return clientErrorStatus(status, message, request, body);
     }
     if (isServerError(status)) {
-      return serverErrorStatus(status, message, body);
+      return serverErrorStatus(status, message, request, body);
     }
-    return new FeignException(status, message, body);
+    return new FeignException(status, message, request, body);
   }
 
   private static boolean isClientError(int status) {
     return status >= 400 && status < 500;
   }
 
-  private static FeignClientException clientErrorStatus(int status, String message, byte[] body) {
+  private static FeignClientException clientErrorStatus(int status,
+                                                        String message,
+                                                        Request request,
+                                                        byte[] body) {
     switch (status) {
       case 400:
-        return new BadRequest(message, body);
+        return new BadRequest(message, request, body);
       case 401:
-        return new Unauthorized(message, body);
+        return new Unauthorized(message, request, body);
       case 403:
-        return new Forbidden(message, body);
+        return new Forbidden(message, request, body);
       case 404:
-        return new NotFound(message, body);
+        return new NotFound(message, request, body);
       case 405:
-        return new MethodNotAllowed(message, body);
+        return new MethodNotAllowed(message, request, body);
       case 406:
-        return new NotAcceptable(message, body);
+        return new NotAcceptable(message, request, body);
       case 409:
-        return new Conflict(message, body);
+        return new Conflict(message, request, body);
       case 410:
-        return new Gone(message, body);
+        return new Gone(message, request, body);
       case 415:
-        return new UnsupportedMediaType(message, body);
+        return new UnsupportedMediaType(message, request, body);
       case 429:
-        return new TooManyRequests(message, body);
+        return new TooManyRequests(message, request, body);
       case 422:
-        return new UnprocessableEntity(message, body);
+        return new UnprocessableEntity(message, request, body);
       default:
-        return new FeignClientException(status, message, body);
+        return new FeignClientException(status, message, request, body);
     }
   }
 
   private static boolean isServerError(int status) {
     return status >= 500 && status <= 599;
   }
 
-  private static FeignServerException serverErrorStatus(int status, String message, byte[] body) {
+  private static FeignServerException serverErrorStatus(int status,
+                                                        String message,
+                                                        Request request,
+                                                        byte[] body) {
     switch (status) {
       case 500:
-        return new InternalServerError(message, body);
+        return new InternalServerError(message, request, body);
       case 501:
-        return new NotImplemented(message, body);
+        return new NotImplemented(message, request, body);
       case 502:
-        return new BadGateway(message, body);
+        return new BadGateway(message, request, body);
       case 503:
-        return new ServiceUnavailable(message, body);
+        return new ServiceUnavailable(message, request, body);
       case 504:
-        return new GatewayTimeout(message, body);
+        return new GatewayTimeout(message, request, body);
       default:
-        return new FeignServerException(status, message, body);
+        return new FeignServerException(status, message, request, body);
     }
   }
 
@@ -156,114 +212,114 @@ static FeignException errorExecuting(Request request, IOException cause) {
         format("%s executing %s %s", cause.getMessage(), request.httpMethod(), request.url()),
         request.httpMethod(),
         cause,
-        null);
+        null, request);
   }
 
   public static class FeignClientException extends FeignException {
-    public FeignClientException(int status, String message, byte[] body) {
-      super(status, message, body);
+    public FeignClientException(int status, String message, Request request, byte[] body) {
+      super(status, message, request, body);
     }
   }
 
   public static class BadRequest extends FeignClientException {
-    public BadRequest(String message, byte[] body) {
-      super(400, message, body);
+    public BadRequest(String message, Request request, byte[] body) {
+      super(400, message, request, body);
     }
   }
 
   public static class Unauthorized extends FeignClientException {
-    public Unauthorized(String message, byte[] body) {
-      super(401, message, body);
+    public Unauthorized(String message, Request request, byte[] body) {
+      super(401, message, request, body);
     }
   }
 
   public static class Forbidden extends FeignClientException {
-    public Forbidden(String message, byte[] body) {
-      super(403, message, body);
+    public Forbidden(String message, Request request, byte[] body) {
+      super(403, message, request, body);
     }
   }
 
   public static class NotFound extends FeignClientException {
-    public NotFound(String message, byte[] body) {
-      super(404, message, body);
+    public NotFound(String message, Request request, byte[] body) {
+      super(404, message, request, body);
     }
   }
 
   public static class MethodNotAllowed extends FeignClientException {
-    public MethodNotAllowed(String message, byte[] body) {
-      super(405, message, body);
+    public MethodNotAllowed(String message, Request request, byte[] body) {
+      super(405, message, request, body);
     }
   }
 
   public static class NotAcceptable extends FeignClientException {
-    public NotAcceptable(String message, byte[] body) {
-      super(406, message, body);
+    public NotAcceptable(String message, Request request, byte[] body) {
+      super(406, message, request, body);
     }
   }
 
   public static class Conflict extends FeignClientException {
-    public Conflict(String message, byte[] body) {
-      super(409, message, body);
+    public Conflict(String message, Request request, byte[] body) {
+      super(409, message, request, body);
     }
   }
 
   public static class Gone extends FeignClientException {
-    public Gone(String message, byte[] body) {
-      super(410, message, body);
+    public Gone(String message, Request request, byte[] body) {
+      super(410, message, request, body);
     }
   }
 
   public static class UnsupportedMediaType extends FeignClientException {
-    public UnsupportedMediaType(String message, byte[] body) {
-      super(415, message, body);
+    public UnsupportedMediaType(String message, Request request, byte[] body) {
+      super(415, message, request, body);
     }
   }
 
   public static class TooManyRequests extends FeignClientException {
-    public TooManyRequests(String message, byte[] body) {
-      super(429, message, body);
+    public TooManyRequests(String message, Request request, byte[] body) {
+      super(429, message, request, body);
     }
   }
 
   public static class UnprocessableEntity extends FeignClientException {
-    public UnprocessableEntity(String message, byte[] body) {
-      super(422, message, body);
+    public UnprocessableEntity(String message, Request request, byte[] body) {
+      super(422, message, request, body);
     }
   }
 
   public static class FeignServerException extends FeignException {
-    public FeignServerException(int status, String message, byte[] body) {
-      super(status, message, body);
+    public FeignServerException(int status, String message, Request request, byte[] body) {
+      super(status, message, request, body);
     }
   }
 
   public static class InternalServerError extends FeignServerException {
-    public InternalServerError(String message, byte[] body) {
-      super(500, message, body);
+    public InternalServerError(String message, Request request, byte[] body) {
+      super(500, message, request, body);
     }
   }
 
   public static class NotImplemented extends FeignServerException {
-    public NotImplemented(String message, byte[] body) {
-      super(501, message, body);
+    public NotImplemented(String message, Request request, byte[] body) {
+      super(501, message, request, body);
     }
   }
 
   public static class BadGateway extends FeignServerException {
-    public BadGateway(String message, byte[] body) {
-      super(502, message, body);
+    public BadGateway(String message, Request request, byte[] body) {
+      super(502, message, request, body);
     }
   }
 
   public static class ServiceUnavailable extends FeignServerException {
-    public ServiceUnavailable(String message, byte[] body) {
-      super(503, message, body);
+    public ServiceUnavailable(String message, Request request, byte[] body) {
+      super(503, message, request, body);
     }
   }
 
   public static class GatewayTimeout extends FeignServerException {
-    public GatewayTimeout(String message, byte[] body) {
-      super(504, message, body);
+    public GatewayTimeout(String message, Request request, byte[] body) {
+      super(504, message, request, body);
     }
   }
 }
diff --git a/core/src/main/java/feign/Response.java b/core/src/main/java/feign/Response.java
@@ -281,11 +281,11 @@ public void close() throws IOException {
 
     @Override
     public String toString() {
-        try {
-            return new String(toByteArray(inputStream), UTF_8);
-        } catch (Exception e) {
-            return super.toString();
-        }
+      try {
+        return new String(toByteArray(inputStream), UTF_8);
+      } catch (Exception e) {
+        return super.toString();
+      }
     }
   }
 

diff --git a/core/src/main/java/feign/RetryableException.java b/core/src/main/java/feign/RetryableException.java
@@ -31,17 +31,18 @@ public class RetryableException extends FeignException {
    * @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
    */
   public RetryableException(int status, String message, HttpMethod httpMethod, Throwable cause,
-      Date retryAfter) {
-    super(status, message, cause);
+      Date retryAfter, Request request) {
+    super(status, message, request, cause);
     this.httpMethod = httpMethod;
     this.retryAfter = retryAfter != null ? retryAfter.getTime() : null;
   }
 
   /**
    * @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER} header.
    */
-  public RetryableException(int status, String message, HttpMethod httpMethod, Date retryAfter) {
-    super(status, message);
+  public RetryableException(int status, String message, HttpMethod httpMethod, Date retryAfter,
+      Request request) {
+    super(status, message, request);
     this.httpMethod = httpMethod;
     this.retryAfter = retryAfter != null ? retryAfter.getTime() : null;
   }