Rationalize exception handling

Author: acogoluegnes

src/main/java/com/rabbitmq/client/amqp/impl/AmqpConnection.java

@@ -205,7 +205,7 @@ private org.apache.qpid.protonj2.client.Connection connect(
       checkBrokerVersion(connection);
       return connection;
     } catch (ClientException e) {
-      throw ExceptionUtils.convertOnConnection(e);
+      throw ExceptionUtils.convert(e);
     } finally {
       LOGGER.debug("Connection attempt took {}", stopWatch.stop());
     }
@@ -256,6 +256,7 @@ TopologyListener createTopologyListener(AmqpConnectionBuilder builder) {
             return;
           }
           AmqpException exception = ExceptionUtils.convert(event.failureCause());
+          LOGGER.debug("Converted native exception to {}", exception.getClass().getSimpleName());
 
           if (RECOVERY_PREDICATE.test(exception) && this.state() != OPENING) {
             LOGGER.debug("Queueing recovery task, error is {}", exception.getMessage());

src/main/java/com/rabbitmq/client/amqp/impl/ExceptionUtils.java

@@ -51,20 +51,6 @@ static AmqpException convert(ClientException e) {
     return convert(e, null);
   }
 
-  static AmqpException convertOnConnection(ClientException e) {
-    if (e instanceof ClientConnectionRemotelyClosedException) {
-      if (isNetworkError(e)) {
-        throw new AmqpException.AmqpConnectionException(e.getMessage(), e);
-      } else {
-        // likely a TLS error
-        return new AmqpException.AmqpSecurityException(
-            e.getCause() instanceof SSLException ? e.getCause() : e);
-      }
-    } else {
-      return convert(e);
-    }
-  }
-
   static AmqpException convert(ExecutionException e) {
     if (e.getCause() instanceof ClientException) {
       return convert((ClientException) e.getCause());
@@ -78,7 +64,7 @@ static AmqpException convert(ClientException e, String format, Object... args) {
     if (e.getCause() instanceof SSLException) {
       return new AmqpException.AmqpSecurityException(message, e.getCause());
     } else if (e instanceof ClientConnectionSecurityException) {
-      throw new AmqpException.AmqpSecurityException(message, e);
+      return new AmqpException.AmqpSecurityException(message, e);
     } else if (isNetworkError(e)) {
       return new AmqpException.AmqpConnectionException(e.getMessage(), e);
     } else if (e instanceof ClientSessionRemotelyClosedException) {
@@ -101,14 +87,17 @@ static AmqpException convert(ClientException e, String format, Object... args) {
         return new AmqpException.AmqpResourceClosedException(e.getMessage(), e);
       }
     } else if (e instanceof ClientConnectionRemotelyClosedException) {
-      if (!isUnauthorizedAccess(
-          ((ClientConnectionRemotelyClosedException) e).getErrorCondition())) {
+      ErrorCondition errorCondition =
+          ((ClientConnectionRemotelyClosedException) e).getErrorCondition();
+      if (isNetworkError(e) || !isUnauthorizedAccess(errorCondition)) {
         return new AmqpException.AmqpConnectionException(e.getMessage(), e);
+      } else if (e.getCause() instanceof SSLException) {
+        return new AmqpException.AmqpSecurityException(e.getCause());
       } else {
         return new AmqpException(e.getMessage(), e);
       }
     } else {
-      return new AmqpException(e);
+      return new AmqpException(message, e);
     }
   }
 

src/test/java/com/rabbitmq/client/amqp/impl/ExceptionUtilsTest.java

@@ -21,33 +21,13 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import com.rabbitmq.client.amqp.AmqpException;
-import javax.net.ssl.SSLException;
 import org.apache.qpid.protonj2.client.ErrorCondition;
-import org.apache.qpid.protonj2.client.exceptions.ClientConnectionRemotelyClosedException;
-import org.apache.qpid.protonj2.client.exceptions.ClientException;
 import org.apache.qpid.protonj2.client.exceptions.ClientLinkRemotelyClosedException;
 import org.apache.qpid.protonj2.client.exceptions.ClientSessionRemotelyClosedException;
 import org.junit.jupiter.api.Test;
 
 public class ExceptionUtilsTest {
 
-  @Test
-  void convertOnConnectionTest() {
-    assertThat(
-            convertOnConnection(
-                new ClientConnectionRemotelyClosedException("", new RuntimeException())))
-        .isInstanceOf(AmqpException.AmqpSecurityException.class)
-        .hasCauseInstanceOf(ClientConnectionRemotelyClosedException.class);
-    assertThat(
-            convertOnConnection(
-                new ClientConnectionRemotelyClosedException("", new SSLException(""))))
-        .isInstanceOf(AmqpException.AmqpSecurityException.class)
-        .hasCauseInstanceOf(SSLException.class);
-    assertThat(convertOnConnection(new ClientException("")))
-        .isInstanceOf(AmqpException.class)
-        .hasCauseInstanceOf(ClientException.class);
-  }
-
   @Test
   void convertTest() {
     assertThat(

src/test/resources/logback-test.xml

@@ -6,8 +6,8 @@
     </appender>
 
     <logger name="com.rabbitmq.client.amqp" level="warn" />
-    <logger name="com.rabbitmq.client.amqp.EntityRecovery" level="warn" />
-    <logger name="com.rabbitmq.client.amqp.AmqpConnection" level="warn" />
+    <logger name="com.rabbitmq.client.amqp.impl.EntityRecovery" level="warn" />
+    <logger name="com.rabbitmq.client.amqp.impl.AmqpConnection" level="warn" />
     <logger name="org.apache.qpid" level="warn" />
 
     <root level="warn">