search

eclipse / microprofile-rest-client

MicroProfile Rest Client
Apache License 2.0
142 stars 72 forks source link

Fault Tolerance support in builder created client #347

Open Verdent opened 2 years ago

Verdent commented 2 years ago

I would like to ask you, whether it is expected for clients created via builder (not CDI), to support FT CDI interceptor bindings. There is no TCK test which would verify such a behavior and therefore I am not sure whether it is actually required.

andymc12 commented 2 years ago

The short answer is yes - FT CDI interceptor bindings should be honored - even for programmatically-created client instances. This is the text I would refer to:

If CDI is available, the MP Rest Client implementation must ensure that CDI business method interceptors are invoked when the appropriate interceptor binding is applied to the client interface or method.

Also:

MicroProfile Fault Tolerance MP Rest Client implementations must ensure that MP Fault Tolerance annotations on client interfaces are honored. In general, these annotations are treated as CDI interceptor bindings.

MP Rest Client should ensure that the behavior of most Fault Tolerance annotations should follow the behavior outlined in the MP Fault Tolerance specification. This includes the @Asynchronous, @Bulkhead, @CircuitBreaker, @Fallback and @Retry annotations.

The @Timeout annotation presents a problem since some parts of the MP Rest Client request are non-blocking and non-interruptible. Implementations should override the default connect and read timeouts and use the timeout value specified in the @Timeout annotation instead. This will ensure that the actual time spent in blocking/non-interruptible operations should be less than or equal to the time specified in the annotation, allowing the MP Fault Tolerance implementation to interrupt the request and the throw the appropriate TimeoutException.

Neither texts says that FT interceptors are limited only to CDI-injected interfaces.

I think it would be good to add a test to CDIInterceptorTest to use a programmatic client in addition to the already-tested CDI-injected client.

tomas-langer commented 2 years ago

I can understand the CDI interceptors work for injected cases. I do not clearly understand how the interaction with Buider works. Considering that the builder can override most of the configuration (including endpoint), build cannot use an instance managed by CDI (as it would get an instance configured with different configuration). I can imagine this working in case the injection would be always @Dependent and that we could somehow get the configuration from builder to the CDI factory method, but not when a bean can be @ApplicationScoped. As a result, instances created by builder cannot have CDI interceptors, as they cannot be managed by CDI.

I am wondering how you expect to get CDI interceptors for instances not created and managed by CDI?

andymc12 commented 2 years ago

Open Liberty supports CDI interceptors from programmatic clients. Perhaps you could look at their code (and the CXF code it is built on) to see how it works? That said, CDI has a variety of ways to get interceptors for specified bindings - or you could go all the way of making the programmatically-created instance be managed by CDI.

I'm no longer with IBM, and so I haven't been too much involved in MP Rest Client lately, so if the community would prefer that CDI interceptors be limited to CDI-created/managed client instances, it is fine with me. :-)

Emily-Jiang commented 9 months ago

This needs further discussion and investigation.