I don't recall the details, but I'm pretty sure there are some circumstances when multiple exceptions get passed through this listener. I think we should do something to ensure that exceptions aren't lost under those circumstances, as it's *really* confusing when they are.

--Rafael

[EMAIL PROTECTED] wrote:
Author: aidan
Date: Wed Jul  2 03:05:49 2008
New Revision: 673343

URL: http://svn.apache.org/viewvc?rev=673343&view=rev
Log:
QPID-962 Exception handling was... unpleasing... Fix up of patch from rhs

AMQConnection: Refactor listener and remove list, we're only interested in the 
most recent one anyway. Add get/set for lastException, which can now be any 
Exception

AMQConnectionDelegate_0_8.java: Stop masking/stackign exceptions, just throw 
them.

AMQProtocolHandler.java: attainState can now throw any sort of Exception

AMQStateManager.java: attainState can now throw any Exception

ConnectionTest.java: check that exception cause is not null

Modified:
    
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
    
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_8.java
    
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
    
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
    
incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java

Modified: 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
URL: 
http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java?rev=673343&r1=673342&r2=673343&view=diff
==============================================================================
--- 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
 (original)
+++ 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
 Wed Jul  2 03:05:49 2008
@@ -26,6 +26,7 @@
 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.*;
@@ -234,7 +235,7 @@
     /*
      * The last error code that occured on the connection. Used to return the 
correct exception to the client
      */
-    protected AMQException _lastAMQException = null;
+    protected Exception _lastException = null;
/*
      * The connection meta data
@@ -378,13 +379,20 @@
             _delegate = new AMQConnectionDelegate_0_10(this);
         }
- final ArrayList<JMSException> exceptions = new ArrayList<JMSException>();
-
         class Listener implements ExceptionListener
         {
             public void onException(JMSException e)
             {
-                exceptions.add(e);
+                _lastException = e;
+                try
+                {
+                    
getProtocolHandler().getStateManager().changeState(AMQState.CONNECTION_CLOSED);
+ + }
+                catch (AMQException e1)
+                {
+                    // Wow, badness
+                }
             }
         }
@@ -443,9 +451,6 @@
         // 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 )
@@ -453,8 +458,6 @@
             try
             {
                 makeBrokerConnection(brokerDetails);
-                lastException = null;
-                _connected = true;
             }
             catch (AMQProtocolException pe)
             {
@@ -470,12 +473,14 @@
             }
             catch (Exception e)
             {
-                lastException = e;
-
+                _lastException = e;
+            }
+            if (_lastException != null)
+            {
                 if (_logger.isInfoEnabled())
                 {
                     _logger.info("Unable to connect to broker at " + 
_failoverPolicy.getCurrentBrokerDetails(),
-                            e.getCause());
+                            _lastException.getCause());
                 }
                 retryAllowed = _failoverPolicy.failoverAllowed();
                 brokerDetails = _failoverPolicy.getNextBrokerDetails();
@@ -498,31 +503,16 @@
             {
                 // Eat it, we've hopefully got all the exceptions if this 
happened
             }
-            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 != null)
             {
-                if (lastException.getCause() != null)
+                if (_lastException.getCause() != null)
                 {
-                    message = lastException.getCause().getMessage();
+                    message = _lastException.getCause().getMessage();
                 }
                 else
                 {
-                    message = lastException.getMessage();
+                    message = _lastException.getMessage();
                 }
             }
@@ -534,24 +524,19 @@
                 }
                 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, null);
+            AMQException e = new AMQConnectionFailureException(message, 
_lastException);
- 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;
@@ -1507,4 +1492,14 @@
     {
         return _syncPersistence;
     }
+
+    public Exception getLastException()
+    {
+        return _lastException;
+    }
+
+    public void setLastException(Exception exception)
+    {
+        _lastException = exception;
+    }
 }

Modified: 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_8.java
URL: 
http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_8.java?rev=673343&r1=673342&r2=673343&view=diff
==============================================================================
--- 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_8.java
 (original)
+++ 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_8.java
 Wed Jul  2 03:05:49 2008
@@ -25,7 +25,9 @@
 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;
@@ -76,24 +78,23 @@
         return ((cause instanceof ConnectException) || (cause instanceof 
UnresolvedAddressException));
     }
- public void makeBrokerConnection(BrokerDetails brokerDetail) throws IOException, AMQException
+    public void makeBrokerConnection(BrokerDetails brokerDetail) throws 
AMQException, IOException
     {
-        try
+        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)
         {
-            
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)

Modified: 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
URL: 
http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java?rev=673343&r1=673342&r2=673343&view=diff
==============================================================================
--- 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
 (original)
+++ 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/AMQProtocolHandler.java
 Wed Jul  2 03:05:49 2008
@@ -559,7 +559,7 @@
           _frameListeners.remove(listener);
       }
      */
-    public void attainState(AMQState s) throws AMQException
+    public void attainState(AMQState s) throws Exception
     {
         getStateManager().attainState(s);
     }

Modified: 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
URL: 
http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java?rev=673343&r1=673342&r2=673343&view=diff
==============================================================================
--- 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
 (original)
+++ 
incubator/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
 Wed Jul  2 03:05:49 2008
@@ -102,7 +102,7 @@
     }
- public void attainState(final AMQState s) throws AMQException
+    public void attainState(final AMQState s) throws Exception
     {
         synchronized (_stateLock)
         {
@@ -118,6 +118,11 @@
                 catch (InterruptedException e)
                 {
                     _logger.warn("Thread interrupted");
+                    if (_protocolSession.getAMQConnection().getLastException() 
!= null)
+                    {
+                        throw 
_protocolSession.getAMQConnection().getLastException();
+                    }
+
                 }
if (_currentState != s)
@@ -169,6 +174,11 @@
                 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))

Modified: 
incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
URL: 
http://svn.apache.org/viewvc/incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java?rev=673343&r1=673342&r2=673343&view=diff
==============================================================================
--- 
incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
 (original)
+++ 
incubator/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
 Wed Jul  2 03:05:49 2008
@@ -134,6 +134,7 @@
         }
         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.");


Reply via email to