Introduce 'decodeAll' option so response statuses != 2xx can be decoded as well

[bitx1]

My team is using the Spring framework and RestTemplate to make client calls. We would like to move from RestTemplate to Feign. We currently use RestTemplate.exchange(...) to return a response object irrespective of the status (e.g. 2xx, 4xx, 5xx). So instead of dealing with try-catch exceptions, we interrogate the response object's status after the client call. To make marshaling life easier, our API contracts have a placeholders for both an error result and successful results -- thus we get marshaled responses for all statuses as well.

The default feign implementation (SynchronousMethodHandler) only allows 2xx and optionally 404 to be decoded... while all other statuses get an error thrown. We would like all response objects (regardless of the status) to get passed to the custom Spring Decoder and the client caller get back a ResponseEntity object.

This feature may solve other people's "issues" since it give users a centralized place (e.g. response interceptor) to deal with feign's Response objects. This centralized can return an object or fallback to the feign's original logic of throwing an exception.

This feature is similar to decode404.... but for all statuses.

@spencergibb any thoughts on this?

[tburny]

I came here and just wanted to ask the same question: How to decode everything to JSON or turn an exception into nothing in the error handler?

I saw https://github.com/OpenFeign/feign/wiki/Custom-error-handling but it only seems to allow transforming the error.

The responsible line for this is https://github.com/OpenFeign/feign/blob/master/core/src/main/java/feign/SynchronousMethodHandler.java#L138

A simple fix could be to allow returning null in the ErrorHandler, which means "no exception is thrown, please decode as usual". This would keep existing clients intact.

[kdavisk6]

Initial thought on this point to using the your own ErrorDecoder to manage responses outside of the 200 and 404 range, read the exception, then build your own responses outside of it.

However, I can see where an option to configure which response codes are Errors could be useful. I'll mark this as an enhancement.

[ryandanielspmc]

@kdavisk6

This is a big problem for us as well. For example, the Cloudflare API has a common body structure for 2xx and 4xx statuses, and what we need is to return that object from the client methods in both cases.

A success status looks like this for a 200 status:

{
  "errors": [],
  "messages": [],
  "result": { (result-data) },
  "success": true
}

And a failure looks like this for a 400 status:

{
  "errors": [
    {
      "code": 7003,
      "message": "Could not route to /zones/abc, perhaps your object identifier is invalid?"
    },
    {
      "code": 7000,
      "message": "No route for that URI"
    }
  ],
  "messages": [],
  "result": null,
  "success": false
}

What is needed is probably one of these things:

• A new type of mapper/decoder to determine if a request is an error
• If ErrorDecoder returns null, then the request processing continues as normal
• Replace ErrorDecoder with a new ResponseMapper that has the chance to intercept the response and throw an exception if necessary. The functionality could be very similar to the current ErrorDecoder

[Patouche]

Hi !

I'm facing the same problem described by @ryandanielspmc.

Maybe instead of a decodeAll option, we can add a simple decode option that will take an array of integers (for http status). In this case we will use a Decoder instead of using an ErrorDecoder which will allow users to configure the type of error that can be decoded like any other 2XX response.

I think this is not a major change and it will allow all users to configure their client as they wish.

If I have enough time, I can try to submit a PR if you're ok with that ?