Kong / unirest-java

Unirest in Java: Simplified, lightweight HTTP client library.
http://kong.github.io/unirest-java/
MIT License
2.58k stars 591 forks source link

connectionTTL configuration ignored #494

Closed timomeinen closed 9 months ago

timomeinen commented 9 months ago

Configuring a time-to-live on the connection is getting ignored:

Unirest.config().connectionTTL(Duration.ofSeconds(123));

The field kong.unirest.core.Config # private long ttl = -1; is set, but never read during runtime. Even in the source code I cannot find a read access to getTtl().

Using Java HTTP UrlConnection Client has a property jdk.httpclient.keepalive.timeout that allows configuring the internal pool. Prior JDK 20 the default value is 1200 seconds and later is 30 seconds.

Is this something different or should Unirest.config().connectionTTL() setup that property?

My workaround is to set a system property System.setProperty("jdk.httpclient.keepalive.timeout", "30"); but I would expect to have this configured via Unirest.

Background: We use Unirest within AWS Lambda and a NAT Gateway. The NAT Gateway has a connection timeout of 350 seconds by default. Therefore, we often run into a server-side-closed connection but client status is, that it is still open. This leads to a client request that leads to an IOException as the connection is closed from server side.

Environmental Data:

Additional context Add any other context about the problem here.

ryber commented 9 months ago

ah yea, this was missed as the client itself has no public method to set it. But its reasonable to expect it would be set by the config

ryber commented 9 months ago

4.0.8 is making its way though the maven central pipeline right now, it should be available in the next hour

timomeinen commented 9 months ago

Hi @ryber, thank you for the quick fix. Unfortunately, the scale of "jdk.httpclient.keepalive.timeout" is seconds, but in kong.unirest.core.Config#connectionTTL(long, java.util.concurrent.TimeUnit) you set the value as milliseconds.

public Config connectionTTL(Duration duration) {
    return this.connectionTTL(duration.toMillis(), TimeUnit.MILLISECONDS);
}

This means, that the TTL is factor 1000 too big.

See: https://docs.oracle.com/en/java/javase/17/core/java-networking.html

The number of seconds to keep idle HTTP/1.1 connections alive in the keep alive cache.

ryber commented 9 months ago

derp, 4.0.10 switches that to seconds

timomeinen commented 9 months ago

Now it is working as expected. Thanks.