Skip to content

Improve HTTP client timeouts #275

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

Merged
merged 6 commits into from
Sep 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ abstract class AbstractClientApplicationBase implements IClientApplicationBase {
@Getter
private SSLSocketFactory sslSocketFactory;

@Accessors(fluent = true)
@Getter
private Integer connectTimeoutForDefaultHttpClient;

@Accessors(fluent = true)
@Getter
private Integer readTimeoutForDefaultHttpClient;

@Accessors(fluent = true)
@Getter
protected TokenCache tokenCache;
Expand Down Expand Up @@ -284,6 +292,8 @@ abstract static class Builder<T extends Builder<T>> {
private ITokenCacheAccessAspect tokenCacheAccessAspect;
private AadInstanceDiscoveryResponse aadInstanceDiscoveryResponse;
private String clientCapabilities;
private Integer connectTimeoutForDefaultHttpClient;
private Integer readTimeoutForDefaultHttpClient;

/**
* Constructor to create instance of Builder of client application
Expand Down Expand Up @@ -446,6 +456,34 @@ public T sslSocketFactory(SSLSocketFactory val) {
return self();
}

/**
* Sets the connect timeout value used in HttpsURLConnection connections made by {@link DefaultHttpClient},
* and is not needed if using a custom HTTP client
*
* @param val timeout value in milliseconds
* @return instance of the Builder on which method was called
*/
public T connectTimeoutForDefaultHttpClient(Integer val) {
validateNotNull("connectTimeoutForDefaultHttpClient", val);

connectTimeoutForDefaultHttpClient = val;
return self();
}

/**
* Sets the read timeout value used in HttpsURLConnection connections made by {@link DefaultHttpClient},
* and is not needed if using a custom HTTP client
*
* @param val timeout value in milliseconds
* @return instance of the Builder on which method was called
*/
public T readTimeoutForDefaultHttpClient(Integer val) {
validateNotNull("readTimeoutForDefaultHttpClient", val);

readTimeoutForDefaultHttpClient = val;
return self();
}

T telemetryConsumer(Consumer<List<HashMap<String, String>>> val) {
validateNotNull("telemetryConsumer", val);

Expand Down Expand Up @@ -549,10 +587,12 @@ public T clientCapabilities(Set<String> capabilities) {
telemetryConsumer = builder.telemetryConsumer;
proxy = builder.proxy;
sslSocketFactory = builder.sslSocketFactory;
connectTimeoutForDefaultHttpClient = builder.connectTimeoutForDefaultHttpClient;
readTimeoutForDefaultHttpClient = builder.readTimeoutForDefaultHttpClient;
serviceBundle = new ServiceBundle(
builder.executorService,
builder.httpClient == null ?
new DefaultHttpClient(builder.proxy, builder.sslSocketFactory) :
new DefaultHttpClient(builder.proxy, builder.sslSocketFactory, builder.connectTimeoutForDefaultHttpClient, builder.readTimeoutForDefaultHttpClient) :
builder.httpClient,
new TelemetryManager(telemetryConsumer, builder.onlySendFailureTelemetry));
authenticationAuthority = builder.authenticationAuthority;
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/com/microsoft/aad/msal4j/DefaultHttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ class DefaultHttpClient implements IHttpClient {

private final Proxy proxy;
private final SSLSocketFactory sslSocketFactory;
public int DEFAULTCONNECTIONTIMEOUT = 3000;
public int DEFAULTREADTIMEOUT = 5000;
private int connectTimeout = 10000;
private int readTimeout = 15000;

DefaultHttpClient(Proxy proxy, SSLSocketFactory sslSocketFactory){
DefaultHttpClient(Proxy proxy, SSLSocketFactory sslSocketFactory, Integer connectTimeout, Integer readTimeout){
this.proxy = proxy;
this.sslSocketFactory = sslSocketFactory;
if (connectTimeout != null) this.connectTimeout = connectTimeout;
if (readTimeout != null) this.readTimeout = readTimeout;
}

public IHttpResponse send(HttpRequest httpRequest) throws Exception{
Expand Down Expand Up @@ -78,8 +80,8 @@ private HttpsURLConnection openConnection(final URL finalURL)
connection.setSSLSocketFactory(sslSocketFactory);
}

connection.setConnectTimeout(DEFAULTCONNECTIONTIMEOUT);
connection.setReadTimeout(DEFAULTREADTIMEOUT);
connection.setConnectTimeout(connectTimeout);
connection.setReadTimeout(readTimeout);

return connection;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ private TokenRequestExecutor createMockedTokenRequest() throws URISyntaxExceptio

ServiceBundle serviceBundle = new ServiceBundle(
null,
new DefaultHttpClient(null, null),
new DefaultHttpClient(null, null, null, null),
new TelemetryManager(null, false));

return PowerMock.createPartialMock(
Expand Down