[JENKINS-62479] Automatically retry rate limited Bitbucket Server requests with exponential backoff (#581)

* Retry request in 5 seconds on getting rate limit error from Bitbucket server

* Fix issues with authenticator not being set to static context object

* Retry requests to Bitbucket server for about 300 times(25 minutes) only on getting rate limit error.

* Return reposnse on interrupted exception while thread is in sleep

* Close HttpClient after use, use exponential backoff, and propagate InterruptedException

* No need for try/finally block around releaseConnection because it just resets the request

* Add rate limiting test for Bitbucket Server using BitbucketIntegrationClientFactory

Co-authored-by: Thejashwini Swamy (RBEI/ETP3) <[email protected]>

Author: dwnusbaum

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

@@ -59,6 +59,7 @@
 import com.fasterxml.jackson.core.type.TypeReference;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.Main;
 import hudson.ProxyConfiguration;
 import hudson.Util;
 import java.awt.image.BufferedImage;
@@ -72,6 +73,8 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.charset.StandardCharsets;
+import java.time.Duration;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -114,6 +117,8 @@
 import org.apache.http.impl.client.StandardHttpRequestRetryHandler;
 import org.apache.http.message.BasicNameValuePair;
 import org.apache.http.util.EntityUtils;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.ProtectedExternally;
 
 import static java.util.Objects.requireNonNull;
 
@@ -150,6 +155,9 @@ public class BitbucketServerAPIClient implements BitbucketApi {
 
     private static final String API_COMMIT_STATUS_PATH = "/rest/build-status/1.0/commits{/hash}";
     private static final Integer DEFAULT_PAGE_LIMIT = 200;
+    private static final int API_RATE_LIMIT_STATUS_CODE = 429;
+    private static final Duration API_RATE_LIMIT_INITIAL_SLEEP = Main.isUnitTest ? Duration.ofMillis(100) : Duration.ofSeconds(5);
+    private static final Duration API_RATE_LIMIT_MAX_SLEEP = Duration.ofMinutes(30);
 
     /**
      * Repository owner.
@@ -335,7 +343,7 @@ private List<BitbucketServerPullRequest> getPullRequests(UriTemplate template)
         return pullRequests;
     }
 
-    private void setupPullRequest(BitbucketServerPullRequest pullRequest, BitbucketServerEndpoint endpoint) throws IOException {
+    private void setupPullRequest(BitbucketServerPullRequest pullRequest, BitbucketServerEndpoint endpoint) throws IOException, InterruptedException {
         // set commit closure to make commit information available when needed, in a similar way to when request branches
         setupClosureForPRBranch(pullRequest);
 
@@ -406,7 +414,7 @@ private void setupClosureForPRBranch(BitbucketServerPullRequest pr) {
         }
     }
 
-    private void callPullRequestChangesById(@NonNull String id) throws IOException {
+    private void callPullRequestChangesById(@NonNull String id) throws IOException, InterruptedException {
         String url = UriTemplate
                 .fromTemplate(API_PULL_REQUEST_CHANGES_PATH)
                 .set("owner", getUserCentricOwner())
@@ -416,7 +424,7 @@ private void callPullRequestChangesById(@NonNull String id) throws IOException {
         getRequest(url);
     }
 
-    private boolean getPullRequestCanMergeById(@NonNull String id) throws IOException {
+    private boolean getPullRequestCanMergeById(@NonNull String id) throws IOException, InterruptedException {
         String url = UriTemplate
                 .fromTemplate(API_PULL_REQUEST_MERGE_PATH)
                 .set("owner", getUserCentricOwner())
@@ -436,7 +444,7 @@ private boolean getPullRequestCanMergeById(@NonNull String id) throws IOExceptio
      */
     @Override
     @NonNull
-    public BitbucketPullRequest getPullRequestById(@NonNull Integer id) throws IOException {
+    public BitbucketPullRequest getPullRequestById(@NonNull Integer id) throws IOException, InterruptedException {
         String url = UriTemplate
                 .fromTemplate(API_PULL_REQUEST_PATH)
                 .set("owner", getUserCentricOwner())
@@ -460,7 +468,7 @@ public BitbucketPullRequest getPullRequestById(@NonNull Integer id) throws IOExc
      */
     @Override
     @NonNull
-    public BitbucketRepository getRepository() throws IOException {
+    public BitbucketRepository getRepository() throws IOException, InterruptedException {
         if (repositoryName == null) {
             throw new UnsupportedOperationException(
                     "Cannot get a repository from an API instance that is not associated with a repository");
@@ -482,7 +490,7 @@ public BitbucketRepository getRepository() throws IOException {
      * {@inheritDoc}
      */
     @Override
-    public void postCommitComment(@NonNull String hash, @NonNull String comment) throws IOException {
+    public void postCommitComment(@NonNull String hash, @NonNull String comment) throws IOException, InterruptedException {
         postRequest(
             UriTemplate
                 .fromTemplate(API_COMMIT_COMMENT_PATH)
@@ -500,7 +508,7 @@ public void postCommitComment(@NonNull String hash, @NonNull String comment) thr
      * {@inheritDoc}
      */
     @Override
-    public void postBuildStatus(@NonNull BitbucketBuildStatus status) throws IOException {
+    public void postBuildStatus(@NonNull BitbucketBuildStatus status) throws IOException, InterruptedException {
         postRequest(
             UriTemplate
                 .fromTemplate(API_COMMIT_STATUS_PATH)
@@ -514,7 +522,7 @@ public void postBuildStatus(@NonNull BitbucketBuildStatus status) throws IOExcep
      * {@inheritDoc}
      */
     @Override
-    public boolean checkPathExists(@NonNull String branchOrHash, @NonNull String path) throws IOException {
+    public boolean checkPathExists(@NonNull String branchOrHash, @NonNull String path) throws IOException, InterruptedException {
         String url = UriTemplate
                 .fromTemplate(API_BROWSE_PATH)
                 .set("owner", getUserCentricOwner())
@@ -536,7 +544,7 @@ public boolean checkPathExists(@NonNull String branchOrHash, @NonNull String pat
 
     @CheckForNull
     @Override
-    public String getDefaultBranch() throws IOException {
+    public String getDefaultBranch() throws IOException, InterruptedException {
         String url = UriTemplate
                 .fromTemplate(API_DEFAULT_BRANCH_PATH)
                 .set("owner", getUserCentricOwner())
@@ -591,7 +599,7 @@ private List<BitbucketServerBranch> getServerBranches(String apiPath) throws IOE
     /** {@inheritDoc} */
     @NonNull
     @Override
-    public BitbucketCommit resolveCommit(@NonNull String hash) throws IOException {
+    public BitbucketCommit resolveCommit(@NonNull String hash) throws IOException, InterruptedException {
         String url = UriTemplate
                 .fromTemplate(API_COMMITS_PATH)
                 .set("owner", getUserCentricOwner())
@@ -739,7 +747,7 @@ public List<? extends BitbucketWebHook> getWebHooks() throws IOException, Interr
      * There is no such Team concept in Bitbucket Server but Project.
      */
     @Override
-    public BitbucketTeam getTeam() throws IOException {
+    public BitbucketTeam getTeam() throws IOException, InterruptedException {
         if (userCentric) {
             return null;
         } else {
@@ -807,7 +815,7 @@ public List<BitbucketServerRepository> getRepositories() throws IOException, Int
     }
 
     @Override
-    public boolean isPrivate() throws IOException {
+    public boolean isPrivate() throws IOException, InterruptedException {
         return getRepository().isPrivate();
     }
 
@@ -841,15 +849,15 @@ private <V> List<V> getResources(UriTemplate template, Class<? extends PagedApiR
         return resources;
     }
 
-    protected String getRequest(String path) throws IOException {
+    protected String getRequest(String path) throws IOException, InterruptedException {
         HttpGet httpget = new HttpGet(this.baseURL + path);
 
         if (authenticator != null) {
             authenticator.configureRequest(httpget);
         }
 
         try(CloseableHttpClient client = getHttpClient(httpget);
-                CloseableHttpResponse response = client.execute(httpget, context)) {
+                CloseableHttpResponse response = executeMethod(client, httpget)) {
             String content;
             long len = response.getEntity().getContentLength();
             if (len == 0) {
@@ -892,7 +900,7 @@ private BufferedImage getImageRequest(String path) throws IOException, Interrupt
         }
 
         try (CloseableHttpClient client = getHttpClient(httpget);
-                CloseableHttpResponse response = client.execute(httpget, context)) {
+                CloseableHttpResponse response = executeMethod(client, httpget)) {
             BufferedImage content;
             long len = response.getEntity().getContentLength();
             if (len == 0) {
@@ -995,14 +1003,14 @@ private void setClientProxyParams(String host, HttpClientBuilder builder) {
         }
     }
 
-    private int getRequestStatus(String path) throws IOException {
+    private int getRequestStatus(String path) throws IOException, InterruptedException {
         HttpGet httpget = new HttpGet(this.baseURL + path);
         if (authenticator != null) {
             authenticator.configureRequest(httpget);
         }
 
         try(CloseableHttpClient client = getHttpClient(httpget);
-                CloseableHttpResponse response = client.execute(httpget, context)) {
+                CloseableHttpResponse response = executeMethod(client, httpget)) {
             EntityUtils.consume(response.getEntity());
             return response.getStatusLine().getStatusCode();
         } finally {
@@ -1016,13 +1024,13 @@ private static String getMethodHost(HttpRequestBase method) {
         return scheme + "://" + uri.getAuthority();
     }
 
-    private String postRequest(String path, List<? extends NameValuePair> params) throws IOException {
+    private String postRequest(String path, List<? extends NameValuePair> params) throws IOException, InterruptedException {
         HttpPost request = new HttpPost(this.baseURL + path);
         request.setEntity(new UrlEncodedFormEntity(params));
         return postRequest(request);
     }
 
-    private String postRequest(String path, String content) throws IOException {
+    private String postRequest(String path, String content) throws IOException, InterruptedException {
         HttpPost request = new HttpPost(this.baseURL + path);
         request.setEntity(new StringEntity(content, ContentType.create("application/json", "UTF-8")));
         LOGGER.log(Level.FINEST, content);
@@ -1037,17 +1045,17 @@ private String nameValueToJson(NameValuePair[] params) {
         return o.toString();
     }
 
-    private String postRequest(HttpPost httppost) throws IOException {
+    private String postRequest(HttpPost httppost) throws IOException, InterruptedException {
         return doRequest(httppost);
     }
 
-    private String doRequest(HttpRequestBase request) throws IOException {
+    private String doRequest(HttpRequestBase request) throws IOException, InterruptedException {
         if (authenticator != null) {
             authenticator.configureRequest(request);
         }
 
         try(CloseableHttpClient client = getHttpClient(request);
-                CloseableHttpResponse response = client.execute(request, context)) {
+                CloseableHttpResponse response = executeMethod(client, request)) {
             if (response.getStatusLine().getStatusCode() == HttpStatus.SC_NO_CONTENT) {
                 EntityUtils.consume(response.getEntity());
                 // 204, no content
@@ -1093,13 +1101,13 @@ private String doRequest(HttpRequestBase request) throws IOException {
         }
     }
 
-    private String putRequest(String path, String content) throws IOException {
+    private String putRequest(String path, String content) throws IOException, InterruptedException {
         HttpPut request = new HttpPut(this.baseURL + path);
         request.setEntity(new StringEntity(content, ContentType.create("application/json", "UTF-8")));
         return doRequest(request);
     }
 
-    private String deleteRequest(String path) throws IOException {
+    private String deleteRequest(String path) throws IOException, InterruptedException {
         HttpDelete request = new HttpDelete(this.baseURL + path);
         return doRequest(request);
     }
@@ -1190,4 +1198,36 @@ private Map<String,Object> collectLines(String response, final List<String> line
         }
         return content;
     }
+
+    private CloseableHttpResponse executeMethod(CloseableHttpClient client, HttpRequestBase httpMethod) throws IOException, InterruptedException {
+        CloseableHttpResponse response = executeMethodNoRetry(client, httpMethod, context);
+        Instant start = Instant.now();
+        Instant forcedEnd = start.plus(API_RATE_LIMIT_MAX_SLEEP);
+        Duration sleepDuration = API_RATE_LIMIT_INITIAL_SLEEP;
+        while (response.getStatusLine().getStatusCode() == API_RATE_LIMIT_STATUS_CODE
+                && Instant.now().plus(sleepDuration).isBefore(forcedEnd)) {
+            response.close();
+            httpMethod.releaseConnection();
+            /*
+             * TODO: If The Bitbucket Server API ever starts sending rate limit expiration time, we should
+             * change this to a more precise sleep.
+             * TODO: It would be better to log this to a context-appropriate TaskListener, e.g. an org/repo scan log.
+             */
+            LOGGER.log(Level.FINE, "Bitbucket server API rate limit reached, sleeping for {0} before retrying",
+                    sleepDuration);
+            Thread.sleep(sleepDuration.toMillis());
+            // Duration increases exponentially: 5s, 7s, 10s, 15s, 22s, ... 6m6s, 9m9s.
+            // We will retry at most 13 times and sleep for roughly 27 minutes.
+            sleepDuration = Duration.ofSeconds((int)(sleepDuration.getSeconds() * 1.5));
+            response = executeMethodNoRetry(client, httpMethod, context);
+        }
+        return response;
+    }
+
+    // Exists just so it can be mocked in BitbucketIntegrationClientFactory.
+    @Restricted(ProtectedExternally.class)
+    protected CloseableHttpResponse executeMethodNoRetry(CloseableHttpClient client, HttpRequestBase httpMethod, HttpClientContext context) throws IOException, InterruptedException {
+        return client.execute(httpMethod, context);
+    }
+
 }