JAMES-2085 Stop swallowing exceptions in JMS related code

apache · Jul 5, 2017 · c5758e6 · c5758e6
1 parent e755858
commit c5758e6
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 252 deletions.
diff --git a/...queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/ActiveMQMailQueue.java b/...queue/queue-activemq/src/main/java/org/apache/james/queue/activemq/ActiveMQMailQueue.java
@@ -207,13 +207,7 @@ protected void produceMail(Session session, Map<String, Object> props, int msgPr
             }
             throw e;
         } finally {
-
-            try {
-                if (producer != null)
-                    producer.close();
-            } catch (JMSException e) {
-                // ignore here
-            }
+            closeProducer(producer);
         }
 
     }
@@ -304,35 +298,16 @@ public long getSize() throws MailQueueException {
                     size = reply.getLong("size");
                     return size;
                 } catch (NumberFormatException e) {
-                    // if we hit this we can't calculate the size so just catch
-                    // it
+                    return super.getSize();
                 }
             }
-
+            return super.getSize();
         } catch (Exception e) {
             throw new MailQueueException("Unable to remove mails", e);
 
         } finally {
-
-            if (consumer != null) {
-
-                try {
-                    consumer.close();
-                } catch (JMSException e1) {
-                    e1.printStackTrace();
-                    // ignore on rollback
-                }
-            }
-
-            if (producer != null) {
-
-                try {
-                    producer.close();
-                } catch (JMSException e1) {
-                    // ignore on rollback
-                }
-            }
-
+            closeConsumer(consumer);
+            closeProducer(producer);
             if (replyTo != null) {
                 try {
 
@@ -342,18 +317,11 @@ public long getSize() throws MailQueueException {
                     // every TemporaryQueue which will never get unregistered
                     replyTo.delete();
                 } catch (JMSException e) {
+                    logger.error("Error while deleting temporary queue", e);
                 }
             }
-            try {
-                if (session != null)
-                    session.close();
-            } catch (JMSException e1) {
-                // ignore here
-            }
+            closeSession(session);
         }
-
-        // if we came to this point we should just fallback to super method
-        return super.getSize();
     }
 
 }
diff --git a/...eue-activemq/src/test/java/org/apache/james/queue/activemq/ActiveMQMailQueueBlobTest.java b/...eue-activemq/src/test/java/org/apache/james/queue/activemq/ActiveMQMailQueueBlobTest.java
@@ -35,8 +35,6 @@ public class ActiveMQMailQueueBlobTest extends ActiveMQMailQueueTest {
     public final static String BASE_DIR = "file://target/james-test";
     private MyFileSystem fs;
 
-    private JMSMailQueue queue;
-
     @Override
     protected ActiveMQConnectionFactory createConnectionFactory() {
         ActiveMQConnectionFactory factory = super.createConnectionFactory();
@@ -61,16 +59,6 @@ public void tearDown() throws Exception {
         }
     }
 
-    @Override
-    public JMSMailQueue getQueue() {
-        return queue;
-    }
-
-    @Override
-    public void setQueue(JMSMailQueue queue) {
-        this.queue = queue;
-    }
-
     @Override
     protected boolean useBlobMessages() {
         return true;