f/error-as-a-value-even-when-error-occurs: init

core/src/main/java/feign/AsyncFeign.java

@@ -245,6 +245,7 @@ public AsyncFeign<C> internalBuild() {
                       dismiss404,
                       closeAfterDecode,
                       decodeVoid,
+                      decodeErrorResponses,
                       responseInterceptorChain()),
                   AsyncResponseHandler.class,
                   capabilities);

core/src/main/java/feign/AsyncResponseHandler.java

@@ -38,6 +38,28 @@ class AsyncResponseHandler {
       boolean closeAfterDecode,
       boolean decodeVoid,
       ResponseInterceptor.Chain executionChain) {
+    this(
+        logLevel,
+        logger,
+        decoder,
+        errorDecoder,
+        dismiss404,
+        closeAfterDecode,
+        decodeVoid,
+        false,
+        executionChain);
+  }
+
+  AsyncResponseHandler(
+      Level logLevel,
+      Logger logger,
+      Decoder decoder,
+      ErrorDecoder errorDecoder,
+      boolean dismiss404,
+      boolean closeAfterDecode,
+      boolean decodeVoid,
+      boolean decodeErrorResponses,
+      ResponseInterceptor.Chain executionChain) {
     this.responseHandler =
         new ResponseHandler(
             logLevel,
@@ -47,6 +69,7 @@ class AsyncResponseHandler {
             dismiss404,
             closeAfterDecode,
             decodeVoid,
+            decodeErrorResponses,
             executionChain);
   }
 

core/src/main/java/feign/BaseBuilder.java

@@ -63,6 +63,7 @@ public abstract class BaseBuilder<B extends BaseBuilder<B, T>, T> implements Clo
   protected InvocationHandlerFactory invocationHandlerFactory =
       new DefaultInvocationHandlerFactory();
   protected boolean dismiss404;
+  protected boolean decodeErrorResponses;
   protected ExceptionPropagationPolicy propagationPolicy = NONE;
   protected List<Capability> capabilities = new ArrayList<>();
 
@@ -246,6 +247,64 @@ public B decode404() {
     return thisB();
   }
 
+  /**
+   * Returns an error response as a value instead of throwing, when the upstream describes the
+   * failure in the response body rather than by status alone.
+   *
+   * <p>Some teams answer a failed call with a status in the 4xx/5xx range <em>and</em> a body
+   * describing what went wrong. By default Feign hands any non-2xx response to the {@link
+   * #errorDecoder(ErrorDecoder) error decoder} and throws, so that body is unreachable. With this
+   * flag set, the body is decoded into the method's declared return type and returned:
+   *
+   * <pre>
+   * interface MyApi {
+   *   &#064;RequestLine("POST /users")
+   *   BaseResponse createUser(NewUser user);
+   * }
+   *
+   * Feign.builder()
+   *      .decoder(new JacksonDecoder())
+   *      .decodeErrorResponses()
+   *      .target(MyApi.class, "https://api.example.com");
+   * </pre>
+   *
+   * <p>The flag engages only when <em>every</em> one of the following holds; in any other case the
+   * response is thrown exactly as it is today:
+   *
+   * <ul>
+   *   <li>the response status is 400 or above &mdash; 3xx is left to {@link
+   *       RedirectionInterceptor}, and is not a failure;
+   *   <li>the {@link #decoder(Decoder) decoder} accepts the response, checked via {@link
+   *       feign.codec.PredicatedDecoder#canDecode}, so a decoder that declares itself as JSON will
+   *       not be handed an HTML error page from a proxy. A decoder that is not a {@code
+   *       PredicatedDecoder} declares nothing, so this check cannot be made and is skipped;
+   *   <li>the {@link #errorDecoder(ErrorDecoder) error decoder} did not classify the response as
+   *       {@link RetryableException retryable}. Retryable failures are thrown as before, so {@link
+   *       Retryer} keeps working;
+   *   <li>the body actually decodes. If it does not, the error decoder's exception is thrown, with
+   *       the decode failure attached as {@linkplain Throwable#addSuppressed suppressed}.
+   * </ul>
+   *
+   * <p><b>This flag applies to every method on the client.</b> It does not check that the return
+   * type is one an error body makes sense as, because there is nothing reliable to check against:
+   * most decoders ignore unknown properties, so an error body will decode into an unrelated type
+   * and produce an object with every field null rather than failing. On a client with this flag
+   * set, a method whose return type is not an error-body shape will return such an object instead
+   * of throwing. Use a separate client for methods that should keep throwing.
+   *
+   * <p><b>Note for custom decoders:</b> when this flag engages, the {@link Response} passed to the
+   * decoder has its status rewritten to {@code 200}. This is deliberate &mdash; several decoders,
+   * including {@code JacksonDecoder} and {@link feign.optionals.OptionalDecoder}, return an empty
+   * value for 404 and 204 without reading the body, which would discard the very body being asked
+   * for. A decoder that branches on {@link Response#status()} will therefore not see the real
+   * status; read it from the response passed to a {@link ResponseInterceptor}, or declare {@code
+   * TypedResponse<T>}, whose {@link TypedResponse#status()} always reports the true status.
+   */
+  public B decodeErrorResponses() {
+    this.decodeErrorResponses = true;
+    return thisB();
+  }
+
   public B errorDecoder(ErrorDecoder errorDecoder) {
     this.errorDecoder = errorDecoder;
     return thisB();

core/src/main/java/feign/Feign.java

@@ -224,6 +224,7 @@ public Feign internalBuild() {
               dismiss404,
               closeAfterDecode,
               decodeVoid,
+              decodeErrorResponses,
               responseInterceptorChain());
       MethodHandler.Factory<Object> methodHandlerFactory =
           new SynchronousMethodHandler.Factory(

core/src/main/java/feign/InvocationContext.java

@@ -21,6 +21,7 @@
 import feign.codec.DecodeException;
 import feign.codec.Decoder;
 import feign.codec.ErrorDecoder;
+import feign.codec.PredicatedDecoder;
 import java.io.IOException;
 import java.lang.reflect.Type;
 
@@ -32,6 +33,7 @@ public class InvocationContext {
   private final boolean dismiss404;
   private final boolean closeAfterDecode;
   private final boolean decodeVoid;
+  private final boolean decodeErrorResponses;
   private final Response response;
   private final Type returnType;
 
@@ -44,12 +46,35 @@ public class InvocationContext {
       boolean decodeVoid,
       Response response,
       Type returnType) {
+    this(
+        configKey,
+        decoder,
+        errorDecoder,
+        dismiss404,
+        closeAfterDecode,
+        decodeVoid,
+        false,
+        response,
+        returnType);
+  }
+
+  InvocationContext(
+      String configKey,
+      Decoder decoder,
+      ErrorDecoder errorDecoder,
+      boolean dismiss404,
+      boolean closeAfterDecode,
+      boolean decodeVoid,
+      boolean decodeErrorResponses,
+      Response response,
+      Type returnType) {
     this.configKey = configKey;
     this.decoder = decoder;
     this.errorDecoder = errorDecoder;
     this.dismiss404 = dismiss404;
     this.closeAfterDecode = closeAfterDecode;
     this.decodeVoid = decodeVoid;
+    this.decodeErrorResponses = decodeErrorResponses;
     this.response = response;
     this.returnType = returnType;
   }
@@ -71,13 +96,26 @@ public Object proceed() throws Exception {
       return disconnectResponseBodyIfNeeded(response);
     }
 
+    Response response = this.response;
     try {
       final boolean shouldDecodeResponseBody =
           (response.status() >= 200 && response.status() < 300)
               || (response.status() == 404 && dismiss404 && !isVoidType(returnType));
 
       if (!shouldDecodeResponseBody) {
-        throw decodeError(configKey, response);
+        if (!shouldDecodeErrorResponseBody(response)) {
+          throw decodeError(configKey, response);
+        }
+        // Buffer once: both the error decoder and the body decoder below need to read it, and a
+        // response body is not generally replayable.
+        response = bufferBody(response);
+        Exception error = errorDecoder.decode(configKey, response);
+        if (error instanceof RetryableException) {
+          // Retryable failures stay exceptions, so Retryer keeps behaving as it does today.
+          ensureClosed(response.body());
+          throw error;
+        }
+        return decodeErrorResponseBody(response, error);
       }
 
       if (isVoidType(returnType) && !decodeVoid) {
@@ -99,6 +137,60 @@ public Object proceed() throws Exception {
     }
   }
 
+  /**
+   * Whether the error body should be returned as a value rather than thrown, per {@link
+   * feign.BaseBuilder#decodeErrorResponses()}. The return type is deliberately not inspected: the
+   * method's signature is the declaration of what an error body should decode to. Retryability is
+   * not checked here either, since it requires reading the body, which the caller only does once it
+   * knows the flag is in play.
+   */
+  private boolean shouldDecodeErrorResponseBody(Response response) {
+    if (!decodeErrorResponses || response.status() < 400) {
+      return false;
+    }
+    // A decoder that declares what it handles gets to refuse an HTML error page from a proxy. One
+    // that declares nothing cannot be asked, so the status range above is the only gate.
+    return !(decoder instanceof PredicatedDecoder)
+        || ((PredicatedDecoder) decoder).canDecode(response, returnType);
+  }
+
+  /**
+   * Decodes the error body, falling back to throwing {@code error} if it does not decode. The
+   * decoder is handed a response whose status reads {@code 200}: decoders commonly short-circuit
+   * 404 and 204 to an empty value without reading the body, which would discard the envelope that
+   * was asked for. {@link TypedResponse} is still built from the real response, so its status stays
+   * truthful.
+   */
+  private Object decodeErrorResponseBody(Response response, Exception error) throws Exception {
+    Response decodable = response.toBuilder().status(200).build();
+    Class<?> rawType = Types.getRawType(returnType);
+    try {
+      if (TypedResponse.class.isAssignableFrom(rawType)) {
+        Type bodyType = Types.resolveLastTypeParameter(returnType, TypedResponse.class);
+        return TypedResponse.builder(response).body(decode(decodable, bodyType)).build();
+      }
+      return decode(decodable, returnType);
+    } catch (FeignException e) {
+      if (error == null) {
+        // A custom ErrorDecoder may return null; the decode failure is then the only diagnosis.
+        throw e;
+      }
+      error.addSuppressed(e);
+      throw error;
+    }
+  }
+
+  private static Response bufferBody(Response response) throws IOException {
+    if (response.body() == null) {
+      return response;
+    }
+    try {
+      return response.toBuilder().body(Util.toByteArray(response.body().asInputStream())).build();
+    } finally {
+      ensureClosed(response.body());
+    }
+  }
+
   private static Response disconnectResponseBodyIfNeeded(Response response) throws IOException {
     final boolean shouldDisconnectResponseBody =
         response.body() != null

core/src/main/java/feign/ResponseHandler.java

@@ -40,6 +40,8 @@ public class ResponseHandler {
 
   private final boolean decodeVoid;
 
+  private final boolean decodeErrorResponses;
+
   private final ResponseInterceptor.Chain executionChain;
 
   public ResponseHandler(
@@ -51,6 +53,28 @@ public ResponseHandler(
       boolean closeAfterDecode,
       boolean decodeVoid,
       ResponseInterceptor.Chain executionChain) {
+    this(
+        logLevel,
+        logger,
+        decoder,
+        errorDecoder,
+        dismiss404,
+        closeAfterDecode,
+        decodeVoid,
+        false,
+        executionChain);
+  }
+
+  public ResponseHandler(
+      Level logLevel,
+      Logger logger,
+      Decoder decoder,
+      ErrorDecoder errorDecoder,
+      boolean dismiss404,
+      boolean closeAfterDecode,
+      boolean decodeVoid,
+      boolean decodeErrorResponses,
+      ResponseInterceptor.Chain executionChain) {
     super();
     this.logLevel = logLevel;
     this.logger = logger;
@@ -59,6 +83,7 @@ public ResponseHandler(
     this.dismiss404 = dismiss404;
     this.closeAfterDecode = closeAfterDecode;
     this.decodeVoid = decodeVoid;
+    this.decodeErrorResponses = decodeErrorResponses;
     this.executionChain = executionChain;
   }
 
@@ -74,6 +99,7 @@ public Object handleResponse(
               dismiss404,
               closeAfterDecode,
               decodeVoid,
+              decodeErrorResponses,
               response,
               returnType));
     } catch (final IOException e) {