Skip to content

fix(DatafileClient): add timeouts to UrlConnections #432

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 2 commits into from
Sep 19, 2022
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 @@ -37,6 +37,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.contains;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doThrow;
Expand Down Expand Up @@ -193,8 +194,6 @@ else if (url == url2) {
verify(client).saveLastModified(urlConnection2);
verify(client).readStream(urlConnection2);
verify(urlConnection2).disconnect();


}


Expand Down Expand Up @@ -303,4 +302,23 @@ public void handlesEmptyStringResponse() throws MalformedURLException {
when(client.execute(any(Client.Request.class), eq(DatafileClient.REQUEST_BACKOFF_TIMEOUT), eq(DatafileClient.REQUEST_RETRIES_POWER))).thenReturn("");
assertEquals("", datafileClient.request(url.toString()));
}

@Test
public void handlesConnectionAndReadTimeout() throws IOException {
URL url = new URL(new DatafileConfig("1", null).getUrl());
when(client.openConnection(url)).thenReturn(urlConnection);
when(urlConnection.getResponseCode()).thenReturn(200);
doThrow(new IOException()).when(urlConnection).connect();

datafileClient.request(url.toString());
ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
verify(client).execute(captor1.capture(), anyInt(), anyInt());
Object response = captor1.getValue().execute();

verify(logger).error(contains("Error making request"), any(IOException.class));
verify(urlConnection).setConnectTimeout(10*1000);
verify(urlConnection).setReadTimeout(60*1000);
verify(urlConnection).disconnect();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
*/
public class DatafileClient {
// easy way to set the connection timeout
public static int CONNECTION_TIMEOUT = 5 * 1000;
public static int CONNECTION_TIMEOUT = 10 * 1000;
public static int READ_TIMEOUT = 60 * 1000;

// the numerical base for the exponential backoff
public static final int REQUEST_BACKOFF_TIMEOUT = 2;
// power the number of retries
Expand Down Expand Up @@ -81,8 +83,10 @@ public String execute() {
}

client.setIfModifiedSince(urlConnection);

// set timeouts for releasing failed connections (default is 0 = no timeout).
urlConnection.setConnectTimeout(CONNECTION_TIMEOUT);
urlConnection.setReadTimeout(READ_TIMEOUT);

urlConnection.connect();

int status = urlConnection.getResponseCode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.slf4j.Logger;

Expand All @@ -33,10 +34,10 @@
import java.net.HttpURLConnection;
import java.net.URL;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertTrue;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.contains;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -71,4 +72,23 @@ public void testEventClient() {

verify(logger).debug("SendEvent completed: {}", event);
}

@Test
public void testConnectionAndReadTimeout() throws IOException {
when(client.openConnection(event.getURL())).thenReturn(urlConnection);
when(urlConnection.getResponseCode()).thenReturn(200);
doThrow(new IOException()).when(urlConnection).getOutputStream();

eventClient.sendEvent(event);


ArgumentCaptor<Client.Request> captor1 = ArgumentCaptor.forClass(Client.Request.class);
verify(client).execute(captor1.capture(), anyInt(), anyInt());
Object response = captor1.getValue().execute();

verify(logger).error(contains("Unable to send event"), any(Event.class), any(IOException.class));
verify(urlConnection).setConnectTimeout(10*1000);
verify(urlConnection).setReadTimeout(60*1000);
verify(urlConnection).disconnect();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
* Makes network requests related to events
*/
class EventClient {
// easy way to set the connection timeout
public static int CONNECTION_TIMEOUT = 10 * 1000;
public static int READ_TIMEOUT = 60 * 1000;

private final Client client;
// Package private and non final so it can easily be mocked for tests
private final Logger logger;
Expand Down Expand Up @@ -57,6 +61,10 @@ public Boolean execute() {
return Boolean.FALSE;
}

// set timeouts for releasing failed connections (default is 0 = no timeout).
urlConnection.setConnectTimeout(CONNECTION_TIMEOUT);
urlConnection.setReadTimeout(READ_TIMEOUT);

urlConnection.setRequestMethod("POST");
urlConnection.setRequestProperty("Content-Type", "application/json");
urlConnection.setDoOutput(true);
Expand Down