Add default request timeout to FaultTolerantHttpClient

signalapp · Aug 31, 2023 · a1e534a · a1e534a
1 parent ebbe19b
commit a1e534a
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 3 deletions.
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/http/FaultTolerantHttpClient.java b/service/src/main/java/org/whispersystems/textsecuregcm/http/FaultTolerantHttpClient.java
@@ -28,6 +28,7 @@
 public class FaultTolerantHttpClient {
 
   private final HttpClient httpClient;
+  private final Duration defaultRequestTimeout;
   private final ScheduledExecutorService retryExecutor;
   private final Retry retry;
   private final CircuitBreaker breaker;
@@ -40,9 +41,12 @@ public static Builder newBuilder() {
   }
 
   private FaultTolerantHttpClient(String name, HttpClient httpClient, ScheduledExecutorService retryExecutor,
-      RetryConfiguration retryConfiguration, CircuitBreakerConfiguration circuitBreakerConfiguration) {
+      Duration defaultRequestTimeout, RetryConfiguration retryConfiguration,
+      CircuitBreakerConfiguration circuitBreakerConfiguration) {
+
     this.httpClient = httpClient;
     this.retryExecutor = retryExecutor;
+    this.defaultRequestTimeout = defaultRequestTimeout;
     this.breaker = CircuitBreaker.of(name + "-breaker", circuitBreakerConfiguration.toCircuitBreakerConfig());
 
     CircuitBreakerUtil.registerMetrics(breaker, FaultTolerantHttpClient.class);
@@ -61,6 +65,12 @@ private FaultTolerantHttpClient(String name, HttpClient httpClient, ScheduledExe
   }
 
   public <T> CompletableFuture<HttpResponse<T>> sendAsync(HttpRequest request, HttpResponse.BodyHandler<T> bodyHandler) {
+    if (request.timeout().isEmpty()) {
+      request = HttpRequest.newBuilder(request, (n, v) -> true)
+          .timeout(defaultRequestTimeout)
+          .build();
+    }
+
     Supplier<CompletionStage<HttpResponse<T>>> asyncRequest = sendAsync(httpClient, request, bodyHandler);
 
     if (retry != null) {
@@ -83,6 +93,7 @@ public static class Builder {
     private HttpClient.Version version = HttpClient.Version.HTTP_2;
     private HttpClient.Redirect redirect = HttpClient.Redirect.NEVER;
     private Duration connectTimeout = Duration.ofSeconds(10);
+    private Duration requestTimeout = Duration.ofSeconds(60);
 
     private String name;
     private Executor executor;
@@ -120,6 +131,11 @@ public Builder withConnectTimeout(Duration connectTimeout) {
       return this;
     }
 
+    public Builder withRequestTimeout(Duration requestTimeout) {
+      this.requestTimeout = requestTimeout;
+      return this;
+    }
+
     public Builder withRetry(RetryConfiguration retryConfiguration) {
       this.retryConfiguration = retryConfiguration;
       return this;
@@ -164,7 +180,7 @@ public FaultTolerantHttpClient build() {
 
       builder.sslContext(sslConfigurator.createSSLContext());
 
-      return new FaultTolerantHttpClient(name, builder.build(), retryExecutor, retryConfiguration,
+      return new FaultTolerantHttpClient(name, builder.build(), retryExecutor, requestTimeout, retryConfiguration,
           circuitBreakerConfiguration);
     }
   }

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/BraintreeManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/subscriptions/BraintreeManager.java
@@ -19,7 +19,9 @@
 import com.braintreegateway.exceptions.BraintreeException;
 import com.braintreegateway.exceptions.NotFoundException;
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.google.common.annotations.VisibleForTesting;
 import java.math.BigDecimal;
+import java.time.Duration;
 import java.time.Instant;
 import java.util.Collections;
 import java.util.Comparator;
@@ -37,7 +39,6 @@
 import javax.ws.rs.ClientErrorException;
 import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.Response;
-import com.google.common.annotations.VisibleForTesting;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
@@ -74,6 +75,10 @@ public BraintreeManager(final String braintreeMerchantId, final String braintree
             .withCircuitBreaker(circuitBreakerConfiguration)
             .withExecutor(executor)
             .withRetryExecutor(retryExecutor)
+            // Braintree documents its internal timeout at 60 seconds, and we want to make sure we don’t miss
+            // a response
+            // https://developer.paypal.com/braintree/docs/reference/general/best-practices/java#timeouts
+            .withRequestTimeout(Duration.ofSeconds(70))
             .build(), graphqlUri, braintreePublicKey, braintreePrivateKey),
         executor);
   }