spring-cloud-openfeign icon indicating copy to clipboard operation
spring-cloud-openfeign copied to clipboard

Implement Feign ErrorDecoder that is aware of spring HttpClientErrorException and HttpServerErrorException

Open xyloman opened this issue 9 years ago • 33 comments

In our consumption of feign we ran into an issue where we need access to the response status code and response body associated to a client error. We have implemented the following ErrorDecoder implementation in our code base and wanted to share it with the community. It seems like others that are familiar with spring RestOperations are accustom to HttpClientErrorException and HttpServerErrorException exception trees versus the stripped down FeignException. The only concern with the implementation below is how to handle the response body if an exception where to happen during the copying process from feign response to a given instance of spring HttpStatusCodeException:

public class SpringWebClientErrorDecoder implements ErrorDecoder {

    private ErrorDecoder delegate = new ErrorDecoder.Default();

    @Override
    public Exception decode(String methodKey, Response response) {
        HttpHeaders responseHeaders = new HttpHeaders();
        response.headers().entrySet().stream()
                .forEach(entry -> responseHeaders.put(entry.getKey(), new ArrayList<>(entry.getValue())));

        HttpStatus statusCode = HttpStatus.valueOf(response.status());
        String statusText = response.reason();

        byte[] responseBody;
        try {
            responseBody = IOUtils.toByteArray(response.body().asInputStream());
        } catch (IOException e) {
            throw new RuntimeException("Failed to process response body.", e);
        }

        if (response.status() >= 400 && response.status() <= 499) {
            return new HttpClientErrorException(statusCode, statusText, responseHeaders, responseBody, null);
        }

        if (response.status() >= 500 && response.status() <= 599) {
            return new HttpServerErrorException(statusCode, statusText, responseHeaders, responseBody, null);
        }
        return delegate.decode(methodKey, response);
    }
}

xyloman avatar Apr 27 '15 13:04 xyloman

The feign support includes support for returning a ResponseEntity<MyObject> which gives you access to the status code, headers and body.

@FeignClient
interface TestClient {
    @RequestMapping(method = RequestMethod.GET, value = "/helloresponse")
    public ResponseEntity<Hello> getHelloResponse();
}

spencergibb avatar Apr 27 '15 14:04 spencergibb

Updated our code to use the ResponsEntity. Works great. It would be useful if there was something adding to the spring cloud documentation around this topic. If there is something I apologize for missing it.

xyloman avatar Apr 27 '15 17:04 xyloman

Nope, opened #319

spencergibb avatar Apr 27 '15 18:04 spencergibb

We started to do more integration testing and this appears to be working slightly different than I expected based upon your comment of using the ResponseEntity. Our use case is to handle a rest API that would return a 409 in the case of a conflict and a 201 in the case of a resource that is created. In these two scenarios the 409 results in a FeignException and the 201 results in a null object being returned instead of a resource entity. The 201 null object response we can account for. However, I was hoping that the 409 would result in a populated ResponseEntity. We also experience this in the case of a 400.

xyloman avatar Apr 29 '15 03:04 xyloman

Pull requests are welcome.

spencergibb avatar Apr 29 '15 14:04 spencergibb

When I use

@FeignClient("usermanagement")
public interface UserClient {
    @RequestMapping(method = RequestMethod.GET, value = "/um/v1/user/{secLoginId}", produces = "application/json")
    ResponseEntity<List<SecUserDTO>> getUsersWithHeader(@PathVariable("secLoginId") String secLoginId);
}

and then getUsersWithHeader returns 404, in exception (FeignException) e .g. like this:

feign.FeignException: status 404 reading UserClient#getUsersWithHeader(String); content:
{
  "httpStatus" : "NOT_FOUND",
  "code" : "U001",
  "message" : "User [ blah ] not found",
 }

How do I determine that I got 404 ? I thought if I wrap it into ResponseEntity, I wont get Exception, but I'm still getting FeignException. Here is my call:

