Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cases where Connection Establishment Timed Out is incorrectly logged (1.6.x) (SimplePool) #2178

Merged
merged 5 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)) );
Expand All @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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); }
}
Original file line number Diff line number Diff line change
@@ -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); }
}
Original file line number Diff line number Diff line change
@@ -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); }
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -69,8 +70,8 @@ public SimpleConnection makeConnection(
final Set<SimpleConnection> allCreatedConnections) throws RuntimeException
{
boolean isHttps = uri.getScheme().equalsIgnoreCase("https");
XnioWorker worker = getWorker(isHttp2);
XnioSsl ssl = getSSL(isHttps, isHttp2);
XnioWorker worker = getWorker();
XnioSsl ssl = getSSL(isHttps);
OptionMap connectionOptions = getConnectionOptions(isHttp2);
InetSocketAddress bindAddress = null;

Expand All @@ -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);
}
};
Expand All @@ -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;
}
Expand All @@ -138,10 +148,9 @@ private static OptionMap getConnectionOptions(boolean isHttp2) {
* WARNING: This is called by getSSL(). Therefore, this method must never
* call getSSL(), or any method that transitively calls getSSL()
*
* @param isHttp2 if true, sets worker thread names to show HTTP2
* @return new XnioWorker
*/
private static XnioWorker getWorker(boolean isHttp2)
private static XnioWorker getWorker()
{
if(WORKER.get() != null) return WORKER.get();

Expand All @@ -151,7 +160,7 @@ private static XnioWorker getWorker(boolean isHttp2)

Xnio xnio = Xnio.getInstance(Undertow.class.getClassLoader());
try {
WORKER.set(xnio.createWorker(null, getWorkerOptionMap(isHttp2)));
WORKER.set(xnio.createWorker(null, getWorkerOptionMap()));
} catch (IOException e) {
logger.error("Exception while creating new XnioWorker", e);
throw new RuntimeException(e);
Expand All @@ -160,13 +169,13 @@ private static XnioWorker getWorker(boolean isHttp2)
return WORKER.get();
}

private static OptionMap getWorkerOptionMap(boolean isHttp2)
private static OptionMap getWorkerOptionMap()
{
OptionMap.Builder optionBuild = OptionMap.builder()
.set(Options.WORKER_IO_THREADS, 8)
.set(Options.TCP_NODELAY, true)
.set(Options.KEEP_ALIVE, true)
.set(Options.WORKER_NAME, isHttp2 ? "Callback-HTTP2" : "Callback-HTTP11");
.set(Options.WORKER_NAME, "simplepool");
return optionBuild.getMap();
}

Expand All @@ -176,10 +185,9 @@ private static OptionMap getWorkerOptionMap(boolean isHttp2)
* WARNING: This calls getWorker()
*
* @param isHttps true if this is an HTTPS connection
* @param isHttp2 if true, sets worker thread names to show HTTP2
* @return new XnioSSL
*/
private static XnioSsl getSSL(boolean isHttps, boolean isHttp2)
private static XnioSsl getSSL(boolean isHttps)
{
if(!isHttps) return null;
if(SSL.get() != null) return SSL.get();
Expand All @@ -189,7 +197,7 @@ private static XnioSsl getSSL(boolean isHttps, boolean isHttp2)
if(SSL.get() != null) return SSL.get();

try {
SSL.set(new UndertowXnioSsl(getWorker(isHttp2).getXnio(), OptionMap.EMPTY, BUFFER_POOL, SimpleSSLContextMaker.createSSLContext()));
SSL.set(new UndertowXnioSsl(getWorker().getXnio(), OptionMap.EMPTY, BUFFER_POOL, SimpleSSLContextMaker.createSSLContext()));
} catch (Exception e) {
logger.error("Exception while creating new shared UndertowXnioSsl used to create connections", e);
throw new RuntimeException(e);
Expand All @@ -198,6 +206,19 @@ private static XnioSsl getSSL(boolean isHttps, boolean isHttp2)
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();
Expand Down