Merge pull request #66 from krzsztf/http-read-timeouts

Allow customization of HTTP POST/GET timeouts
jenkinsci · Apr 12, 2020 · b3841c5 · b3841c5
2 parents 119acef + b9b566a
commit b3841c5
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 37 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/RemoteBuildConfiguration.java b/src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/RemoteBuildConfiguration.java
@@ -90,11 +90,15 @@ public class RemoteBuildConfiguration extends Builder implements SimpleBuildStep
 	 */
 	private final static Auth2 DEFAULT_AUTH = NullAuth.INSTANCE;
 
+	private static final int DEFAULT_HTTP_GET_READ_TIMEOUT = 10000;
+	private static final int DEFAULT_HTTP_POST_READ_TIMEOUT = 30000;
+
 	/**
 	 * The TTL value of all `queued` items is only 5 minutes.
 	 * That is why we have to ignore user specified poll interval for such items.
 	 */
 	private static final int QUEUED_ITEMS_POLLINTERVALL = 30;
+
 	private static final int DEFAULT_POLLINTERVALL = 10;
 	private static final int connectionRetryLimit = 5;
 
@@ -112,6 +116,8 @@ public class RemoteBuildConfiguration extends Builder implements SimpleBuildStep
 	private boolean trustAllCertificates;
 	private boolean overrideTrustAllCertificates;
 	private boolean preventRemoteBuildQueue;
+	private int httpPostReadTimeout;
+	private int httpGetReadTimeout;
 	private int pollInterval;
 	private boolean blockBuildUntilComplete;
 	private String job;
@@ -133,6 +139,8 @@ public class RemoteBuildConfiguration extends Builder implements SimpleBuildStep
 
 	@DataBoundConstructor
 	public RemoteBuildConfiguration() {
+		httpGetReadTimeout = DEFAULT_HTTP_GET_READ_TIMEOUT;
+		httpPostReadTimeout = DEFAULT_HTTP_POST_READ_TIMEOUT;
 		pollInterval = DEFAULT_POLLINTERVALL;
 	}
 
@@ -207,6 +215,22 @@ public void setPreventRemoteBuildQueue(boolean preventRemoteBuildQueue) {
 		this.preventRemoteBuildQueue = preventRemoteBuildQueue;
 	}
 
