Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsatisfying 302 handling #728

Open
vguna opened this issue Jun 29, 2018 · 7 comments
Open

Unsatisfying 302 handling #728

vguna opened this issue Jun 29, 2018 · 7 comments

Comments

@vguna
Copy link

@vguna vguna commented Jun 29, 2018

Hi.

I'm a long time JAX-RS client proxy user which likes the usage of interface generated clients very much. Now I wanted to give feign a try and see how it performs. Setup is simple and integrates nicely in the Spring ecosystems.

But there are still some things which are quite inconvenient and IMHO better solved in JAX-RS.
E.g. 302 handling of errors. JAX-RS won't throw exceptions if 302 is encountered. I already read #249 about the several "workarounds" that I'm not very keen of.

Why not just providing an option in the builder whether to throw or to ignore 302 and leaving the default as before to keep backwards compatibility?

Even If I would try to live with the exception that is thrown, how do I get access to the Response? Since in our case when 302 is returned, the payload contains information we need. FeignException sadly doesn't expose the Response. Also JAX-RS does a better job here where I have access to the response and so the payload. It even provides concrete exception types for well known statuses which make it easy to handle specific error cases.

I would love at least to see that exposing of the response in FeignException to gain more control. Now I have to write my own decoder/error handler and throw my own RedirectException and putting the response to it, being able to process it in the code.

@kdavisk6

This comment has been minimized.

Copy link
Member

@kdavisk6 kdavisk6 commented Jul 1, 2018

Are you using the legacy jaxrs library or the updated jaxrs2 library?

@vguna

This comment has been minimized.

Copy link
Author

@vguna vguna commented Jul 1, 2018

I'm using Jersey 2.22 which implements JAX-RS 2.0.

@kdavisk6

This comment has been minimized.

Copy link
Member

@kdavisk6 kdavisk6 commented Jul 4, 2018

I was referring to which feign library. There are two, feign-jaxrs with is based on the JAX-RS 1.x specification and feign-jaxrs2 which is based on the JAX-RS 2.x specification. Can you please confirm which one you are using?

@vguna

This comment has been minimized.

Copy link
Author

@vguna vguna commented Jul 5, 2018

None of them as it seems. I'm using the spring-cloud-starter-openfeign 1.4.4 which depends on feign-core/hystrix/slf4j 9.5.0 together with feign-jackson 9.7.0. I have a interface defined using Spring annotations. No JAX-RS involved.

@kdavisk6

This comment has been minimized.

Copy link
Member

@kdavisk6 kdavisk6 commented Jul 5, 2018

I misunderstood your issue. My suggestion is to not use Client.Default and use one of the more robust versions available via ApacheHttp, OkHttp, or Ribbon, if you are under the gun. As you've discovered in #249, the default client handles 3xx responses differently than the others.

The Spring Cloud documentation outlines how to use these with Spring Cloud OpenFeign

http://cloud.spring.io/spring-cloud-static/spring-cloud-openfeign/2.0.0.RELEASE/multi/multi_spring-cloud-feign.html#spring-cloud-feign-overriding-defaults

I do agree, that leaving the handling of redirects up to the Clients may feel like a cop-out, it's our only option here without more feedback regarding the suggestions outlined in #249

@vguna

This comment has been minimized.

Copy link
Author

@vguna vguna commented Jul 9, 2018

Ok, so you're saying if I choose another client, it will behave differently in terms of 302? But it won't change the fact that I don't have access to the response when FeignException is thrown, right :D? I'll have a look @ the other clients then. Thanks!

@kdavisk6

This comment has been minimized.

Copy link
Member

@kdavisk6 kdavisk6 commented Jul 10, 2018

Unfortunately, that's the case right now. The Client is ultimately what is responsible for making the request and handling the initial response. Feign includes Clients for ApacheHttp, OkHttp and Netflix Ribbon. These Clients are a more appropriate choice for production environments over the default one.

We have a number of issues open around improving FeignException.

See #703, #709, #642

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.