try
        {
        ResponseEntity<List<SecUserDTO>> response = userClient.getUsersWithHeader(loginId);
        if (response.getStatusCode().equals(HttpStatus.OK))
        {
            List<SecUserDTO> userList = response.getBody();
            log.info("userList :{}", userList );
            if (userList != null && userList.size()>0)
            {
                returnUser=userList.get(0);
            }
        }
        else
        {
            log.error("Got error code:{} headers:{}", response.getStatusCode(), response.getHeaders());
        }

        }
        catch(Exception e)
        {
            log.error("Exception:",e);
        }

How do I call getUsersWithHeader and determine if I get 404 ?

bogdatov avatar Sep 04 '15 21:09 bogdatov

@bogdatov you need to use a create a feign.ErrorDecoder.

spencergibb avatar Sep 04 '15 21:09 spencergibb

How I can registermy feign.ErrorDecoder with FeignClient?

Why Error 404 is not catch by ResponseEntity with FeignClient?

renatojava avatar Oct 30 '15 01:10 renatojava

How I can registermy feign.ErrorDecoder with FeignClient?

See the docs Overriding Feign Defaults for how to register your ErrorDecoder.

Why Error 404 is not catch by ResponseEntity with FeignClient?

Because feign throws an error, though feign 8.12 will change that.

spencergibb avatar Nov 02 '15 19:11 spencergibb

Thanks.

renatojava avatar Nov 02 '15 19:11 renatojava

@spencergibb I implemented ErrorDecoder, but not able to get the exact error response message. Inside Response only getting reason and status. can we get detail message like as follows:

{"error":"invalid_token","error_description":"Token was not recognised"}

(this error is thrown by Spring oauth2 server and I was calling it through one service to authorize token)

do we need any additional configuration ? will FeignClientConfiguration help in this ?

agupta1989 avatar Dec 17 '15 05:12 agupta1989

Hi, I think automatically decode exception objects depend on the declared thrown types maybe a good idea. Just like:

@FeignClient
interface TestClient {
    @RequestMapping(method = RequestMethod.GET, value = "/hello/{name}")
    public Hello getHelloResponse(@Param("name") String name) thrown InvalidNameException;
}

@ResponseStatus(HttpStatus.BAD_REQUEST)
class InvalidNameException extends IllegalArgumentException {
    // ...
}

If the response status is BAD_REQUEST, it will decode and throw InvalidNameException automatically. If the method has more than one declared exception types, the decoder will choose one by some kind of conditions, like the ResponseStatus annotation on the InvalidNameException.

starlight36 avatar Sep 10 '16 03:09 starlight36

I used @AuthorizedFeignClient Instead of the default Feign Client and the issue is solved

mutisyap avatar Dec 09 '17 19:12 mutisyap

Hey guys what status of this?

I use @FeignClient(value = "test-service", decode404 = true) and it's handle 404 error, so I got ResponseEntity filled by code without exception.

Is this da way? way

Basically I use FeignClient to communicate with HATEOAS like service, full example code:

@FeignClient(value = "test-service", decode404 = true)
public interface TestServiceClient {
    @PostMapping(value = "/testItems", consumes = MediaTypes.HAL_JSON_VALUE)
    ResponseEntity<Resource<TestItem>> create(TestItem testItem);

    @GetMapping(value = "/testItems", consumes = MediaTypes.HAL_JSON_VALUE)
    ResponseEntity<Resources<TestItem>> findAll();

    @GetMapping(value = "/testItems/{id}", consumes = MediaTypes.HAL_JSON_VALUE)
    ResponseEntity<Resource<TestItem>> findOne(@PathVariable("id") String id);
}

Hronom avatar Feb 14 '18 15:02 Hronom

@Hronom the issue is still open so the issue still exists

ryanjbaxter avatar Feb 19 '18 15:02 ryanjbaxter

Not sure why FeignException has to convert the actual exception into a String and stuff it inside the Body. Most rest services throw custom exception and it would be nice to let them propagate as such. Now with @ResponseStatus we can define various Http Codes on exceptions all over an application. Having a centralized error decoder and trying to untangle them from the String seems like a waste of time. I have spent enough time just to untangle the actual business exception I want buried deep inside FeignException as a string!

seetharamani avatar Feb 23 '18 21:02 seetharamani

feign.FeignException: status 404 reading FeignInterface#test(String); content: {"timestamp":1519611212015,"status":404,"error":"Not Found","message":"No message available","path":"/**/test"}

iver3on avatar Feb 26 '18 02:02 iver3on

Guys I think me and other people a little bit confused about that we use ResponseEntity<> and still get exceptions instead of "filled" ResponseEntity<> with error code 4xx, 5xx etc.

It would be nice if you put flag in annotation org.springframework.cloud.netflix.feign.FeignClient like enable processing of ResponseEntity<>.

Or do it automatically if you see ResponseEntity<> as return type.

Hronom avatar Feb 27 '18 22:02 Hronom

I think the problem here is that ResponseEntity is parameterized class, and the only way to deserialize json response is to read it's type parameter.

Imageine a feign method that returns something like ResponseEntity<MyDTO>

When API returns response with status 500, then usually it's body is not deserializable into MyDTO. It should be deserialized into MyApiError or something. Or just Map<String, Object>. But anyway not into MyDTO.

This is the reason why RestTemplate can't just return ResponseEntity when response status is not 2xx and throw exception instead.

And this is the reason, I think, why this library can't do this.

Instead an ErrorDecoder should decode error response into ApiError model and throw specific appliation exception or some general exception, that includes ApiError as field.

xak2000 avatar May 02 '18 19:05 xak2000

how do we define errorDecoder if we use @FeignClient annotation on an interface to act as feign client or remote service interface?

coolprashi06 avatar Sep 18 '18 18:09 coolprashi06

@coolprashi06 your question seems unrelated, please open a separate issue in https://github.com/spring-cloud/spring-cloud-openfeign

ryanjbaxter avatar Sep 19 '18 13:09 ryanjbaxter

The feign-annotation-error-decoder seems to provide a good solution for this issue. Having the ability to decode exceptions by methodKey and status code combined with the ability to construct Exceptions using @FeignExceptionConstructor is very handy.

Is there interest in adding this feature and it is in line with the future of this project?

pway99 avatar Apr 26 '19 16:04 pway99

@pway99 sure we are open to that idea

ryanjbaxter avatar May 02 '19 13:05 ryanjbaxter

Thanks @ryanjbaxter I will start working on a pull request

pway99 avatar May 02 '19 15:05 pway99

I think the problem here is that ResponseEntity is parameterized class, and the only way to deserialize json response is to read it's type parameter.

Imageine a feign method that returns something like ResponseEntity<MyDTO>

When API returns response with status 500, then usually it's body is not deserializable into MyDTO. It should be deserialized into MyApiError or something. Or just Map<String, Object>. But anyway not into MyDTO.

This is the reason why RestTemplate can't just return ResponseEntity when response status is not 2xx and throw exception instead.

And this is the reason, I think, why this library can't do this.

Instead an ErrorDecoder should decode error response into ApiError model and throw specific appliation exception or some general exception, that includes ApiError as field.

This seems like the best solution I have come across for this problem. I have implemented exactly this and have a custom ErrorDecoder that returns a business exception with all the relevant information about the error.

The problem is that surrounding the call to the method on the Feign client with a try-catch does not catch the exception. The exception seems to be thrown using some sort of reflection. I believe I could define an ExceptionHandler somewhere, but I would like to handle the exception where the call is made and that does not allow for it.

Any suggestions on how I can catch the exception returned by the custom ErrorDecoder?

DewaldDeJager avatar Nov 15 '19 15:11 DewaldDeJager

