Optional request body should be supported by feign

[dimw]

In Spring MVC it is possible to annotate body parameter as optional using @RequestBody(required = false) parameter annotation, e.g.:

@FeignClient("foo-service")
public interface FooService {
    @RequestMapping(value = "/foo", method = RequestMethod.POST)
    void doFoo(@RequestBody(required = false) FilterDto filter);
}

However, Spring-Feign contract does not consider the optionality flag which leads to an IllegalStateException on proxy bootstrapping in consumer when calling something like:

fooService.doFoo(null);

java.lang.IllegalArgumentException: Body parameter 0 was null
    at feign.Util.checkArgument(Util.java:102)
    at feign.ReflectiveFeign$BuildEncodedTemplateFromArgs.resolve(ReflectiveFeign.java:323)
    at feign.ReflectiveFeign$BuildTemplateByResolvingArgs.create(ReflectiveFeign.java:213)
    at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:72)
    at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:103)

It is expected that null can be passed as parameter since it is optional.

Using:

• spring-cloud-starter-feign:1.1.0.RELEASE
• feign-core:8.16.2

[daniellavoie]

From the stack trace you should understand that this issue is a pure Feign problem. Spring Cloud Netflix facilitate Netflix OSS component integration into Spring. I would suggest you to open an issue on the feign project.

[dimw]

@daniellavoie, thanks for your feedback. Indeed, initially I was about to open this issue on the Feign project (because of the reasons admitted by you) but stack traces should not be always interpreted literally. Basically it seems not like a "a pure Feign problem" for me. Please correct me if I am wrong but:

1. The Spring MVC-Feign contract alongside with annotation processors (both part of this project) are responsible for proper processing of the producer interface. Thus, they should process the @RequestBody annotation and translate the required flag to Feign metadata.
2. Feign does not know anything about Spring MVC annotations (i.e. @RequestBody) so even it can see the annotations provided on the method, it cannot (and should not) understand semantics of these.

In fact, there might be currently no way in Feign to handle optional request bodies. However, this behavior should be at least documented in this project (because of the difference to behavior anticipated in Spring MVC). Further, it might be considered to create a certain RequestBodyParameterProcessor which catches the issue one level higher before letting Feign to throw semi-reasonable exception.

[daniellavoie]

You are right. Looks like I had a bad understanding of Spring Cloud Feign. I'll try to have a look and see if I can come up with a trick to handle an optinal parameter. If you have any inspiration, feel free to give some help ;)

[dimw]

@spencergibb: Do you think you can drop a line and explain why this issue has been closed without any further comment?

[spencergibb]

Sorry about that, it was a mistake.

[realdammy]

Hello @spencergibb, any update on this issues?

[ryanjbaxter]

@realdammy it is labeled as an enhancement but as far as I know no one is working on it at the moment. If you want to submit a PR for it that would be more than welcome.

[dyc87112]

@ryanjbaxter I try to fix this issue. But feign always check the body, so I have to wait this PR : https://github.com/OpenFeign/feign/pull/583.

@Override
protected RequestTemplate resolve(Object[] argv, RequestTemplate mutable, Map<String, Object> variables) {
  Object body = argv[metadata.bodyIndex()];
  checkArgument(body != null, "Body parameter %s was null", metadata.bodyIndex());
  ...
  return super.resolve(argv, mutable, variables);
}

If this PR can be accept. I will create another PR for here.

@dimw Thanks for your analysis.

[sdhery]

Already have a solution?

[dyc87112]

@sdhery https://github.com/OpenFeign/feign/pull/583 has not been accept，so you need compile feign by yourself，then write an RequestBodyParameterProcessor implements AnnotatedParameterProcessor interface to support this issue