Revert "QPID-962 Exception handling was... unpleasing... Fix up of patch from rhs"

This reverts commit 673343.

git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/trunk@673401 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 55 insertions, 62 deletions

diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
index 59bf103089..04f5a6d204 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
@@ -26,7 +26,6 @@ import org.apache.qpid.AMQProtocolException;
 import org.apache.qpid.AMQUnresolvedAddressException;
 import org.apache.qpid.client.failover.FailoverException;
 import org.apache.qpid.client.protocol.AMQProtocolHandler;
-import org.apache.qpid.client.state.AMQState;
 import org.apache.qpid.client.configuration.ClientProperties;
 import org.apache.qpid.exchange.ExchangeDefaults;
 import org.apache.qpid.framing.*;
@@ -235,7 +234,7 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
     /*
      * The last error code that occured on the connection. Used to return the correct exception to the client
      */
-    protected Exception _lastException = null;
+    protected AMQException _lastAMQException = null;
 
     /*
      * The connection meta data
@@ -379,20 +378,13 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
             _delegate = new AMQConnectionDelegate_0_10(this);
         }
 
+        final ArrayList<JMSException> exceptions = new ArrayList<JMSException>();
+
         class Listener implements ExceptionListener
         {
             public void onException(JMSException e)
             {
-                _lastException = e;
-                try
-                {
-                    getProtocolHandler().getStateManager().changeState(AMQState.CONNECTION_CLOSED);
-                    
-                }
-                catch (AMQException e1)
-                {
-                    // Wow, badness
-                }
+                exceptions.add(e);
             }
         }
 
@@ -451,6 +443,9 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
         // We are not currently connected
         _connected = false;
 
+        Exception lastException = new Exception();
+        lastException.initCause(new ConnectException());
+
         // TMG FIXME this seems... wrong...
         boolean retryAllowed = true;
         while (!_connected && retryAllowed )
@@ -458,6 +453,8 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
             try
             {
                 makeBrokerConnection(brokerDetails);
+                lastException = null;
+                _connected = true;
             }
             catch (AMQProtocolException pe)
             {
@@ -473,14 +470,12 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
             }
             catch (Exception e)
             {
-                _lastException = e;
-            }
-            if (_lastException != null)
-            {
+                lastException = e;
+
                 if (_logger.isInfoEnabled())
                 {
                     _logger.info("Unable to connect to broker at " + _failoverPolicy.getCurrentBrokerDetails(),
-                            _lastException.getCause());
+                            e.getCause());
                 }
                 retryAllowed = _failoverPolicy.failoverAllowed();
                 brokerDetails = _failoverPolicy.getNextBrokerDetails();
@@ -503,16 +498,31 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
             {
                 // Eat it, we've hopefully got all the exceptions if this happened
             }
-            
-            if (_lastException != null)
+            if (exceptions.size() > 0)
+            {
+                JMSException e = exceptions.get(0);
+                int code = -1;
+                try
+                {
+                    code = new Integer(e.getErrorCode()).intValue();
+                }
+                catch (NumberFormatException nfe)
+                {
+                    // Ignore this, we have some error codes and messages swapped around
+                }
+
+                throw new AMQConnectionFailureException(AMQConstant.getConstant(code),
+                                                        e.getMessage(), e);
+            }
+            else if (lastException != null)
             {
-                if (_lastException.getCause() != null)
+                if (lastException.getCause() != null)
                 {
-                    message = _lastException.getCause().getMessage();
+                    message = lastException.getCause().getMessage();
                 }
                 else
                 {
-                    message = _lastException.getMessage();
+                    message = lastException.getMessage();
                 }
             }
 
@@ -524,19 +534,24 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
                 }
                 else // can only be "" if getMessage() returned it therfore lastException != null
                 {
-                    message = "Unable to Connect:" + _lastException.getClass();
+                    message = "Unable to Connect:" + lastException.getClass();
                 }
             }
 
-            AMQException e = new AMQConnectionFailureException(message, _lastException);
+            AMQException e = new AMQConnectionFailureException(message, null);
 
-            if (_lastException != null)
+            if (lastException != null)
             {
-                if (_lastException instanceof UnresolvedAddressException)
+                if (lastException instanceof UnresolvedAddressException)
                 {
                     e = new AMQUnresolvedAddressException(message, _failoverPolicy.getCurrentBrokerDetails().toString(),
                                                           null);
                 }
+
+                if (e.getCause() != null)
+                {
+                    e.initCause(lastException);
+	        }
             }
 
             throw e;
@@ -1492,14 +1507,4 @@ public class AMQConnection extends Closeable implements Connection, QueueConnect
     {
         return _syncPersistence;
     }
-
-    public Exception getLastException()
-    {
-        return _lastException;
-    }
-
-    public void setLastException(Exception exception)
-    {
-        _lastException = exception;
-    }
 }
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
index 89b489d7e7..b5b28e0b28 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
@@ -25,9 +25,7 @@ import java.net.ConnectException;
 import java.nio.channels.UnresolvedAddressException;
 import java.text.MessageFormat;
 import java.util.ArrayList;
-import java.util.EnumSet;
 import java.util.Iterator;
-import java.util.Set;
 
 import javax.jms.JMSException;
 import javax.jms.XASession;
@@ -78,23 +76,24 @@ public class AMQConnectionDelegate_8_0 implements AMQConnectionDelegate
         return ((cause instanceof ConnectException) || (cause instanceof UnresolvedAddressException));
     }
 
-    public void makeBrokerConnection(BrokerDetails brokerDetail) throws AMQException, IOException
+    public void makeBrokerConnection(BrokerDetails brokerDetail) throws IOException, AMQException
     {
-        final Set<AMQState> openOrClosedStates =
-                EnumSet.of(AMQState.CONNECTION_OPEN, AMQState.CONNECTION_CLOSED);
-
-        TransportConnection.getInstance(brokerDetail).connect(_conn._protocolHandler, brokerDetail);
-        // this blocks until the connection has been set up or when an error
-        // has prevented the connection being set up
-
-        AMQState state = _conn._protocolHandler.attainState(openOrClosedStates);
-        if(state == AMQState.CONNECTION_OPEN)
+        try
         {
+            TransportConnection.getInstance(brokerDetail).connect(_conn._protocolHandler, brokerDetail);
+            // this blocks until the connection has been set up or when an error
+            // has prevented the connection being set up
+            _conn._protocolHandler.attainState(AMQState.CONNECTION_OPEN);
             _conn._failoverPolicy.attainedConnection();
 
             // Again this should be changed to a suitable notify
             _conn._connected = true;
-        } 
+        }
+        catch (AMQException e)
+        {
+            _conn._lastAMQException = e;
+            throw e;
+        }
     }
 
     public org.apache.qpid.jms.Session createSession(final boolean transacted, final int acknowledgeMode, final int prefetch)
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java b/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
index 1b75d6e829..2d8074eea2 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
@@ -559,7 +559,7 @@ public class AMQProtocolHandler extends IoHandlerAdapter
           _frameListeners.remove(listener);
       }
      */
