Address review comments

Author: jonathanedey

src/main/java/com/google/firebase/internal/ApacheHttp2AsyncEntityProducer.java

@@ -30,7 +30,6 @@
 import org.apache.hc.core5.http.nio.AsyncEntityProducer;
 import org.apache.hc.core5.http.nio.DataStreamChannel;
 
-@SuppressWarnings("deprecation")
 public class ApacheHttp2AsyncEntityProducer implements AsyncEntityProducer {
   private ByteBuffer bytebuf;
   private ByteArrayOutputStream baos;
@@ -106,8 +105,8 @@ public void produce(DataStreamChannel channel) throws IOException {
         try {
           content.writeTo(baos);
         } catch (IOException e) {
-          writeFuture.completeExceptionally(e);
-          // failed(e);
+          failed(e);
+          throw e;
         }
       }
 
@@ -139,11 +138,13 @@ public final Exception getException() {
 
   @Override
   public void releaseResources() {
-    bytebuf.clear();
+    if (bytebuf != null) {
+      bytebuf.clear();
+    }
   }
 
   @VisibleForTesting
   ByteBuffer getBytebuf() {
     return bytebuf;
   }
-}
+}

src/main/java/com/google/firebase/internal/ApacheHttp2Request.java

@@ -29,6 +29,7 @@
 import java.util.concurrent.TimeoutException;
 
 import org.apache.hc.client5.http.ConnectTimeoutException;
+import org.apache.hc.client5.http.HttpHostConnectException;
 import org.apache.hc.client5.http.async.methods.SimpleHttpRequest;
 import org.apache.hc.client5.http.async.methods.SimpleHttpResponse;
 import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder;
@@ -123,10 +124,12 @@ public void cancelled() {
     } catch (ExecutionException e) {
       if (e.getCause() instanceof ConnectTimeoutException) {
         throw new IOException("Connection Timeout", e.getCause());
+      } else if (e.getCause() instanceof HttpHostConnectException) {
+        throw new IOException("Connection exception in request", e.getCause());
       } else if (e.getCause() instanceof H2StreamResetException) {
         throw new IOException("Stream exception in request", e.getCause());
       } else {
-        throw new IOException("Exception in request", e);
+        throw new IOException("Unknown exception in request", e);
       }
     } catch (InterruptedException e) {
       throw new IOException("Request Interrupted", e);

src/main/java/com/google/firebase/internal/ApacheHttp2Transport.java

@@ -24,6 +24,7 @@
 
 import org.apache.hc.client5.http.async.HttpAsyncClient;
 import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder;
+import org.apache.hc.client5.http.config.ConnectionConfig;
 import org.apache.hc.client5.http.config.TlsConfig;
 import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient;
 import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder;
@@ -33,8 +34,10 @@
 import org.apache.hc.core5.http2.HttpVersionPolicy;
 import org.apache.hc.core5.http2.config.H2Config;
 import org.apache.hc.core5.io.CloseMode;
-import org.apache.hc.core5.util.TimeValue;
 
+  /**
+   * HTTP/2 enabled async transport based on the Apache HTTP Client library
+   */
 public final class ApacheHttp2Transport extends HttpTransport {
 
   private final CloseableHttpAsyncClient httpAsyncClient;
@@ -62,14 +65,19 @@ public static CloseableHttpAsyncClient newDefaultHttpAsyncClient() {
   public static HttpAsyncClientBuilder defaultHttpAsyncClientBuilder() {
     PoolingAsyncClientConnectionManager connectionManager = 
         new PoolingAsyncClientConnectionManager();
-    connectionManager.setMaxTotal(100);
-    connectionManager.setDefaultMaxPerRoute(100);
-    connectionManager.closeIdle(TimeValue.of(30, TimeUnit.SECONDS));
+
+    // Set Max total connections and max per route to match google api client limits
+    // https://github.com/googleapis/google-http-java-client/blob/f9d4e15bd3c784b1fd3b0f3468000a91c6f79715/google-http-client-apache-v5/src/main/java/com/google/api/client/http/apache/v5/Apache5HttpTransport.java#L151
+    connectionManager.setMaxTotal(200);
+    connectionManager.setDefaultMaxPerRoute(20);
+    connectionManager.setDefaultConnectionConfig(
+        ConnectionConfig.custom().setTimeToLive(-1, TimeUnit.MILLISECONDS).build());
     connectionManager.setDefaultTlsConfig(
         TlsConfig.custom().setVersionPolicy(HttpVersionPolicy.NEGOTIATE).build());
 
     return HttpAsyncClientBuilder.create()
-      .setH2Config(H2Config.DEFAULT)
+      // Set maxConcurrentStreams to 100 to match the concurrent stream limit of the FCM backend.
+      .setH2Config(H2Config.custom().setMaxConcurrentStreams(100).build())
       .setHttp1Config(Http1Config.DEFAULT)
       .setConnectionManager(connectionManager)
       .setRoutePlanner(new SystemDefaultRoutePlanner(ProxySelector.getDefault()))
@@ -88,15 +96,21 @@ protected ApacheHttp2Request buildRequest(String method, String url) {
     return new ApacheHttp2Request(httpAsyncClient, requestBuilder);
   }
 
+  /**
+   * Gracefully shuts down the connection manager and releases allocated resources. This closes all
+   * connections, whether they are currently used or not.
+   */
   @Override
   public void shutdown() throws IOException {
     httpAsyncClient.close(CloseMode.GRACEFUL);
   }
 
+  /** Returns the Apache HTTP client. */
   public HttpAsyncClient getHttpClient() {
     return httpAsyncClient;
   }
 
+  /** Returns if the underlying HTTP client is mTLS. */
   @Override
   public boolean isMtls() {
     return isMtls;

src/test/java/com/google/firebase/internal/ApacheHttp2TransportIT.java

@@ -58,9 +58,10 @@ public class ApacheHttp2TransportIT {
   private static final ImmutableMap<String, Object> payload = 
       ImmutableMap.<String, Object>of("foo", "bar");
 
-  // Sets a 5 second delay before response
+  // Sets a 5 second delay before server response to simulate a slow network that results in a read timeout.
   private static final String DELAY_URL = "https://nghttp2.org/httpbin/delay/5";
-  private static final String NO_CONNECT_URL = "http://google.com:81";
+  // Points to a unused port that simulates a slow conncetion which results in a conncetion timeout
+  private static final String NO_CONNECT_URL = "https://google.com:81";
   private static final String GET_URL = "https://nghttp2.org/httpbin/get";
   private static final String POST_URL = "https://nghttp2.org/httpbin/post";