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.");