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

Call Timeout(OKHTTP) not working for my Open Feign Config. #2342

Open
parthiv-groww opened this issue Mar 6, 2024 · 17 comments
Open

Call Timeout(OKHTTP) not working for my Open Feign Config. #2342

parthiv-groww opened this issue Mar 6, 2024 · 17 comments

Comments

@parthiv-groww
Copy link

I have made okhttp config bean and used that for my feign client config. I need to use call timeout in my service and it is not working the way it should. I have also configured read and connect timeout and they are working fine.

@Bean Feign feignBuilder(@Qualifier("cdpOkHttpClient") OkHttpClient cdpOkHttpClient) { return Feign .builder() .client(new feign.okhttp.OkHttpClient(cdpOkHttpClient)) .encoder(new SpringEncoder(messageConverters)) .decoder(new SpringDecoder(messageConverters)) .retryer(new CDPRetryerConfig()) .build(); }

My Okhttp config

`@Bean(name = "baseOkHttpClient")
public OkHttpClient baseOkHttpClient() {
return new OkHttpClient.Builder()
.readTimeout(500, TimeUnit.MILLISECONDS)
.connectTimeout(500, TimeUnit.MILLISECONDS)
.callTimeout(10,TimeUnit.MILLISECONDS)
.build();
}

@Bean(name = "cdpOkHttpClient")
public OkHttpClient cdpOkHttpClient(@Qualifier("baseOkHttpClient") OkHttpClient okHttpClient) {
    return okHttpClient.newBuilder()
        .readTimeout(rewardsConfig.getDpBroker().getReadTimeout(), TimeUnit.MILLISECONDS)
        .connectTimeout(rewardsConfig.getDpBroker().getConnectTimeout(), TimeUnit.MILLISECONDS)
            .callTimeout(rewardsConfig.getDpBroker().getCallTimeout(),TimeUnit.MILLISECONDS)
        .build();
}`

and I am picking the timeout values from application.yml file

I have also tried using writing unit test to test this configuration. With custom Okhttp bean it works fine but does not work when used with Feign client.

@gromspys
Copy link
Contributor

@parthiv-groww it looks like you need to add options for your feign client to use the same OkHttpClient:

@Bean 
Feign feignBuilder(@Qualifier("cdpOkHttpClient") OkHttpClient cdpOkHttpClient) { 
    return Feign.builder() 
        .client(new feign.okhttp.OkHttpClient(cdpOkHttpClient)) 
        .options(new Request.Options(cdpOkHttpClient.connectTimeoutMillis(), TimeUnit.MILLISECONDS, 
                                     cdpOkHttpClient.readTimeoutMillis(), TimeUnit.MILLISECONDS,
                                     cdpOkHttpClient.followRedirects())
        .encoder(new SpringEncoder(messageConverters)) 
        .decoder(new SpringDecoder(messageConverters)) 
        .retryer(new CDPRetryerConfig())
        .build(); 
}

@parthiv-groww
Copy link
Author

Hello @gromspys , thank you for your response.
The problem that I am facing here is using call Timeout in my feign client using Okhttp.
I have tested my both read and connect timeout using the configs that I have shared before. It is working as expected but the real issue is with using call timeout in the similar way.
Okhttp supports call timeout but it is not properly configurable with feign client and hence unable to use it.
Using the (.options) configuration, I am able to use both read and connect timeout but I am not sure how to use that for call timeout.
My retryer is also working as expected.

@gromspys
Copy link
Contributor

Could you please give your unit test? I tried to reproduce, but it looks like call timeout works as expected:

  public interface OkHttpClientTestApi {

    @RequestLine("GET /{delay}")
    Response get(@Param("delay") String delay);
  }

  @Test
  public void whenCallTimeoutExceeded_thenInterruptedIOException() {
    okhttp3.OkHttpClient client = new okhttp3.OkHttpClient.Builder()
            .callTimeout(1, TimeUnit.SECONDS)
            .build();

    okhttp3.Request request = new okhttp3.Request.Builder()
            .url("https://httpbin.org/delay/2")
            .build();

    Throwable thrownClient = catchThrowable(() -> client.newCall(request).execute());
    assertThat(thrownClient).isInstanceOf(InterruptedIOException.class);

    OkHttpClientTestApi api = Feign.builder()
            .client(new OkHttpClient(client))
            .target(OkHttpClientTestApi.class, "https://httpbin.org/delay/");

    Throwable thrownApi = catchThrowable(() -> api.get("2"));
    assertThat(thrownApi).isInstanceOf(RetryableException.class);
  }

@parthiv-groww
Copy link
Author

parthiv-groww commented Mar 12, 2024

wireMock.stubFor(post(urlPathEqualTo("/v1/readFeatureValues")).willReturn(aResponse().withFixedDelay(1001).withStatus(200)));
       Throwable throwable = catchThrowable(() -> cdpDao.getFeatures("user1", CdpRequestDto.builder().build()));
       assertThat(throwable).isInstanceOf(InterruptedIOException.class);

This is my unit test . Even I did try with okhttp client in similar manner and it did work but not with feign client.

OkHttpClient client = new OkHttpClient.Builder() .callTimeout(1, TimeUnit.MILLISECONDS) .build(); Request request = new Request.Builder().url("http://localhost:8030").build(); Throwable throwable= catchThrowable(()-> client.newCall(request).execute()); assertThat(throwable).isInstanceOf(InterruptedIOException.class);

And here is how my cdpDao is annotated with Feign client:

@FeignClient(name = "cdp-client", url = "${url}" , configuration = FeignClientConfig.class) public interface CdpDao

One more thing, in the second assert in your code, dont we need to check with "InterruptedIOException.class"

@gromspys
Copy link
Contributor

As I remember feign client has default retryer. In unit test we can change RetryableException to InterruptedIOException:

assertThat(thrownApi.getCause()).isInstanceOf(InterruptedIOException.class);

@parthiv-groww
Copy link
Author

So can you please suggest any changes that we need to do so that we are able to use call timeout?

@gromspys
Copy link
Contributor

It's hard to say what is wrong without full configuration. But it looks like your configuration was ignored:

@Bean 
Feign feignBuilder(@Qualifier("cdpOkHttpClient") OkHttpClient cdpOkHttpClient) { 
    return Feign.builder()
        .client(new feign.okhttp.OkHttpClient(cdpOkHttpClient)) 
        .encoder(new SpringEncoder(messageConverters)) 
        .decoder(new SpringDecoder(messageConverters)) 
        .retryer(new CDPRetryerConfig()) 
        .build(); 
}

Try to remove annotation @FeignClient(name = "cdp-client", url = "${url}" , configuration = FeignClientConfig.class) and build your client manually:

@Bean 
CdpDao cdpDao(@Qualifier("cdpOkHttpClient") OkHttpClient cdpOkHttpClient, @Value("url") String url) { 
    return Feign.builder()
        .client(new feign.okhttp.OkHttpClient(cdpOkHttpClient)) 
        .encoder(new SpringEncoder(messageConverters)) 
        .decoder(new SpringDecoder(messageConverters)) 
        .retryer(new CDPRetryerConfig()) 
        .target(CdpDao.class, url); 
}

@parthiv-groww
Copy link
Author

parthiv-groww commented Mar 14, 2024

Hey @gromspys
I did write an exact test case the way you did .

@Test
    public void sampleCallTimeoutTest(){
        long start = System.currentTimeMillis();
        Throwable throwable = catchThrowable(()-> testDao.get("3"));
        long end = System.currentTimeMillis();
        log.info("elasped time {}", end-start);
        assertThat(throwable).isInstanceOf(InterruptedIOException.class);
    }

My sample Interface

public interface TestDao {
    @RequestLine("POST /{delay}")
    Response get(@Param("delay") String delay);

}

My sample configs

import feign.Feign;
import okhttp3.OkHttpClient;
import org.springframework.beans.factory.ObjectFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.boot.autoconfigure.http.HttpMessageConverters;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
public class FeignClientConfig {
    @Autowired
    private ObjectFactory<HttpMessageConverters> messageConverters;

    @Bean
    public TestDao feignBuilder(@Qualifier("cdpOkHttpClient") okhttp3.OkHttpClient cdpOkHttpClient) {
        return  Feign
                .builder()
                .client(new feign.okhttp.OkHttpClient(cdpOkHttpClient))
                .target(TestDao.class, "http://localhost:8090");

    }
}

package com.example.demo.Config;

import lombok.RequiredArgsConstructor;
import okhttp3.OkHttpClient;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

import java.util.concurrent.TimeUnit;

@Configuration
@RequiredArgsConstructor
public class OkHttpClientConfig {


    @Bean(name = "baseOkHttpClient")
    public okhttp3.OkHttpClient baseOkHttpClient() {
        return new okhttp3.OkHttpClient.Builder()
                .readTimeout(500,TimeUnit.MILLISECONDS)
                .connectTimeout(500, TimeUnit.MILLISECONDS)
                .callTimeout(10,TimeUnit.MILLISECONDS)
                .build();
    }

    @Bean(name = "cdpOkHttpClient")
    public OkHttpClient cdpOkHttpClient(@Qualifier("baseOkHttpClient") okhttp3.OkHttpClient okHttpClient) {
        return okHttpClient.newBuilder()
                .readTimeout(500,TimeUnit.MILLISECONDS)
                .connectTimeout(500, TimeUnit.MILLISECONDS)
                .callTimeout(1000,TimeUnit.MILLISECONDS)
                .build();
    }
}

My pom.xml file

<dependency>
			<groupId>io.github.openfeign</groupId>
			<artifactId>feign-core</artifactId>
			<version>13.2.1</version>
		</dependency>
		<dependency>
			<groupId>io.github.openfeign</groupId>
			<artifactId>feign-okhttp</artifactId>
			<version>13.1</version>
		</dependency>

I set a call timeout of 100ms and started a timer to evaluate the exact time the thread runs for.
Even though the call timeout is set , the thread duration for that particular test is roughly around 1300 ms . With call timeout set, it should probably get terminated.

@gromspys
Copy link
Contributor

It looks like there is default retryer bacause I'm getting 6300 ms. Try to set .retryer(Retryer.NEVER_RETRY)

@parthiv-groww
Copy link
Author

import feign.Retryer;
import okhttp3.OkHttpClient;
import org.springframework.beans.factory.ObjectFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.boot.autoconfigure.http.HttpMessageConverters;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
public class FeignClientConfig {
    @Autowired
    private ObjectFactory<HttpMessageConverters> messageConverters;

    @Bean
    public TestDao feignBuilder(@Qualifier("cdpOkHttpClient") okhttp3.OkHttpClient cdpOkHttpClient) {
        return  Feign
                .builder()
                .client(new feign.okhttp.OkHttpClient(cdpOkHttpClient))
                .retryer(Retryer.NEVER_RETRY)
                .target(TestDao.class, "http://localhost:8090");

    }
}

I have made the suggested changes and my call timeout is 10ms but still my total thread duration is somewhat around 90ms (which suggests that call timeout is not working). And I am not getting the required InterruptedIOException.

@gromspys
Copy link
Contributor

Need some time for initialization. Try to call twice in the same test and compare time. To get InterruptedIOException exception use throwable.getCause()

@parthiv-groww
Copy link
Author

parthiv-groww commented Mar 15, 2024

    @Test
    public void sampleCallTimeoutTest(){
        long start = System.currentTimeMillis();
        Throwable throwable = catchThrowable(()-> testDao.get("3"));
        long end = System.currentTimeMillis();
        long start1 = System.currentTimeMillis();
        Throwable throwable1 = catchThrowable(() -> testDao.get("4"));
        long end1 = System.currentTimeMillis();
        log.info("elasped time {}", end-start);
        log.info("elasped second time {}", end1-start1);
        assertThat(throwable.getCause()).isInstanceOf(InterruptedIOException.class);
    }

This is the changes that I have made, My call timeout is 10ms

2024-03-15T10:41:58.796+05:30  INFO 30665 --- [           main] c.e.demo.Service.PortfolioServiceTest    : elasped time 88
2024-03-15T10:41:58.796+05:30  INFO 30665 --- [           main] c.e.demo.Service.PortfolioServiceTest    : elasped second time 2

java.lang.AssertionError: 
Expecting actual throwable to be an instance of:
  java.io.InterruptedIOException
but was:
  java.net.ConnectException: Failed to connect to localhost/[0:0:0:0:0:0:0:1]:8090

and this was my output.

@gromspys
Copy link
Contributor

Looks like the url http://localhost:8090 is not working. Try using https://httpbin.org/delay/ instead.

@parthiv-groww
Copy link
Author

@Bean(name = "baseOkHttpClient")
    public OkHttpClient baseOkHttpClient() {
        return new OkHttpClient.Builder()
            .readTimeout(500, TimeUnit.MILLISECONDS)
            .connectTimeout(500, TimeUnit.MILLISECONDS)
                .callTimeout(100,TimeUnit.MILLISECONDS)
            .build();
    }

    @Bean(name = "cdpOkHttpClient")
    public OkHttpClient cdpOkHttpClient(@Qualifier("baseOkHttpClient") OkHttpClient okHttpClient) {
        return okHttpClient.newBuilder()
            .readTimeout(1000, TimeUnit.MILLISECONDS)
            .connectTimeout(1000, TimeUnit.MILLISECONDS)
                .callTimeout(10,TimeUnit.MILLISECONDS)
            .build();
    }

Now this is my config for okhttp

@Bean
    CdpDao feignBuilder(@Qualifier("cdpOkHttpClient") OkHttpClient cdpOkHttpClient) {
        return Feign
            .builder()
            .client(new feign.okhttp.OkHttpClient(cdpOkHttpClient))
            .encoder(new SpringEncoder(messageConverters))
            .decoder(new SpringDecoder(messageConverters))
            .retryer(new CDPRetryerConfig())
                .target(CdpDao.class, "https://httpbin.org/delay");
    }

And my config for Feign Client bean

 @Test
    void testCallTimeout(){
        long start = System.currentTimeMillis();
        Throwable throwable = catchThrowable(()-> cdpDao.getFeatures("3"));
        long end = System.currentTimeMillis();
        long start1 = System.currentTimeMillis();
        Throwable throwable1 = catchThrowable(() -> cdpDao.getFeatures("4"));
        long end1 = System.currentTimeMillis();
        log.info("elasped time {}", end-start);
        log.info("elasped second time {}", end1-start1);
        assertThat(throwable1.getCause()).isInstanceOf(InterruptedIOException.class);
    }
2024-03-15T14:47:01.410+05:30  INFO 39192 --- [           main]    : Feign retry attempt 1 due to timeout executing POST https://httpbin.org/delay/
2024-03-15T14:47:01.475+05:30  INFO 39192 --- [           main]     : Feign retry attempt 2 due to timeout executing POST https://httpbin.org/delay/
2024-03-15T14:47:01.590+05:30  INFO 39192 --- [           main]   : Feign retry attempt 3 due to timeout executing POST https://httpbin.org/delay/
2024-03-15T14:47:01.590+05:30  INFO 39192 --- [           main]   : elasped time 317
2024-03-15T14:47:01.590+05:30  INFO 39192 --- [           main]       : elasped second time 194

I have a retryer setup at 1000 ms of read or connect Timeout and my call timeout is 10 ms as per the above config. So my retryer should not have been called and call timeout should have been executed. (But I am still getting the InterruptedIoException as expected and my test case is successful).

@gromspys
Copy link
Contributor

Everything works as expected. Retryer was executed by call timeout exception (like 500 server status code). If you don't need to retry, just use Retryer.NEVER_RETRY.

@parthiv-groww
Copy link
Author

 @PostMapping(value = "/v1/readFeatureValues",
            headers = {"X-USER-ID: {userAccountId}", "Content-Type: application/json"})
    CdpResponseDto getFeatures(@RequestHeader("X-USER-ID") String userAccountId,
                               @RequestBody CdpRequestDto cdpRequestDto);

Here postmapping is not working also and I want to replace it with RequestLine which does not support using more parameters with RequestBody.
Can you help on this.

@gromspys
Copy link
Contributor

You can find how to do it in documentation https://github.com/OpenFeign/feign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants