From dceb0ee142c31eb805434fd533e81abb26f73ab3 Mon Sep 17 00:00:00 2001 From: Darin Kelkhoff Date: Tue, 19 Dec 2023 15:12:52 -0600 Subject: [PATCH] CE-775 add timeouts to outbound http calls --- .../module/api/actions/BaseAPIActionUtil.java | 67 ++++++++++- .../api/actions/BaseAPIActionUtilTest.java | 107 ++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) diff --git a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java index bdcb5fe6..a14b23b1 100644 --- a/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java +++ b/qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java @@ -81,6 +81,7 @@ import org.apache.http.HttpEntity; import org.apache.http.HttpEntityEnclosingRequest; import org.apache.http.HttpResponse; import org.apache.http.HttpStatus; +import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpDelete; import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; @@ -1048,7 +1049,7 @@ public class BaseAPIActionUtil ////////////////////////////////////////////////////// // make sure to use closeable client to avoid leaks // ////////////////////////////////////////////////////// - try(CloseableHttpClient httpClient = HttpClientBuilder.create().build()) + try(CloseableHttpClient httpClient = buildHttpClient()) { //////////////////////////////////////////////////////////// // call utility methods that populate data in the request // @@ -1153,6 +1154,25 @@ public class BaseAPIActionUtil + /******************************************************************************* + ** Build the default HttpClient used by the makeRequest method + *******************************************************************************/ + protected CloseableHttpClient buildHttpClient() + { + /////////////////////////////////////////////////////////////////////////////////////// + // do we want this?? .setConnectionManager(new PoolingHttpClientConnectionManager()) // + // needs some good scrutiny. // + /////////////////////////////////////////////////////////////////////////////////////// + return HttpClientBuilder.create() + .setDefaultRequestConfig(RequestConfig.custom() + .setConnectTimeout(getConnectionTimeoutMillis()) + .setConnectionRequestTimeout(getConnectionRequestTimeoutMillis()) + .setSocketTimeout(getSocketTimeoutMillis()).build()) + .build(); + } + + + /******************************************************************************* ** *******************************************************************************/ @@ -1439,6 +1459,51 @@ public class BaseAPIActionUtil + /******************************************************************************* + ** For the HttpClientBuilder RequestConfig, specify its ConnectionTimeout. See + ** - https://www.baeldung.com/httpclient-timeout + ** - https://hc.apache.org/httpcomponents-client-5.1.x/current/httpclient5/apidocs/org/apache/hc/client5/http/config/RequestConfig.Builder.html + *******************************************************************************/ + protected int getConnectionTimeoutMillis() + { + ////////////// + // 1 minute // + ////////////// + return (60 * 1000); + } + + + + /******************************************************************************* + ** For the HttpClientBuilder RequestConfig, specify its ConnectionRequestTimeout. See + ** - https://www.baeldung.com/httpclient-timeout + ** - https://hc.apache.org/httpcomponents-client-5.1.x/current/httpclient5/apidocs/org/apache/hc/client5/http/config/RequestConfig.Builder.html + *******************************************************************************/ + protected int getConnectionRequestTimeoutMillis() + { + ////////////// + // 1 minute // + ////////////// + return (60 * 1000); + } + + + + /******************************************************************************* + ** For the HttpClientBuilder RequestConfig, specify its ConnectionRequestTimeout. See + ** - https://www.baeldung.com/httpclient-timeout + ** - https://hc.apache.org/httpcomponents-client-5.1.x/current/httpclient5/apidocs/org/apache/hc/client5/http/config/RequestConfig.Builder.html + *******************************************************************************/ + protected int getSocketTimeoutMillis() + { + /////////////// + // 3 minutes // + /////////////// + return (3 * 60 * 1000); + } + + + /******************************************************************************* ** *******************************************************************************/ diff --git a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java index 25b1af88..4fb4d8a9 100644 --- a/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java +++ b/qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java @@ -22,11 +22,18 @@ package com.kingsrook.qqq.backend.module.api.actions; +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.io.PrintWriter; import java.io.Serializable; +import java.net.ServerSocket; +import java.net.Socket; +import java.net.SocketTimeoutException; import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.List; import java.util.Map; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import com.kingsrook.qqq.backend.core.actions.tables.CountAction; import com.kingsrook.qqq.backend.core.actions.tables.DeleteAction; @@ -36,6 +43,7 @@ import com.kingsrook.qqq.backend.core.actions.tables.QueryAction; import com.kingsrook.qqq.backend.core.actions.tables.UpdateAction; import com.kingsrook.qqq.backend.core.context.QContext; import com.kingsrook.qqq.backend.core.exceptions.QException; +import com.kingsrook.qqq.backend.core.logging.QLogger; import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountInput; import com.kingsrook.qqq.backend.core.model.actions.tables.count.CountOutput; import com.kingsrook.qqq.backend.core.model.actions.tables.delete.DeleteInput; @@ -82,6 +90,8 @@ import static org.junit.jupiter.api.Assertions.fail; *******************************************************************************/ class BaseAPIActionUtilTest extends BaseTest { + private static final QLogger LOG = QLogger.getLogger(BaseAPIActionUtilTest.class); + private static MockApiUtilsHelper mockApiUtilsHelper = new MockApiUtilsHelper(); @@ -822,6 +832,103 @@ class BaseAPIActionUtilTest extends BaseTest + /******************************************************************************* + ** + *******************************************************************************/ + @Test + void testTimeouts() throws QException + { + ShortTimeoutActionUtil shortTimeoutActionUtil = new ShortTimeoutActionUtil(); + shortTimeoutActionUtil.setBackendMetaData((APIBackendMetaData) QContext.getQInstance().getBackend(TestUtils.MOCK_BACKEND_NAME)); + + ///////////////////////////////////////////////////////////// + // make sure we work correctly with a large enough timeout // + ///////////////////////////////////////////////////////////// + { + startSimpleHttpServer(8888); + HttpGet request = new HttpGet("http://localhost:8888"); + shortTimeoutActionUtil.setTimeoutMillis(3000); + + shortTimeoutActionUtil.makeRequest(QContext.getQInstance().getTable(TestUtils.MOCK_TABLE_NAME), request); + } + + //////////////////////////////////////////////// + // make sure we fail with a too-small timeout // + //////////////////////////////////////////////// + { + startSimpleHttpServer(8889); + HttpGet request = new HttpGet("http://localhost:8889"); + shortTimeoutActionUtil.setTimeoutMillis(1); + + assertThatThrownBy(() -> shortTimeoutActionUtil.makeRequest(QContext.getQInstance().getTable(TestUtils.MOCK_TABLE_NAME), request)) + .hasRootCauseInstanceOf(SocketTimeoutException.class) + .rootCause().hasMessageContaining("timed out"); + } + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + private static void startSimpleHttpServer(int port) + { + Executors.newSingleThreadExecutor().submit(() -> + { + LOG.info("Listening on " + port); + try(ServerSocket serverSocket = new ServerSocket(port)) + { + Socket clientSocket = serverSocket.accept(); + PrintWriter out = new PrintWriter(clientSocket.getOutputStream(), true); + BufferedReader in = new BufferedReader(new InputStreamReader(clientSocket.getInputStream())); + String greeting = in.readLine(); + LOG.info("Read: " + greeting); + SleepUtils.sleep(1, TimeUnit.SECONDS); + out.println("HTTP/1.1 200 OK"); + out.close(); + clientSocket.close(); + } + catch(Exception e) + { + LOG.info("Exception in simple http server", e); + } + }); + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + static class ShortTimeoutActionUtil extends BaseAPIActionUtil + { + private int timeoutMillis = 1; + + + + /******************************************************************************* + ** Setter for timeoutMillis + ** + *******************************************************************************/ + public void setTimeoutMillis(int timeoutMillis) + { + this.timeoutMillis = timeoutMillis; + } + + + + /******************************************************************************* + ** + *******************************************************************************/ + @Override + protected int getSocketTimeoutMillis() + { + return (timeoutMillis); + } + } + + + /******************************************************************************* ** *******************************************************************************/