Add response headers to FeignException

[vitalijr2]

Implementation of #1268

This PR adds the ability to retrieve the response headers from a FeignException.

This is replacement for #1277 as the source branch is gone.

[vitalijr2]

I think it would be great if @pzapasni and @virtual-machinist left some comment.

[virtual-machinist]

Can you please also add headers to feign.codec.DecodeException? Doing this would be trivial with your existing changes - you just have to add a couple of constructors that accept feign.Response instead of feign.Request and then calling another super() that accepts both feign.Request and headers. AFAIK all places that throw it also have the response available.

[vitalijr2]

Can you please also add headers to feign.codec.DecodeException?

I am OK with this but looks like it takes more time: there are a lot of calls of the method Util.toByteArray(response.body().asInputStream()) that could throw IOException.
In my opinion it should be refactored: at least to make responseBodyToByteArray that hides work with InputStream or pass to constructors whole response object instead of pair body as array and headers.

[virtual-machinist]

Ah, I see what you mean. However no feign.codec.DecodeException constructor currently even passes the body object to super(), so I'd argue we can safely pass a null instead of the body array.

But perhaps you're right and it's better to do this addition as a follow-up in a separate issue.

[vitalijr2]

@velo && @kdavisk6 how about remove some constructors and do not use the parameter response body as prepared byte array but use the response?

Proposal constructors of FeignException:

• status, message // EncodeException
• status, message, cause // EncodeException
• status, message, request // RetryableException but we do not use it - so it could be removed too
• status, message, request, cause // RetryableException when a client could not read response at all.
• status, message, response // DecodeException, errorStatus() -> ErrorDecoder, FeignClientException, FeignServerException
• status, message, response, cause // AsyncJoinException, DecodeException, errorReading() -> AsyncResponseHandler

The reading of response body move to an utility method that hides possible IOException and returns Nullable byte array.

[velo]

I think is a good improvement overall.

Could you please just file a ticket to simplify FeignException?

Ideally it should have one constructor for Request and one for Response. I'm sure it's more complicated than that.

[velo]

can we move this to the parent pom?!

[velo]

also, there is a hamcrest 2.0 somewhere, would prefer to go with that one

[velo]

I don't think FeignException should re-order response headers. It may make sense to sort it, sure, but not here.

[velo]

this exception is getting more and more complicated.... wondering if we should have one constructor that takes Request as argument and the other one that takes Response

Anyway, just concerned that this class is getting more and more complicated.