Fix behavior of thrift proxy when getting a new client fails by throwing an exception instead of returning null

2 files changed, 94 insertions, 81 deletions

diff --git a/src/main/java/org/openslx/thrifthelper/ThriftHandler.java b/src/main/java/org/openslx/thrifthelper/ThriftHandler.java
index c9a88cc..121d34b 100644
--- a/src/main/java/org/openslx/thrifthelper/ThriftHandler.java
+++ b/src/main/java/org/openslx/thrifthelper/ThriftHandler.java
@@ -68,35 +68,36 @@ class ThriftHandler<T extends Object> implements InvocationHandler
 				throw cause;
 			}
 		}
-		LOGGER.debug( "Proxying '" + method.getName() + "'" );
 
 		T client = getClient( false );
 		Throwable cause = null;
-		for ( int i = 1; ; i++ ) {
+		for ( int i = 1;; i++ ) {
 			if ( client == null ) {
 				LOGGER.debug( "Transport error - re-initialising ..." );
 				client = getClient( true );
-				if ( client == null )
-					continue;
 			}
-			try {
-				return method.invoke( client, args );
-			} catch ( InvocationTargetException e ) {
-				cause = e.getCause();
-				if ( cause != null && ! ( cause instanceof TTransportException ) )
-					throw cause;
-				client = null;
-				if ( cause == null )
-					cause = e;
+			if ( client != null ) {
+				try {
+					return method.invoke( client, args );
+				} catch ( InvocationTargetException e ) {
+					cause = e.getCause();
+					if ( cause != null && ! ( cause instanceof TTransportException ) )
+						throw cause;
+					client = null;
+					if ( cause == null )
+						cause = e;
+				}
 			}
-			if ( !callback.error( i, method.getName(), cause ) )
+			// Call the error callback. As long as true is returned, keep retrying
+			if ( !callback.error( i, method.getName(), cause ) ) {
 				break;
+			}
 		}
 
 		// Uh oh
 		if ( cause != null )
 			throw cause;
-		return null;
+		throw new TTransportException( "Could not connect" );
 	}
 
 	private T getClient( boolean forceNew )
diff --git a/src/main/java/org/openslx/thrifthelper/ThriftManager.java b/src/main/java/org/openslx/thrifthelper/ThriftManager.java
index 2ffe02c..4ca6a0b 100644
--- a/src/main/java/org/openslx/thrifthelper/ThriftManager.java
+++ b/src/main/java/org/openslx/thrifthelper/ThriftManager.java
@@ -22,7 +22,7 @@ public class ThriftManager
 	{
 		/**
 		 * Called if connecting/reconnecting to the thrift server failed.
-		 *
+		 * 
 		 * @param failCount how many failures occured for this call so far
 		 * @param method name of method that failed
 		 * @param t the exception that occured (may be null)
@@ -52,80 +52,92 @@ public class ThriftManager
 	 */
 	private static SatelliteServer.Iface satClient = (SatelliteServer.Iface)Proxy.newProxyInstance(
 			SatelliteServer.Iface.class.getClassLoader(),
-			new Class[] { SatelliteServer.Iface.class }, new ThriftHandler<SatelliteServer.Client>( SatelliteServer.Client.class, new EventCallback<SatelliteServer.Client>() {
-
-				@Override
-				public SatelliteServer.Client getNewClient()
-				{
-					// first check if we have a sat ip
-					if ( SATELLITE_IP == null ) {
-						LOGGER.error( "Satellite ip adress was not set prior to getting the sat client. Use setSatelliteAddress(<addr>)." );
-						return null;
-					}
-					// ok lets do it
-					TTransport transport = new TFramedTransport( new TSocket( SATELLITE_IP, SATELLITE_PORT, SATELLITE_TIMEOUT ) );
-					try {
-						transport.open();
-					} catch ( TTransportException e ) {
-						LOGGER.error( "Could not open transport to thrift's server with IP: " + SATELLITE_IP );
-						transport.close();
-						return null;
-					}
-					final TProtocol protocol = new TBinaryProtocol( transport );
-					// now we are ready to create the client, according to ClientType!
-					LOGGER.info( "Satellite '" + SATELLITE_IP + "' reachable. Client initialised." );
-					return new SatelliteServer.Client( protocol );
-				}
-
-				@Override
-				public boolean error( int failCount, String method, Throwable t )
-				{
-					return errorCallback != null && errorCallback.thriftError( failCount, method, t );
-				}
-			} ) );
+			new Class[] { SatelliteServer.Iface.class }, new ThriftHandler<SatelliteServer.Client>(
+					SatelliteServer.Client.class, new EventCallback<SatelliteServer.Client>() {
+
+						@Override
+						public SatelliteServer.Client getNewClient()
+						{
+							// first check if we have a sat ip
+							if ( SATELLITE_IP == null ) {
+								LOGGER.error( "Satellite ip adress was not set prior to getting the sat client. Use setSatelliteAddress(<addr>)." );
+								return null;
+							}
+							// ok lets do it
+							TTransport transport = new TFramedTransport(
+									new TSocket(
+											SATELLITE_IP, SATELLITE_PORT, SATELLITE_TIMEOUT ) );
+							try {
+								transport.open();
+							} catch ( TTransportException e ) {
+								LOGGER.error( "Could not open transport to thrift's server with IP: " + SATELLITE_IP );
+								transport.close();
+								return null;
+							}
+							final TProtocol protocol = new TBinaryProtocol(
+									transport );
+							// now we are ready to create the client, according to ClientType!
+							LOGGER.info( "Satellite '" + SATELLITE_IP + "' reachable. Client initialised." );
+							return new SatelliteServer.Client(
+									protocol );
+						}
+
+						@Override
+						public boolean error( int failCount, String method, Throwable t )
+						{
+							return errorCallback != null && errorCallback.thriftError( failCount, method, t );
+						}
+					} ) );
 
 	/**
 	 * Master connection. As its address is known in advance, create the object right away.
 	 */
 	private static MasterServer.Iface masterClient = (MasterServer.Iface)Proxy.newProxyInstance(
 			MasterServer.Iface.class.getClassLoader(),
-			new Class[] { MasterServer.Iface.class }, new ThriftHandler<MasterServer.Client>( MasterServer.Client.class, new EventCallback<MasterServer.Client>() {
-
-				@Override
-				public MasterServer.Client getNewClient()
-				{
-					// first check if we have a sat ip
-					if ( MASTERSERVER_ADDRESS == null ) {
-						LOGGER.error( "Master server adress was not set prior to getting the client. Use setMasterServerAddress(<addr>)." );
-						return null;
-					}
-					// ok lets do it
-					TTransport transport =
-							new TFramedTransport( new TSocket( MASTERSERVER_ADDRESS, MASTERSERVER_PORT, MASTERSERVER_TIMEOUT ) );
-					try {
-						transport.open();
-					} catch ( TTransportException e ) {
-						LOGGER.error( "Could not open transport to thrift's server with IP: " + MASTERSERVER_ADDRESS );
-						transport.close();
-						return null;
-					}
-					final TProtocol protocol = new TBinaryProtocol( transport );
-					// now we are ready to create the client, according to ClientType!
-					return new MasterServer.Client( protocol );
-
-				}
-
-				@Override
-				public boolean error( int failCount, String method, Throwable t )
-				{
-					return errorCallback != null && errorCallback.thriftError( failCount, method, t );
-				}
-			} ) );
+			new Class[] { MasterServer.Iface.class }, new ThriftHandler<MasterServer.Client>(
+					MasterServer.Client.class, new EventCallback<MasterServer.Client>() {
+
+						@Override
+						public MasterServer.Client getNewClient()
+						{
+							// first check if we have a sat ip
+							if ( MASTERSERVER_ADDRESS == null ) {
+								LOGGER.error( "Master server adress was not set prior to getting the client. Use setMasterServerAddress(<addr>)." );
+								return null;
+							}
+							// ok lets do it
+							TTransport transport =
+									new TFramedTransport(
+											new TSocket(
+													MASTERSERVER_ADDRESS, MASTERSERVER_PORT, MASTERSERVER_TIMEOUT ) );
+							try {
+								transport.open();
+							} catch ( TTransportException e ) {
+								LOGGER.error( "Could not open transport to thrift's server with IP: " + MASTERSERVER_ADDRESS );
+								transport.close();
+								return null;
+							}
+							final TProtocol protocol = new TBinaryProtocol(
+									transport );
+							// now we are ready to create the client, according to ClientType!
+							return new MasterServer.Client(
+									protocol );
+
+						}
+
+						@Override
+						public boolean error( int failCount, String method, Throwable t )
+						{
+							synchronized ( LOGGER ) {
+								return errorCallback != null && errorCallback.thriftError( failCount, method, t );
+							}
+						}
+					} ) );
 
 	/**
 	 * Sets the address of the master server
 	 * 
-	 * @param host the ip/hostname  of the master server
+	 * @param host the ip/hostname of the master server
 	 * @return true if setting the address worked, false otherwise
 	 */
 	public static boolean setMasterServerAddress( String host )
@@ -146,7 +158,7 @@ public class ThriftManager
 	/**
 	 * Sets the IP of the satellite to connect to
 	 * 
-	 * @param host the ip/hostname  of the satellite
+	 * @param host the ip/hostname of the satellite
 	 * @return true if setting the address worked, false otherwise
 	 */
 	public static boolean setSatelliteAddress( String host )