Fix cases where Connection Establishment Timed Out is incorrectly log…

…ged (1.6.x) (SimplePool) (#2178) * make simplepool thread names generic (not related to http/1.1 or http/2 * new custom exceptions * update to use new custom exceptions * correctly classify connection-creation timeouts by checking all future statuses (only WAITING means a timeout). Also update to use new custom exceptions * add attributions and license --------- Co-authored-by: Michael Christoff <[email protected]>
networknt · Apr 10, 2024 · f320032 · f320032
1 parent 4804533
commit f320032
Show file tree

Hide file tree

Showing 7 changed files with 121 additions and 18 deletions.
diff --git a/client/src/main/java/com/networknt/client/simplepool/SimpleConnectionState.java b/client/src/main/java/com/networknt/client/simplepool/SimpleConnectionState.java
@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.ArrayList;
 import java.util.concurrent.ConcurrentHashMap;
+import com.networknt.client.simplepool.exceptions.*;
 
 /***
  * A SimpleConnectionState is a simplified interface for a connection, that also keeps track of the connection's state.
@@ -160,7 +161,7 @@ public SimpleConnectionState(
         // throw exception if connection creation failed
         if(!connection.isOpen()) {
             if(logger.isDebugEnabled()) logger.debug("{} closed connection", logLabel(connection, now));
-            throw new RuntimeException("[" + port(connection) + "] Error creating connection to " + uri.toString());
+            throw new SimplePoolConnectionFailureException("[" + port(connection) + "] Error creating connection to " + uri.toString());
 
             // start life-timer and determine connection type
         } else {
@@ -206,7 +207,7 @@ public synchronized ConnectionToken borrow(long now) throws RuntimeException, Il
 
         if(borrowable(now)) {
             if(closed())
-                throw new RuntimeException("Connection was unexpectedly closed");
+                throw new SimplePoolConnectionClosureException("Connection was unexpectedly closed");
 
             // add connectionToken to the Set of borrowed tokens
             borrowedTokens.add( (connectionToken = new ConnectionToken(connection)) );
@@ -218,7 +219,7 @@ public synchronized ConnectionToken borrow(long now) throws RuntimeException, Il
         }
         else {
             if(closed())
-                throw new RuntimeException("Connection was unexpectedly closed");
+                throw new SimplePoolConnectionClosureException("Connection was unexpectedly closed");
             else
                 throw new IllegalStateException("Attempt made to borrow connection outside of BORROWABLE state");
         }

diff --git a/client/src/main/java/com/networknt/client/simplepool/SimpleURIConnectionPool.java b/client/src/main/java/com/networknt/client/simplepool/SimpleURIConnectionPool.java
@@ -21,11 +21,11 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import java.net.URI;
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ThreadLocalRandom;
+import com.networknt.client.simplepool.exceptions.*;
 
 /***
  SimpleURIConnectionPool is a connection pool for a single URI, which means that it manages a pool of reusable
@@ -108,7 +108,8 @@ public synchronized SimpleConnectionState.ConnectionToken borrow(long createConn
                 connectionState = new SimpleConnectionState(EXPIRY_TIME, createConnectionTimeout, isHttp2, uri, allCreatedConnections, connectionMaker);
                 trackedConnections.add(connectionState);
             } else
-                throw new RuntimeException("An attempt was made to exceed the maximum size of the " + uri.toString() + " connection pool");
+                throw new SimplePoolConnectionLimitReachedException(
+                    "An attempt was made to exceed the maximum size of the " + uri.toString() + " connection pool");
         }
 
         SimpleConnectionState.ConnectionToken connectionToken = connectionState.borrow(now);

diff --git a/...java/com/networknt/client/simplepool/exceptions/SimplePoolConnectionClosureException.java b/...java/com/networknt/client/simplepool/exceptions/SimplePoolConnectionClosureException.java
@@ -0,0 +1,26 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * @author miklish Michael N. Christoff
+ *
+ * testing / QA
+ *   AkashWorkGit
+ *   jaydeepparekh1311
+ */
+
+package com.networknt.client.simplepool.exceptions;
+
+public class SimplePoolConnectionClosureException extends RuntimeException {
+    public SimplePoolConnectionClosureException(String message) { super(message); }
+    public SimplePoolConnectionClosureException(String message, Exception e) { super(message, e); }
+}
diff --git a/...java/com/networknt/client/simplepool/exceptions/SimplePoolConnectionFailureException.java b/...java/com/networknt/client/simplepool/exceptions/SimplePoolConnectionFailureException.java
@@ -0,0 +1,26 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * @author miklish Michael N. Christoff
+ *
+ * testing / QA
+ *   AkashWorkGit
+ *   jaydeepparekh1311
+ */
+
+package com.networknt.client.simplepool.exceptions;
+
+public class SimplePoolConnectionFailureException extends RuntimeException {
+    public SimplePoolConnectionFailureException(String message) { super(message); }
+    public SimplePoolConnectionFailureException(String message, Exception e) { super(message, e); }
+}
diff --git a/...com/networknt/client/simplepool/exceptions/SimplePoolConnectionLimitReachedException.java b/...com/networknt/client/simplepool/exceptions/SimplePoolConnectionLimitReachedException.java
@@ -0,0 +1,26 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * @author miklish Michael N. Christoff
+ *
+ * testing / QA
+ *   AkashWorkGit
+ *   jaydeepparekh1311
+ */
+
+package com.networknt.client.simplepool.exceptions;
+
+public class SimplePoolConnectionLimitReachedException extends RuntimeException {
+    public SimplePoolConnectionLimitReachedException(String message) { super(message); }
+    public SimplePoolConnectionLimitReachedException(String message, Exception e) { super(message, e); }
+}
diff --git a/client/src/main/java/com/networknt/client/simplepool/mock/TestConnectionMaker.java b/client/src/main/java/com/networknt/client/simplepool/mock/TestConnectionMaker.java
@@ -21,9 +21,9 @@
 
 import com.networknt.client.simplepool.SimpleConnection;
 import com.networknt.client.simplepool.SimpleConnectionMaker;
+import com.networknt.client.simplepool.exceptions.*;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import java.lang.reflect.Constructor;
 import java.net.URI;
 import java.util.Set;
@@ -63,7 +63,7 @@ private SimpleConnection instantiateConnection(long createConnectionTimeout, fin
         try {
             connection = future.get(createConnectionTimeout, TimeUnit.SECONDS);
         } catch(Exception e) {
-            throw new RuntimeException("Connection creation timed-out");
+            throw new SimplePoolConnectionFailureException("Connection creation timed-out");
         }
         return connection;
     }

diff --git a/...src/main/java/com/networknt/client/simplepool/undertow/SimpleUndertowConnectionMaker.java b/...src/main/java/com/networknt/client/simplepool/undertow/SimpleUndertowConnectionMaker.java
@@ -22,6 +22,7 @@
 import com.networknt.client.ClientConfig;
 import com.networknt.client.simplepool.SimpleConnection;
 import com.networknt.client.simplepool.SimpleConnectionMaker;
+import com.networknt.client.simplepool.exceptions.*;
 import io.undertow.Undertow;
 import io.undertow.UndertowOptions;
 import io.undertow.client.ClientCallback;
@@ -88,7 +89,7 @@ public void completed(ClientConnection connection) {
 
             @Override
             public void failed(IOException e) {
-                if(logger.isDebugEnabled()) logger.debug("Failed to establish new connection for uri: {}", uri);
+                logger.error("Failed to establish new connection for uri {}: {}", uri, exceptionDetails(e));
                 result.setException(e);
             }
         };
@@ -105,25 +106,34 @@ public void failed(IOException e) {
     /***
      * Never returns null
      *
-     * @param timeoutSeconds
-     * @param future
-     * @return
+     * @param timeoutSeconds connection timeout in seconds
+     * @param future contains future response containing new connection
+     * @return the new Undertow connection wrapped in a SimpleConnection
+     * @throws RuntimeException if connection fails
      */
-    private static SimpleConnection safeConnect(long timeoutSeconds, IoFuture<SimpleConnection> future)
+    private static SimpleConnection safeConnect(long timeoutSeconds, IoFuture<SimpleConnection> future) throws RuntimeException
     {
-        SimpleConnection connection = null;
-
-        if(future.await(timeoutSeconds, TimeUnit.SECONDS) != org.xnio.IoFuture.Status.DONE)
-            throw new RuntimeException("Connection establishment timed out");
+        switch(future.await(timeoutSeconds, TimeUnit.SECONDS)) {
+            case DONE:
+                break;
+            case WAITING:
+                throw new SimplePoolConnectionFailureException("Connection establishment timed out after " + timeoutSeconds + " second(s)");
+            case FAILED:
+                Exception e = future.getException();
+                throw new SimplePoolConnectionFailureException("Connection establishment failed: " + exceptionDetails(e), e);
+            default:
+                throw new SimplePoolConnectionFailureException("Connection establishment failed");
+        }
 
+        SimpleConnection connection = null;
         try {
             connection = future.get();
         } catch (IOException e) {
-            throw new RuntimeException("Connection establishment generated I/O exception", e);
+            throw new SimplePoolConnectionFailureException("Connection establishment generated I/O exception: " + exceptionDetails(e), e);
         }
 
         if(connection == null)
-            throw new RuntimeException("Connection establishment failed (null) - Full connection terminated");
+            throw new SimplePoolConnectionFailureException("Connection establishment failed (null) - Full connection terminated");
 
         return connection;
     }
@@ -196,6 +206,19 @@ private static XnioSsl getSSL(boolean isHttps)
         return SSL.get();
     }
 
+    /***
+     * Handles empty Exception messages for printing in logs (to avoid having "null" in logs for empty Exception messages)
+     *
+     * @param e Exception to look for a detail message in
+     * @return the Exception message, or "" if the Exception does not contain a message
+     */
+    public static String exceptionDetails(Exception e) {
+        if(e == null || e.getMessage() == null)
+            return "";
+        else
+            return e.getMessage();
+    }
+
     public static String port(ClientConnection connection) {
         if(connection == null) return "NULL";
         String url = connection.getLocalAddress().toString();