Skip to content

Commit

Permalink
issue #526 split connection timeout and request timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
ryber committed May 8, 2024
1 parent 386e94a commit 6eadf15
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 13 deletions.
4 changes: 2 additions & 2 deletions unirest/src/main/java/kong/unirest/core/BaseRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,8 @@ protected ObjectMapper getObjectMapper() {
}

@Override
public int getConnectTimeout() {
return valueOr(connectTimeout, config::getConnectionTimeout);
public Integer getRequestTimeout() {
return valueOr(connectTimeout, config::getRequestTimeout);
}

@Override
Expand Down
33 changes: 31 additions & 2 deletions unirest/src/main/java/kong/unirest/core/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@

import javax.net.ssl.SSLContext;
import java.io.InputStream;
import java.net.http.HttpClient;
import java.net.http.HttpConnectTimeoutException;
import java.net.http.*;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.nio.charset.StandardCharsets;
Expand All @@ -54,6 +53,7 @@ public class Config {
private Headers headers;
private Proxy proxy;
private int connectionTimeout;
private Integer requestTimeout;
private boolean followRedirects;
private boolean cookieManagement;
private boolean useSystemProperties;
Expand Down Expand Up @@ -84,6 +84,7 @@ private void setDefaults() {
customExecutor = null;
headers = new Headers();
connectionTimeout = DEFAULT_CONNECT_TIMEOUT;
requestTimeout = null;
followRedirects = true;
useSystemProperties = false;
cookieManagement = true;
Expand Down Expand Up @@ -288,6 +289,26 @@ public Config connectTimeout(int inMillies) {
return this;
}

/**
* Sets a default timeout for all requests. If the response is not received
* within the specified timeout then an {@link HttpTimeoutException} is
* thrown.
* completes exceptionally with an {@code HttpTimeoutException}. The effect
* of not setting a timeout is the same as setting an infinite Duration, ie.
* block forever.
*
* @param inMillies the timeout duration in millies
* @return this builder
* @throws IllegalArgumentException if the duration is non-positive
*/
public Config requestTimeout(int inMillies){
if(inMillies < 1){
throw new IllegalArgumentException("request timeout must be a positive integer");
}
this.requestTimeout = inMillies;
return this;
}

/**
* Clear default headers
* @return this config object
Expand Down Expand Up @@ -709,6 +730,14 @@ public int getConnectionTimeout() {
return connectionTimeout;
}

/**
* @return the connection timeout in milliseconds
* default: null (infinite)
*/
public Integer getRequestTimeout() {
return requestTimeout;
}

/**
* @return a security keystore if one has been provided
*/
Expand Down
2 changes: 1 addition & 1 deletion unirest/src/main/java/kong/unirest/core/HttpRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ default Optional<Body> getBody(){
/**
* @return the connect timeout for this request
*/
int getConnectTimeout();
Integer getRequestTimeout();

/**
* @return a summary for the response, used in metrics
Expand Down
9 changes: 7 additions & 2 deletions unirest/src/main/java/kong/unirest/core/java/JavaClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.net.http.HttpClient;
import java.net.http.WebSocket;
import java.time.Duration;
import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.function.Function;

Expand Down Expand Up @@ -86,7 +87,11 @@ private java.net.http.HttpRequest getRequest(HttpRequest<?> request) {
.method(
request.getHttpMethod().name(),
new BodyBuilder(request).getBody()
).timeout(Duration.ofMillis(request.getConnectTimeout()));
);

if(!Objects.isNull(request.getRequestTimeout())){
jreq.timeout(Duration.ofMillis(request.getRequestTimeout()));
}

setHeaders(request, jreq);

Expand All @@ -106,7 +111,7 @@ private void setHeaders(HttpRequest<?> request, java.net.http.HttpRequest.Builde
var value = "text/plain";
var charset = request.getBody().get().getCharset();
if (charset != null) {
value = value + "; charset=" + charset.toString();
value = value + "; charset=" + charset;
}
jreq.header(CONTENT_TYPE, value);
}
Expand Down
9 changes: 4 additions & 5 deletions unirest/src/test/java/kong/unirest/core/BaseRequestTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

package kong.unirest.core;

import kong.unirest.core.*;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -54,13 +53,13 @@ void tearDown() throws Exception {

@Test
void connectTimeoutCanOverrrideConfig() {
testConfig.connectTimeout(42);
testConfig.requestTimeout(42);

HttpRequest request = new TestRequest(testConfig);
var request = new TestRequest(testConfig);

assertEquals(42, request.getConnectTimeout());
assertEquals(42, request.getRequestTimeout());
request.connectTimeout(111);
assertEquals(111, request.getConnectTimeout());
assertEquals(111, request.getRequestTimeout());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ public RequestAsserts objectMapperIs(ObjectMapper om) {
}

public RequestAsserts hasTimeout(int timeout) {
assertEquals(timeout, actual.getConnectTimeout());
assertEquals(timeout, actual.getRequestTimeout());
return this;
}

Expand Down

0 comments on commit 6eadf15

Please sign in to comment.