openfeign / feign Goto Github PK
View Code? Open in Web Editor NEWFeign makes writing java http clients easier
License: Apache License 2.0
Feign makes writing java http clients easier
License: Apache License 2.0
Maybe I'm missing something, but it doesn't seem possible to mix a module with the .target()
-style builder. It'd be great to have a .module(Object...)
method so targets can be mixed with modules such as the JAXRS module. I'd be happy to PR it if I'm not missing something.
In some cases, it may be useful to have Feign instances that don't have any base URL at all. Instead, all methods would include a URI parameter.
Currently, this can be accomplished by using a custom implementation of Target that requires a URL to be provided. I've included an example below. This target can then be passed to Feign.builder().target(Target)
Feign.create(Target, Object...)
.
If such an EmptyTarget were included in feign-core, we could add signatures like Feign.builder().target(Class)
that allow creating Feign instances without a base URL and without explicitly specifying a Target type. A signature for Feign.create
will require a bit more thought, since Feign.create(Class, Object...)
would conflict with the pre-existing Feign.create(Class, String, Object...)
.
public class EmptyTarget<T> implements Target<T> {
private final Class<T> type;
private final String name;
public EmptyTarget(Class<T> type) {
this.type = checkNotNull(type, "type");
this.name = "empty:" + type.getSimpleName();
}
@Override
public Class<T> type() {
return type;
}
@Override
public String name() {
return name;
}
@Override
public String url() {
throw new UnsupportedOperationException("Empty targets don't have URLs");
}
@Override
public Request apply(RequestTemplate input) {
if (input.url().indexOf("http") != 0) {
throw new UnsupportedOperationException("Request with non-absolute URL not supported with empty target");
}
return input.request();
}
@Override
public int hashCode() {
return Arrays.hashCode(new Object[]{type, name});
}
@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}
if (this == obj) {
return true;
}
if (EmptyTarget.class != obj.getClass()) {
return false;
}
EmptyTarget<?> that = EmptyTarget.class.cast(obj);
return this.type.equals(that.type) && this.name.equals(that.name);
}
}
As you may be aware, Java's support for handling and resolving generic types is not particularly good. For background info, here -- http://www.cowtowncoder.com/blog/archives/2010/12/entry_436.html -- is a blog entry I wrote a while back explaining why use of java.lang.reflect.Type
is not sufficient for handling generic types of method calls.
Unfortunately many frameworks, such as JAX-RS, have used this is their canonical type representation.
Note that this is not just a theoretical issue: it is very difficult to, for example, implement java.lang.reflect.Type
. The only uses for it really are:
Class<?>
instances, ORType
one gets via reflection from Method
or Field
instances.Of these, second case is the problematic one, since it is not possible to resolve many cases that involve type parameters.
An immediate problem is that there aren't many obviously better mechanisms. For what it is worth, Classmate:
https://github.com/cowtowncoder/java-classmate
does actually solve the problem of handling of generic types. But downside is that not many codec libraries support ClassMate -- because of this, it may or may not be a good solution here.
Another smaller improvement would be introduction of "type token" type (Jackson and Gson have their own token types), which at least would allow static passing of generic types, with constructs like:
new TypeReference<Map<String,Integer>>() { }
The problem here is similar to ClassMate case: since JDK does not provide such standard wrapper, nor is there a universally used 3rd party lib, it may be difficult to add full support.
@rspieldenner seems @jdamick found and fixed a pretty nasty bug #161. He and crew are blocked on it. Mind kicking a release of 7.2.1 from 7.x?
tip should be at f3252a1
When Module(overrides=true)
is set, any @Provides(type = SET)
methods will override as opposed to adding to ones specified by feign.
This leads to hacks inside ReflectiveFeign like below, which manually enforce a base set of decoders. These cannot be overridden by dagger at all.
this.decoders.put(void.class, stringDecoder);
this.decoders.put(Response.class, stringDecoder);
this.decoders.put(String.class, stringDecoder);
As soon as Dagger 1.1 is out (likely this week), we'll have access to the SET_VALUES feature, which should allow us to eliminate the troublesome Module(overrides=true)
features.
With this in, we should update examples and any how-to docs to show how to wire up feign so as to not require or suggest Module(overrides=true)
, except in very specific cases.
Mind if I release Feign to resolve the binary compatibility issues with Ribbon 0.3.x?
It would be really helpful if you can please release version 6.1.1 in maven central?
Incremental callbacks facilitate asynchronous invocations, where the http connection and decoding of elements inside the response are decoupled from the calling thread.
In a typical synchronous response, the calling thread blocks on the http connect, reading from the socket, and also decoding of the response data into a java type. As such, a java type is returned.
interface GitHub {
@RequestLine("GET /repos/{owner}/{repo}/contributors")
Iterator<Contributor> contributors(@Named("owner") String owner, @Named("repo") String repo);
}
In an incremental callback pattern, the calling thread blocks until the request is queued, but processing of the response (including connect, read, decode) are free to be implemented on different threads. To unblock the caller, void is returned.
interface GitHub {
@RequestLine("GET /repos/{owner}/{repo}/contributors")
contributors(@Named("owner") String owner, @Named("repo") String repo, IncrementalCallback<Contributor> contributors);
}
Denominator works primarily on high latency collections, where responses can take up to minutes to return. Even-though denominator returns Iterators, allowing iterative response processing, usually update granularity is per http request due to response parsing semantics of returning a list.
Ex. Sax parsing into a list which is later returned.
if (qName.equals("HostedZone")) {
zones.add(Zone.create(this.name, id));
...
@Override
public ZoneList getResult() {
return zones;
Ex. Incremental callback pattern allows for immediate visibility of new elements
if (qName.equals("HostedZone")) {
observer.onNext(Zone.create(this.name, id));
Also, Netflix RxJava is increasingly employed, so optimizing for Observers makes sense.
The following design takes from RxJava Observer and Retrofit Callback.
public interface IncrementalCallback<T> {
void onNext(T element);
void onSuccess();
void onFailure(Throwable cause);
}
The design we are discussing is how to address async callbacks in a Java idiomatic way. We issue Throwable on error instead of Exception, as Feign doesn't have a catch-all exception wrapper and Errors could be the reason why the failure occurred. This allows visibility of Errors such as AssertionErrors or thread starvation to propagate without confusing exception wrappers. It also allows us to simply propagate exception.getCause()
without a wrapper.
The above rationale supports the case for Throwable vs Exception where Feign exists in isolation. Where Feign interacts with other libraries unveils other considerations. For example, Every current java async web callback issues throwable, not exception, for error signals. If we chose to retain Exception as opposed to Throwable, converting to or from these interfaces will be lossy.
In order to support asynchronous processing of http responses, we need to implement responses of Future, (RxJava) Observable, or allow users to pass in a Callback or and Observer.
As Observable is the centerpiece of RxJava's reactive pattern, it is a relatively wide interface. As such, the source for Observable.java is larger than the entire codebase of feign including tests. Making a move to couple to Observable seems overkill just to afford asynchronous invocation.
Future semantics are hard to guarantee, as operations such as cancel() rarely perform as expected with blocking i/o. For example, many implementations of http futures simply fire a callable which has no means to stop the connection or stop reading from it. Moreover timeouts are hard to make sense in future. For example connect vs read timeouts are hard to understand. Typically a response is returned after connect, but during connection you have no means to stop it. After future.get is returned, another timeout is possible reading from the stream. In summary while Future is often used, it is a tricky abstraction.
Callback and Observer approaches are simplest and incur no core dependencies. They can both be type-safe interface, such as the one in Retrofit or android-http. IncrementalCallback slightly wins as it allows for incremental parsing, and doesn't imply that Feign is Observable.
RxJava Observer is not a wide interface, so much easier to grok than Observable. However, the Observer type in rx is a concrete class and implies a dependency on rxjava, a dependency which is an order of magnitude larger than feign itself. Decoupling in core, and recoupling as an extension is the leanest way to interop.
I would expect it to be prefixed onto the method level @Path
annotations.
For many users, feign will be the first time they see Dagger. Dagger has a wonderful system of compile-time failures. However, when Module(library=true)
is set, unused bindings will not fail. This causes an issue when for example, someone binds a parameterized type where a raw type is needed.
Let's hunt down anything that implies users will have to supply Module(library=true)
and kill it or identify why.
For those who do not use dagger, or do not wish to, we could expose a builder which defines all dependencies explicitly that can be provided. This includes logging config, decoders, etc. This could be modeled similarly to retrofit RestAdapter.Builder or jclouds ContextBuilder
ex.
api = Feign.builder()
.loggingLevel(BASIC)
.decoder(customType)
.target(GitHub.class, "http://foo");
Note that this will still use dagger under the scenes, albeit via reflection. When/if someone wants to switch to pure dagger, they could use the module system directly.
Consider the example:
public interface BaseAPI {
@RequestLine("GET /health")
String health();
@RequestLine("GET /all")
List<Entity> all();
}
And then some specific API:
public interface CustomAPI extends BaseAPI {
@RequestLine("GET /custom")
String custom();
}
I'd like to use CustomAPI
when creating Feign client and be able to access methods of the BaseAPI
.
Currently only methods defined directly in the CustomAPI
are bound to handlers.
It looks like the only required changes to be made are:
1 feign.Contract.BaseContract.parseAndValidatateMetadata(Class<?> declaring)
:
// for (Method method : declaring.getDeclaredMethods()) {
for (Method method : declaring.getMethods()) {
2 feign.ReflectiveFeign.newInstance(Target<T> target)
:
// for (Method method : target.type().getDeclaredMethods()) {
for (Method method : target.type().getMethods()) {
I was able to get the required functionality with the above changes.
Would be great to have this functionality either out of the box or easily configurable.
I think would be great to have a log4j/slf4j Logger.
If you agree I will can to send a PR for that.
I implemented an enhanced contract that extends the default contract but adds the ability to set interface level Headers as defaults unless overridden at the method level..
I also added convenience annotations for ContentType & Accept since those (at least in our experience) are most typically set.
The annotations follow more of the jaxrs style, but another option I could see would be adding these as settables in the Feign.Builder when creating the client.. I'm on fence as which one i like better.
Are either of these of interest as a pull? @adriancole
Dagger 1.x is not likely to continue. Dagger 2 is.
I'm interested enough to help port everything here (and maybe fix some bugs etc along the way), plus revamp denominator accordingly.
Please ping this issue if you feel strongly that feign should not move to Dagger 2.
making this v4 so that we can delete IncrementalCallback :)
If viewed in the light that Iterator is to Observer as Iterable is to Observable, the essential relationships needed to make a functional Observer pattern are relatively tight.
unsubscribe()
which means "stop giving me stuff!"Subscription subscribe(Observer<T> observer);
Currently, when a @QueryParam
value is specified, and the user passes null, there's a literal null in the http request. Using as close to JAX-RS and URI-Template semantics as we can, we should allow the client (only in jax-rs) to skip a query parameter on null.
This should support at least an Observer implementation (plus other ideas @benjchristensen has in mind)
I've seen the RequestInterceptor example. Does Feign allow user to put stuff in header for passing additional information on each call without recreating the client?
I have client code that will create a Feign client one time, but based on the Request Interceptor example, we need to recreate the feign client every time the code is called.
There are a number of ideas in feign that didn't work out very well, are edge case, or pure over-engineering. We should get rid of them as feign is used in production and should focus its code base, failure cases, and WTFs on things relevant to those use cases.
Set<Decoder>
is confusing when the type parameter is meaningful.This doesn't mean we shouldn't play with ideas or that these ideas will never reach prod. Just that we shouldn't commit all ideas to trunk as that increases the cognitive and maintenance load of feign without helping real production use. This may imply we need a "labs" repo or persistent forks to facilitate research.
Particularly in relation to @Provides
and @Consumes
, we need to interpret jax-rs annotations knowing that they are defined on service apis. For example, @Consumes
implies that the server accepts the media type. So, feign's interpretation should be that @Consumes
affects the content-type header, not the accept header, sent.
cc @syed-haq
It would be really helpful to trackdown things like socket timeout errors, if the logger emitted a message when an IOException occurs.
This came up using denominator where a service emitted read timeouts longer than expected.
Currently, it looks to me like there are entries in CHANGES.md for 6.0 and 6.0.2, but nothing for 6.0.1. From looking around a bit, it looks like 6.0.1 was released, and should include the change that is listed as 6.0.2. Probably the correct fix is to change the existing entry in CHANGES.md from 6.0.2 to 6.0.1?
I'd like to use hystrix with feign, but it appears i'd have to shoehorn it into the invocation handler factory, and if i wanted the ability to do things async it would be more changes to methodmetadata to look for Future<?> return types..
I could do this on the outside of the feign, but that seems kludgy so i wanted to see if there were any better ideas floating around?
I have an API that I'd like to use Feign to connect to that requires that the POST request body use gzip content encoding.
This feels to me like something that should be possible using a RequestInterceptor, but the current structure of RequestTemplate (storing body as String, and converting to bytes in the client) doesn't really seem like it supports it. I know I could add support for this using a custom Client implementation. Is there a better approach?
The GitHub example seems like a nice intro to Feign, but it is actually a poor choice. The json model is clean, as are the way rest calls are made. Most apis are not even close to as clean as this, so the example leads to a cliff.
@benjchristensen had a good idea to add an example a nastier, more realistic api, such as wikipedia. Let's do that.
While there are a couple examples, there's no step-by-step introduction for using feign, or how to make custom decoders, etc.
It'd be nice to be able to supply custom query param encoders. For example, sometimes we need to encode a Date
query param as a String
with a given format.
As far as I can tell this isn't currently possible. I'm trying to use feign for writing integration tests against an internal service. At the moment I've had to write my own ErrorDecoder to intercept and clone the Response object. It would be much easier from my perspective to disable the exceptions.
I'll try to get it implemented myself. But I'd need a little help with working out why the gradle build doesn't appear to work.
"Artifact 'org.codehaus.plexus:plexus-utils:2.0.5@jar' not found."
I can't even run gradle dependencies
to find out what's pulling it in.
There are cases where it may be useful to submit binary content as the body for a request. Examples of this include uploading files or using content encodings (such as compression).
This was first discussed under issue #52.
If you run the following test - https://gist.github.com/bstick12/2a21ffd30b14ec68716f#file-ribbonclienttest
You will occasionally get the the following warning issued from servo.
The com.netflix.loadbalancer.LoadBalancerContext.initWithNiwsConfig(IClientConfig) registers the load balancer and the RibbonClient is recreating the LBClient on each request.
0:44:38.155 [pool-1-thread-2] WARN c.n.servo.jmx.JmxMonitorRegistry - Unable to register Monitor:MonitorConfig{name=Client_github, tags=class=LBClient, policy=DefaultPublishingPolicy}
javax.management.InstanceNotFoundException: com.netflix.servo:name=github_LoadBalancerExecutionTimer,statistic=max,class=LBClient,type=GAUGE,id=Client_github,unit=MILLISECONDS
at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getMBean(DefaultMBeanServerInterceptor.java:1095) ~[na:1.7.0_72]
at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.exclusiveUnregisterMBean(DefaultMBeanServerInterceptor.java:427) ~[na:1.7.0_72]
at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.unregisterMBean(DefaultMBeanServerInterceptor.java:415) ~[na:1.7.0_72]
at com.sun.jmx.mbeanserver.JmxMBeanServer.unregisterMBean(JmxMBeanServer.java:546) ~[na:1.7.0_72]
at com.netflix.servo.jmx.JmxMonitorRegistry.register(JmxMonitorRegistry.java:82) ~[servo-core-0.7.4.jar:na]
at com.netflix.servo.jmx.JmxMonitorRegistry.register(JmxMonitorRegistry.java:106) ~[servo-core-0.7.4.jar:na]
at com.netflix.servo.DefaultMonitorRegistry.register(DefaultMonitorRegistry.java:135) [servo-core-0.7.4.jar:na]
at com.netflix.servo.monitor.Monitors.registerObject(Monitors.java:207) [servo-core-0.7.4.jar:na]
at com.netflix.loadbalancer.LoadBalancerContext.initWithNiwsConfig(LoadBalancerContext.java:111) [ribbon-loadbalancer-2.0-RC5.jar:na]
at com.netflix.loadbalancer.LoadBalancerContext.<init>(LoadBalancerContext.java:82) [ribbon-loadbalancer-2.0-RC5.jar:na]
at com.netflix.loadbalancer.LoadBalancerExecutor.<init>(LoadBalancerExecutor.java:45) [ribbon-loadbalancer-2.0-RC5.jar:na]
at com.netflix.client.AbstractLoadBalancerAwareClient.<init>(AbstractLoadBalancerAwareClient.java:54) [ribbon-loadbalancer-2.0-RC5.jar:na]
at feign.ribbon.LBClient.<init>(LBClient.java:47) [feign-ribbon-7.2.0.jar:7.2.0]
at feign.ribbon.RibbonClient.lbClient(RibbonClient.java:76) [feign-ribbon-7.2.0.jar:7.2.0]
at feign.ribbon.RibbonClient.execute(RibbonClient.java:64) [feign-ribbon-7.2.0.jar:7.2.0]
at feign.SynchronousMethodHandler.executeAndDecode(SynchronousMethodHandler.java:95) [feign-core-7.2.0.jar:7.2.0]
at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:74) [feign-core-7.2.0.jar:7.2.0]
at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:101) [feign-core-7.2.0.jar:7.2.0]
at com.brendan.atlassian.$Proxy6.users(Unknown Source) [na:na]
at com.brendan.atlassian.RibbonClientTest$1.run(RibbonClientTest.java:56) [test-classes/:na]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471) [na:1.7.0_72]
Do you think it's possible to make the RibbonModule configurable with FeignBuilder ?
It's a requirement to exploit RibbonModule on spring-cloud-netflix.
Thanks.
Per a discussion with @NiteshKant, I believe we can make a compatible http response callback interface to facilitate nio support.
Many async http clients provide a callback interface such as below
Future<Integer> f = asyncHttpClient.prepareGet("http://www.ning.com/").execute(
new AsyncCompletionHandler<Integer>(){
@Override
public Integer onCompleted(Response response) throws Exception{
// Do something with the Response
return response.getStatusCode();
}
@Override
public void onThrowable(Throwable t){
// Something wrong happened.
}
});
https://github.com/AsyncHttpClient/async-http-client
It should be possible to extend our Client
to be compatible with the callback system from one or more async http clients, avoiding the need to independently manage threads in Feign.
ex.
interface Client {
// existing
Response execute(Request request, Options options) throws IOException;
// new
void execute(Request request, Options options, IncrementalCallback<Response> responseCallback);
An asynchronous client would need to map their callback to ours.
Any synchronous http client (including our default) could extend from a base class that implements the callback method like so
void execute(Request request, Options options, IncrementalCallback<Response> responseCallback) {
httpExecutor.get().execute(new Runnable() {
@Override public void run() {
Error error = null;
try {
responseCallback.onNext(execute(request, options));
responseCallback.onSuccess();
} catch (Error cause) {
// assign to a variable in case .onFailure throws a RTE
error = cause;
responseCallback.onFailure(cause);
} catch (Throwable cause) {
responseCallback.onFailure(cause);
} finally {
Thread.currentThread().setName(IDLE_THREAD_NAME);
if (error != null)
throw error;
}
}
}
}
This will ensure that the examples are always up-to-date. Also, this fixes the problem that snapshot/release build jobs do not always have access to old releases in its internal artifacts repository.
currently, we use method key (ex. IAM
or IAM#auth(String,String)
) to wire decoders to the methods that need them. This is consistent with error handlers, etc, but possibly overkill for decoders, where the intended result is always correlated to the return type of the method (or the type param of the observer).
If we switch to type-only, it is possible for us to use Set bindings, to collect decoders. Something far less complicated then our current approach.
Currently feign doesn't support iterable query param. Could you add this support?
Also, it should probably throw a better exception then the current one when using iterable query param:
feign.FeignException: status 505 reading ; content:
I've got a working implementation of this as a RequestInterceptor that I'd be happy to contribute. Assuming that there's interest in this, would you prefer it within core, in a separate module in the main repo, or publishing it as a standalone repo elsewhere?
We haven't had a release in a while..
Using the following test - https://gist.github.com/bstick12/2a21ffd30b14ec68716f#file-ribbonclienttest
You will see the following exception raised in the logs
21:04:25.583 [pool-1-thread-2] DEBUG c.n.l.LoadBalancerExecutor - Got error java.net.SocketException: Unexpected end of file from server when executed on server api.github.com:443
This is due to the request being made to http://api.github.com:443/.
The URI created in RibbonClient does include a scheme and eventually com.netflix.loadbalancer.LoadBalancerContext.deriveSchemeAndPortFromPartialUri(URI) will default the scheme to http.
Hi,
I'm using Feign with a JAX-RS interface using JAX-RS Response return types (not feign.Response).
For instance:
@Path("/users")
@Produces(MediaType.APPLICATION_JSON)
public interface UserContract {
@POST
@Path("/")
@Consumes(MediaType.APPLICATION_JSON)
public Response create(User user);
...
When using feign and calling the "create" method, it seems that feigns tries to deserialize the jaxrs.Response object from the HTTP reponse.
Well it makes sense since it's the return type but it's obviously not what I expect. What would be the best way to handle jaxrs.Response type with feign ?
Thanks
Currently any error caused by the executed HTTP call results in a FeignException. This makes it impossible to handle according to the type of HTTP error. (Or am I missing something?)
Would it be an idea to:
A vendor management system at netflix started looking at feign and realized they needed a way to trace every java call that results in feign calls.
While this could be done via the feign Logger, it is cleaner to extract out a profiling interface. I don't have a specific design, yet, but we could borrow from retrofit's Profiler.
Before making a final choice, we should document what metadata is needed in the requests and see how (if at all) this is addressed in other systems such as JAX-RS 2.0, WebSocket, Netty, etc.
A vendor management system at netflix started looking at feign and realized they needed a way to add contextual headers to a request, such as application id, and request id.
While this could be done with Target.apply, the essence of the change is more environment-specific. Exposing an Interceptor interface could be a better way to model this.
class FooInterceptor implements RequestInterceptor {
@Override public void intercept(RequestTemplate request) {
request.header("extra", "value");
}
}
In attempt to pull things closer to retrofit, let's sync to the same logging style now that retrofit supports HEADERS scope:
Picking up the latest version I'd expect to be able to do something like the following
@RequestLine("GET /users?name={name}")
public JsonObject getUsersByName(@nAmed("name") String name);
Unfortunately this doesn't seem to work. The default encoder receives a LinkedHashMap and complains about it not being a String. The GsonEncoder copes fine with it but the end result is that a POST is made with the query parameters encoded into the body.
Adds features needed to cut a denominator release.
Let's release 7.0.0, particularly as some backports are only partially merged into 6.x. Doing this also frees up master to do more bold things.
What's the thought on applying a workaround similar to jersey to use PATCH or other methods to HttpURLConnection?
If feign doesn't explicitly set an Accept header then HttpURLConnection.java (line 376) will set the Accept header below.
Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2
This can cause API validations to choke, if they're expecting no Accept header or an Accept header from a predefined list of application/json
, application/xml
, etc.
Feign should check for default Accept header in mock tests to prevent this scenario from occurring.
See this comment in Denominator for more context.
Let's cut 7.1, as it has the feign.Param
annotation needed to bridge to 8.x. Also includes OkHttp support and some bug fixes.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.