Consider refactoring toward finer control over Response Handling and Retry

[kdavisk6]

The current logic treats all IOExceptions from a Client as something that is network related; however, not all of the Client implementations do so. For example, the ApacheHttpClient treat URISyntaxExceptions as IOExceptions, triggering a retry.

Also, all RequestMethods are treated the same. This can lead to retries of POST, PUT, and DELETE methods as outline in #719

Finally, the default Retryer implementation lacks the ability to control if the original exception should be thrown or not. This is called out #709 and #703.

I'd like to start a discussion around which of these the community feels is the most important and how we can go about updating Feign. Thoughts?

[holy12345]

Hi @kdavisk6

My thoughts about retries is

• Have a parameter let clients decide which HttpStatus can retry(such as 500 or 404)

• Have a parameter let clients decide which HttpMethod can retry(such as GET or POST)

• I noticed that feign retry mechanism is implemented in an infinite loop.

public class SynchronousMethodHandler {
    @Override
     public Object invoke(Object[] argv) throws Throwable {
         RequestTemplate template = buildTemplateFromArgs.create(argv);
         Retryer retryer = this.retryer.clone();
         while (true) {
             try {
                 return executeAndDecode(template);
              } catch (RetryableException e) {
                retryer.continueOrPropagate(e);
                if (logLevel != Logger.Level.NONE) {
                logger.logRetry(metadata.configKey(), logLevel);
             }
            continue;
          }
     }
}

Can we use some retry tools to complete the retry function(such as spring-retry or guava-retry).

I hope to hear your suggestions.

Thanks : )

[kdavisk6]

Closing in favor of #709

[Tal24]

I don't know why it is closed, #709 is about throw the actual exception.
@holy12345 suggests to:
Have a parameter let clients decide which HttpStatus.
I think it is a good suggestion.