+	@DataBoundSetter
+	public void setHttpGetReadTimeout(int readTimeout) {
+		if (readTimeout < 1000)
+			this.httpGetReadTimeout = DEFAULT_HTTP_GET_READ_TIMEOUT;
+		else
+			this.httpGetReadTimeout = readTimeout;
+	}
+
+	@DataBoundSetter
+	public void setHttpPostReadTimeout(int readTimeout) {
+		if (readTimeout < 1000)
+			this.httpPostReadTimeout = DEFAULT_HTTP_POST_READ_TIMEOUT;
+		else
+			this.httpPostReadTimeout = readTimeout;
+	}
+
 	@DataBoundSetter
 	public void setPollInterval(int pollInterval) {
 		if (pollInterval <= 0)
@@ -667,8 +691,8 @@ public Handle performTriggerAndGetQueueId(BuildContext context) throws IOExcepti
 
 		try {
 			ConnectionResponse responseRemoteJob = HttpHelper.tryPost(triggerUrlString, context, cleanedParams,
-					this.getPollInterval(buildInfo.getStatus()), this.getConnectionRetryLimit(), this.getAuth2(), getLock(triggerUrlString),
-					isUseCrumbCache());
+					this.getHttpPostReadTimeout(), this.getPollInterval(buildInfo.getStatus()), this.getConnectionRetryLimit(),
+					this.getAuth2(), getLock(triggerUrlString), isUseCrumbCache());
 			QueueItem queueItem = new QueueItem(responseRemoteJob.getHeader());
 			buildInfo.setQueueId(queueItem.getId());
 			buildInfo = updateBuildInfo(buildInfo, context);
@@ -923,7 +947,8 @@ private String printOffsetConsoleOutput(BuildContext context, String offset, Rem
 	 * @throws IOException          if any HTTP error occurred.
 	 */
 	public ConnectionResponse doGet(String urlString, BuildContext context, RemoteBuildStatus remoteBuildStatus) throws IOException, InterruptedException {
-		return HttpHelper.tryGet(urlString, context, this.getPollInterval(remoteBuildStatus), this.getConnectionRetryLimit(),
+		return HttpHelper.tryGet(urlString, context, this.getHttpGetReadTimeout(),
+				this.getPollInterval(remoteBuildStatus), this.getConnectionRetryLimit(),
 				this.getAuth2(), getLock(urlString));
 	}
 
@@ -1010,6 +1035,14 @@ public String getRemoteJenkinsUrl() {
 		return trimToNull(remoteJenkinsUrl);
 	}
 
+	public int getHttpGetReadTimeout() {
+		return httpGetReadTimeout;
+	}
+
+	public int getHttpPostReadTimeout() {
+		return httpPostReadTimeout;
+	}
+
 	/**
 	 * @return true, if the authorization is overridden in the job configuration,
 	 *         otherwise false.

diff --git a/...va/org/jenkinsci/plugins/ParameterizedRemoteTrigger/pipeline/RemoteBuildPipelineStep.java b/...va/org/jenkinsci/plugins/ParameterizedRemoteTrigger/pipeline/RemoteBuildPipelineStep.java
@@ -119,6 +119,16 @@ public void setPreventRemoteBuildQueue(boolean preventRemoteBuildQueue) {
 		remoteBuildConfig.setPreventRemoteBuildQueue(preventRemoteBuildQueue);
 	}
 
+	@DataBoundSetter
+	public void setHttpGetReadTimeout(int readTimeout) {
+		remoteBuildConfig.setHttpGetReadTimeout(readTimeout);
+	}
+
+	@DataBoundSetter
+	public void setHttpPostReadTimeout(int readTimeout) {
+		remoteBuildConfig.setHttpPostReadTimeout(readTimeout);
+	}
+
 	@DataBoundSetter
 	public void setPollInterval(int pollInterval) {
 		remoteBuildConfig.setPollInterval(pollInterval);
@@ -313,6 +323,14 @@ public boolean getPreventRemoteBuildQueue() {
 		return remoteBuildConfig.getPreventRemoteBuildQueue();
 	}
 
+	public int getHttpGetReadTimeout() {
+		return remoteBuildConfig.getHttpGetReadTimeout();
+	}
+
+	public int getHttpPostReadTimeout() {
+		return remoteBuildConfig.getHttpPostReadTimeout();
+	}
+
 	public int getPollInterval() {
 		return remoteBuildConfig.getPollInterval(RemoteBuildStatus.RUNNING);
 	}

diff --git a/src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/utils/HttpHelper.java b/src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/utils/HttpHelper.java
@@ -428,6 +428,8 @@ public static String buildTriggerUrl(String jobNameOrUrl, String securityToken,
 	 *            parameters to post
 	 * @param numberOfAttempts
 	 *            number of time that the connection has been attempted
+	 * @param readTimeout
+	 *            read timeout in milliseconds
 	 * @param pollInterval
 	 *            interval between each retry in second
 	 * @param retryLimit
@@ -444,8 +446,9 @@ public static String buildTriggerUrl(String jobNameOrUrl, String securityToken,
 	 *
 	 */
 	private static ConnectionResponse sendHTTPCall(String urlString, String requestType, BuildContext context,
-			Collection<String> postParams, int numberOfAttempts, int pollInterval, int retryLimit, Auth2 overrideAuth,
-			StringBuilder rawRespRef, boolean isCrubmCacheEnabled) throws IOException, InterruptedException {
+			Collection<String> postParams, int readTimeout, int numberOfAttempts, int pollInterval, int retryLimit,
+			Auth2 overrideAuth, StringBuilder rawRespRef, boolean isCrubmCacheEnabled)
+			throws IOException, InterruptedException {
 
 		JSONObject responseObject = null;
 		Map<String, List<String>> responseHeader = null;
@@ -466,18 +469,15 @@ private static ConnectionResponse sendHTTPCall(String urlString, String requestT
 			conn.setRequestProperty("Accept", "application/json");
 			conn.setRequestProperty("Accept-Language", "UTF-8");
 			conn.setRequestMethod(requestType);
+			conn.setReadTimeout(readTimeout);
 			addCrumbToConnection(conn, context, overrideAuth, isCrubmCacheEnabled);
 			// wait up to 5 seconds for the connection to be open
 			conn.setConnectTimeout(5000);
 			if (HTTP_POST.equalsIgnoreCase(requestType)) {
-				// use longer timeout during POST due to not performing retries since POST is not idem-potent
-				conn.setReadTimeout(30000);
 				conn.setRequestProperty("Content-Type", "application/x-www-form-urlencoded");
 				conn.setRequestProperty("Content-Length", String.valueOf(postDataBytes.length));
 				conn.setDoOutput(true);
 				conn.getOutputStream().write(postDataBytes);
-			}else {
-				conn.setReadTimeout(10000);
 			}
 
 			SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'");
@@ -561,8 +561,8 @@ private static ConnectionResponse sendHTTPCall(String urlString, String requestT
 
 				context.logger.println("Retry attempt #" + numberOfAttempts + " out of " + retryLimit);
 				numberOfAttempts++;
-				return sendHTTPCall(urlString, requestType, context, postParams, numberOfAttempts, pollInterval,
-						retryLimit, overrideAuth, rawRespRef, isCrubmCacheEnabled);
+				return sendHTTPCall(urlString, requestType, context, postParams, readTimeout,
+						numberOfAttempts, pollInterval, retryLimit, overrideAuth, rawRespRef, isCrubmCacheEnabled);
 
 			} else {
 				// reached the maximum number of retries, time to fail
@@ -579,12 +579,12 @@ private static ConnectionResponse sendHTTPCall(String urlString, String requestT
 	}
 
 	private static ConnectionResponse tryCall(String urlString, String method, BuildContext context,
-			Collection<String> params, int pollInterval, int retryLimit, Auth2 overrideAuth, StringBuilder rawRespRef,
+			Collection<String> params, int readTimeout, int pollInterval, int retryLimit, Auth2 overrideAuth, StringBuilder rawRespRef,
 			Semaphore lock, boolean isCrubmCacheEnabled) throws IOException, InterruptedException {
 		if (lock == null) {
 			context.logger.println("calling remote without locking...");
-			return sendHTTPCall(urlString, method, context, null, 1, pollInterval, retryLimit, overrideAuth, rawRespRef,
-					isCrubmCacheEnabled);
+			return sendHTTPCall(urlString, method, context, null, readTimeout,
+					1, pollInterval, retryLimit, overrideAuth, rawRespRef, isCrubmCacheEnabled);
 		}
 		Boolean isAcquired = null;
 		try {
@@ -600,8 +600,8 @@ private static ConnectionResponse tryCall(String urlString, String method, Build
 				logger.warning("fail to acquire lock because of timeout, skip locking...");
 			}
 
-			ConnectionResponse cr = sendHTTPCall(urlString, method, context, params, 1, pollInterval, retryLimit,
-					overrideAuth, rawRespRef, isCrubmCacheEnabled);
+			ConnectionResponse cr = sendHTTPCall(urlString, method, context, params, readTimeout,
+					1, pollInterval, retryLimit, overrideAuth, rawRespRef, isCrubmCacheEnabled);
 			return cr;
 
 		} finally {
@@ -612,38 +612,45 @@ private static ConnectionResponse tryCall(String urlString, String method, Build
 	}
 
 	public static ConnectionResponse tryPost(String urlString, BuildContext context, Collection<String> params,
-			int pollInterval, int retryLimit, Auth2 overrideAuth, Semaphore lock, boolean isCrubmCacheEnabled)
-			throws IOException, InterruptedException {
+			int readTimeout, int pollInterval, int retryLimit, Auth2 overrideAuth, Semaphore lock,
+			boolean isCrubmCacheEnabled) throws IOException, InterruptedException {
 
-		return tryCall(urlString, HTTP_POST, context, params, pollInterval, retryLimit, overrideAuth, null, lock,isCrubmCacheEnabled);
+		return tryCall(urlString, HTTP_POST, context, params, readTimeout, pollInterval, retryLimit,
+				overrideAuth, null, lock,isCrubmCacheEnabled);
 	}
 
-	public static ConnectionResponse tryGet(String urlString, BuildContext context, int pollInterval, int retryLimit,
-			Auth2 overrideAuth, Semaphore lock) throws IOException, InterruptedException {
-		return tryCall(urlString, HTTP_GET, context, null, pollInterval, retryLimit, overrideAuth, null, lock, false);
+	public static ConnectionResponse tryGet(String urlString, BuildContext context, int readTimeout,
+			int pollInterval, int retryLimit, Auth2 overrideAuth, Semaphore lock)
+			throws IOException, InterruptedException {
+		return tryCall(urlString, HTTP_GET, context, null, readTimeout, pollInterval, retryLimit,
+				overrideAuth, null, lock, false);
 	}
 
-	public static String tryGetRawResp(String urlString, BuildContext context, int pollInterval, int retryLimit,
-			Auth2 overrideAuth, Semaphore lock) throws IOException, InterruptedException {
+	public static String tryGetRawResp(String urlString, BuildContext context, int readTimeout,
+			int pollInterval, int retryLimit, Auth2 overrideAuth, Semaphore lock)
+			throws IOException, InterruptedException {
 		StringBuilder resp = new StringBuilder();
-		tryCall(urlString, HTTP_GET, context, null, pollInterval, retryLimit, overrideAuth, resp, lock, false);
+		tryCall(urlString, HTTP_GET, context, null, readTimeout, pollInterval, retryLimit,
+				overrideAuth, resp, lock, false);
 		return resp.toString();
 	}
 
 	public static ConnectionResponse post(String urlString, BuildContext context, Collection<String> params,
-			int pollInterval, int retryLimit, Auth2 overrideAuth, boolean isCrubmCacheEnabled) throws IOException, InterruptedException {
-		return tryPost(urlString, context, params, pollInterval, retryLimit, overrideAuth, null, isCrubmCacheEnabled);
+			int readTimeout, int pollInterval, int retryLimit, Auth2 overrideAuth, boolean isCrubmCacheEnabled)
+			throws IOException, InterruptedException {
+		return tryPost(urlString, context, params, readTimeout, pollInterval, retryLimit, overrideAuth,
+				null, isCrubmCacheEnabled);
 	}
 
-	public static ConnectionResponse get(String urlString, BuildContext context, int pollInterval, int retryLimit,
-			Auth2 overrideAuth) throws IOException, InterruptedException {
-		return tryGet(urlString, context, pollInterval, retryLimit, overrideAuth, null);
+	public static ConnectionResponse get(String urlString, BuildContext context, int readTimeout,
+			int pollInterval, int retryLimit, Auth2 overrideAuth) throws IOException, InterruptedException {
+		return tryGet(urlString, context, readTimeout, pollInterval, retryLimit, overrideAuth, null);
 	}
 
 	public static String getRawResp(String urlString, String requestType, BuildContext context,
-			Collection<String> postParams, int numberOfAttempts, int pollInterval, int retryLimit, Auth2 overrideAuth)
-			throws IOException, InterruptedException {
-		return tryGetRawResp(urlString, context, pollInterval, retryLimit, overrideAuth, null);
+			Collection<String> postParams, int readTimeout, int numberOfAttempts, int pollInterval,
+			int retryLimit, Auth2 overrideAuth) throws IOException, InterruptedException {
+		return tryGetRawResp(urlString, context, readTimeout, pollInterval, retryLimit, overrideAuth, null);
 	}
 
 }
diff --git a/src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/utils/RestUtils.java b/src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/utils/RestUtils.java
@@ -24,7 +24,8 @@ public static ConnectionResponse cancelQueueItem(String rootUrl, Handle handle,
 		String cancelQueueUrl = String.format("%s/queue/cancelItem?id=%s", rootUrl, handle.getQueueId());
 		ConnectionResponse resp = null;
 		try {
-			resp = HttpHelper.tryPost(cancelQueueUrl, context, null, remoteConfig.getPollInterval(RemoteBuildStatus.QUEUED) * 2, 0,
+			resp = HttpHelper.tryPost(cancelQueueUrl, context, null, remoteConfig.getHttpPostReadTimeout(),
+					remoteConfig.getPollInterval(RemoteBuildStatus.QUEUED) * 2, 0,
 					remoteConfig.getAuth2(), remoteConfig.getLock(cancelQueueUrl), remoteConfig.isUseCrumbCache());
 		} catch (ExceedRetryLimitException e) {
 			// Due to https://issues.jenkins-ci.org/browse/JENKINS-21311, we can't tell
@@ -41,9 +42,9 @@ public static ConnectionResponse stopRemoteJob(Handle handle, BuildContext conte
 
 		RemoteBuildInfo buildInfo = handle.getBuildInfo();
 		String stopJobUrl = String.format("%sstop", buildInfo.getBuildURL());
-		ConnectionResponse resp = HttpHelper.tryPost(stopJobUrl, context, null, remoteConfig.getPollInterval(buildInfo.getStatus()),
-				remoteConfig.getConnectionRetryLimit(), remoteConfig.getAuth2(), remoteConfig.getLock(stopJobUrl),
-				remoteConfig.isUseCrumbCache());
+		ConnectionResponse resp = HttpHelper.tryPost(stopJobUrl, context, null, remoteConfig.getHttpPostReadTimeout(),
+				remoteConfig.getPollInterval(buildInfo.getStatus()), remoteConfig.getConnectionRetryLimit(),
+				remoteConfig.getAuth2(), remoteConfig.getLock(stopJobUrl), remoteConfig.isUseCrumbCache());
 		context.logger.println(String.format("Remote Job:%s was aborted!", buildInfo.getBuildURL()));
 		return resp;
 	}

diff --git a/...enkinsci/plugins/ParameterizedRemoteTrigger/pipeline/RemoteBuildPipelineStep/config.jelly b/...enkinsci/plugins/ParameterizedRemoteTrigger/pipeline/RemoteBuildPipelineStep/config.jelly
@@ -28,6 +28,14 @@
             <f:checkbox />
         </f:entry>
 
+        <f:entry title="HTTP GET read timeout (milliseconds)" field="httpGetReadTimeout">
+            <f:number clazz="positive-number" min="1000" step="1000" default="10000" />
+        </f:entry>
+
+       <f:entry title="HTTP POST read timeout (milliseconds)" field="httpPostReadTimeout">
+            <f:number clazz="positive-number" min="1000" step="1000" default="30000" />
+        </f:entry>
+
         <f:entry title="Poll Interval (seconds)" field="pollInterval">
              <f:number clazz="positive-number" min="1" step="1" default="10" />
         </f:entry>

diff --git a/...t/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/RemoteBuildConfigurationTest.java b/...t/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/RemoteBuildConfigurationTest.java
@@ -116,6 +116,8 @@ private void _testRemoteBuild(boolean authenticate, boolean withParam, FreeStyle
         configuration.setPreventRemoteBuildQueue(false);
         configuration.setBlockBuildUntilComplete(true);
         configuration.setPollInterval(1);
+        configuration.setHttpGetReadTimeout(1000);
+        configuration.setHttpPostReadTimeout(1000);
         configuration.setUseCrumbCache(false);
         configuration.setUseJobInfoCache(false);
         configuration.setEnhancedLogging(true);
@@ -179,6 +181,8 @@ public void testDefaults() throws IOException {
       assertEquals(false, config.getOverrideAuth());
       assertEquals("", config.getParameterFile());
       assertEquals("", config.getParameters());
+      assertEquals(10000, config.getHttpGetReadTimeout());
+      assertEquals(30000, config.getHttpPostReadTimeout());
       assertEquals(10, config.getPollInterval(RemoteBuildStatus.RUNNING));
       assertEquals(false, config.getPreventRemoteBuildQueue());
       assertEquals(null, config.getRemoteJenkinsName());
@@ -200,6 +204,8 @@ public void testDefaultsPipelineStep() throws IOException {
       assertTrue(config.getAuth() instanceof NullAuth);
       assertEquals("", config.getParameterFile());
       assertEquals("", config.getParameters());
+      assertEquals(10000, config.getHttpGetReadTimeout());
+      assertEquals(30000, config.getHttpPostReadTimeout());
       assertEquals(10, config.getPollInterval());
       assertEquals(false, config.getPreventRemoteBuildQueue());
       assertEquals(null, config.getRemoteJenkinsName());