-    public void attainState(AMQState s) throws Exception
+    public void attainState(AMQState s) throws AMQException
     {
         getStateManager().attainState(s);
     }
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java b/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
index 21f190bd7e..eda1a1f5fd 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
@@ -102,7 +102,7 @@ public class AMQStateManager
     }
 
 
-    public void attainState(final AMQState s) throws Exception
+    public void attainState(final AMQState s) throws AMQException
     {
         synchronized (_stateLock)
         {
@@ -118,11 +118,6 @@ public class AMQStateManager
                 catch (InterruptedException e)
                 {
                     _logger.warn("Thread interrupted");
-                    if (_protocolSession.getAMQConnection().getLastException() != null)
-                    {
-                        throw _protocolSession.getAMQConnection().getLastException();
-                    }
-
                 }
 
                 if (_currentState != s)
@@ -174,11 +169,6 @@ public class AMQStateManager
                 catch (InterruptedException e)
                 {
                     _logger.warn("Thread interrupted");
-                    if (_protocolSession.getAMQConnection().getLastException() != null)
-                    {
-                        throw new AMQException(null, "Could not attain state due to exception",
-                                _protocolSession.getAMQConnection().getLastException());
-                    }
                 }
 
                 if (!stateSet.contains(_currentState))
diff --git a/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java b/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
index 97eed08ab1..f856e8c20b 100644
--- a/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
+++ b/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
@@ -134,7 +134,6 @@ public class ConnectionTest extends TestCase
         }
         catch (AMQException amqe)
         {
-            assertNotNull("No cause set", amqe.getCause());
             if (amqe.getCause().getClass() == Exception.class)
             {
                 System.err.println("QPID-594 : WARNING RACE CONDITION. Unable to determine cause of Connection Failure.");