Hello @DewaldDeJager,
I ended up solving the problem exactly as you have described. The ErrorDecoder below attempts to use the feign.codec.StringDecoder for decoding the response body, so I am not sure it's a suitable solution for the spring-cloud-openfeign project. Here is my implementation maybe it will help.

Example:

try {
	return yourFeignClient.getThing("stuff")
} catch (FeignClientException e) {
	if (!Integer.valueOf(HttpStatus.NOT_FOUND.value()).equals(e.getStatus())) {
		throw e;
	}
}

ErrorDecoder:

import java.io.IOException;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import feign.Response;
import feign.codec.ErrorDecoder;
import feign.codec.StringDecoder;

/**
 * Decodes a failed request response building a FeignClientException
 * Any error decoding the response body will be thrown as a FeignClientErrorDecodingException
 * @author patrick.way
 */
public class FeignClientExceptionErrorDecoder implements ErrorDecoder {
	private static final Logger LOGGER = LoggerFactory.getLogger(FeignClientExceptionErrorDecoder.class);
	private StringDecoder stringDecoder = new StringDecoder();

	@Override
	public FeignClientException decode(final String methodKey, Response response) {
		String message = "Null Response Body.";
		if (response.body() != null) {
			try {
				message = stringDecoder.decode(response, String.class).toString();
			} catch (IOException e) {
				LOGGER.error(methodKey + "Error Deserializing response body from failed feign request response.", e);
			}
		}
		return new FeignClientException(response.status(), message, response.headers());
	}
}

Exception:

public class FeignClientException extends RuntimeException {

	private static final long serialVersionUID = 1L;

	private final Integer status;
	private final String errorMessage;

	private final Map<String, Collection<String>> headers;

	public FeignClientException(Integer status, String errorMessage, Map<String, Collection<String>> headers) {
		super(String.format("%d %s", status, errorMessage));
		this.status = status;
		this.errorMessage = errorMessage;
		this.headers = headers;
	}

	/**
	 * Http Status Code
	 * @return
	 */
	public Integer getStatus() {
		return status;
	}

	public String getErrorMessage() {
		return errorMessage;
	}

	/**
	 * FeignResponse Headers
	 * @return
	 */
	public Map<String, Collection<String>> getHeaders() {
		return headers;
	}

}

Configuration:

@Bean
@ConditionalOnMissingBean(value = ErrorDecoder.class)
public FeignClientExceptionErrorDecoder commonFeignErrorDecoder() {
	return new FeignClientExceptionErrorDecoder();
}

pway99 avatar Nov 15 '19 16:11 pway99

@Hronom

Thanks,I tried your way, and it worked.

I also used openfeign and Spring data rest.

iceCloudZ avatar Dec 03 '19 05:12 iceCloudZ

I'm experiencing an IOException "Stream is closed" everytime I try to read the response.body() in my ErrorDecoder.

Anyone through this?

pliuchkin avatar Jun 04 '20 02:06 pliuchkin

I'm experiencing an IOException "Stream is closed" everytime I try to read the response.body() in my ErrorDecoder.

Anyone through this?

Just found it was the debug that was closing the connection

pliuchkin avatar Jun 04 '20 04:06 pliuchkin

Hello @pway99 ,

can we set different error decoder for each request mapping? i.e: -> If error 500 error I like to convert json responce to Hello500 @RequestMapping(method = RequestMethod.GET, value = "/hello/{name}") public Hello getHelloResponse(@Param("name") String name) thrown InvalidNameException;

-> If error 500 error I like to convert json responce to Bye500 @RequestMapping(method = RequestMethod.GET, value = "/bye/{name}") public Bye getByeResponse(@Param("name") String name) thrown InvalidNameException;

can you please guide me, looking for the similer feature that you pointed out Link: "https://github.com/OpenFeign/feign-annotation-error-decoder"

HelloNishit avatar Aug 26 '20 15:08 HelloNishit