james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From btell...@apache.org
Subject [2/9] james-project git commit: JAMES-2263 Correct mailetContainer error handling
Date Wed, 03 Jan 2018 03:20:36 GMT
JAMES-2263 Correct mailetContainer error handling


Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/d730186e
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/d730186e
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/d730186e

Branch: refs/heads/master
Commit: d730186ee7b71c69201d710db58acbad73b2a66c
Parents: 77125c1
Author: apptaro <apptaro@gmail.com>
Authored: Mon Dec 18 17:03:03 2017 +0900
Committer: benwa <btellier@linagora.com>
Committed: Wed Jan 3 10:16:13 2018 +0700

----------------------------------------------------------------------
 .../mailetcontainer/impl/ProcessorUtil.java     |  6 +-
 .../impl/camel/CamelProcessor.java              |  7 +--
 .../impl/camel/MatcherSplitter.java             | 28 ++++-----
 .../jmx/JMXStateMailetProcessorListener.java    | 12 ++--
 .../lib/AbstractStateMailetProcessor.java       |  8 +--
 .../lib/AbstractStateMailetProcessorTest.java   | 61 ++++++++------------
 6 files changed, 51 insertions(+), 71 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/d730186e/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/ProcessorUtil.java
----------------------------------------------------------------------
diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/ProcessorUtil.java
b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/ProcessorUtil.java
index 25adf18..f1bfab6 100644
--- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/ProcessorUtil.java
+++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/ProcessorUtil.java
@@ -44,11 +44,8 @@ public class ProcessorUtil {
      *            the matcher or mailet than generated the exception
      * @param nextState
      *            the next state to set
-     *
-     * @throws MessagingException
-     *             thrown always, rethrowing the passed in exception
      */
-    public static void handleException(MessagingException me, Mail mail, String offendersName,
String nextState, Logger logger) throws MessagingException {
+    public static void handleException(Exception me, Mail mail, String offendersName, String
nextState, Logger logger) {
         mail.setState(nextState);
         StringWriter sout = new StringWriter();
         PrintWriter out = new PrintWriter(sout, true);
@@ -66,7 +63,6 @@ public class ProcessorUtil {
         String errorString = sout.toString();
         mail.setErrorMessage(errorString);
         logger.error(errorString);
-        throw me;
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/james-project/blob/d730186e/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/CamelProcessor.java
----------------------------------------------------------------------
diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/CamelProcessor.java
b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/CamelProcessor.java
index 1723034..d82f127 100644
--- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/CamelProcessor.java
+++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/CamelProcessor.java
@@ -22,8 +22,6 @@ import java.io.Closeable;
 import java.util.List;
 import java.util.Locale;
 
-import javax.mail.MessagingException;
-
 import org.apache.camel.Exchange;
 import org.apache.camel.Processor;
 import org.apache.james.mailetcontainer.impl.MailetConfigImpl;
@@ -70,7 +68,7 @@ public class CamelProcessor implements Processor {
         Mail mail = exchange.getIn().getBody(Mail.class);
         long start = System.currentTimeMillis();
         TimeMetric timeMetric = metricFactory.timer(mailet.getClass().getSimpleName());
-        MessagingException ex = null;
+        Exception ex = null;
         try (Closeable closeable =
                  MDCBuilder.create()
                      .addContext(MDCBuilder.PROTOCOL, "MAILET")
@@ -84,7 +82,7 @@ public class CamelProcessor implements Processor {
                      .build()) {
             MailetPipelineLogging.logBeginOfMailetProcess(mailet, mail);
             mailet.service(mail);
-        } catch (MessagingException me) {
+        } catch (Exception me) {
             ex = me;
             String onMailetException = null;
 
@@ -101,6 +99,7 @@ public class CamelProcessor implements Processor {
                 // ignore the exception and continue
                 // this option should not be used if the mail object can be
                 // changed by the mailet
+                LOGGER.warn("Encountered error while executing mailet {}. Ignoring it.",
mailet, ex);
                 ProcessorUtil.verifyMailAddresses(mail.getRecipients());
             } else {
                 ProcessorUtil.handleException(me, mail, mailet.getMailetConfig().getMailetName(),
onMailetException, LOGGER);

http://git-wip-us.apache.org/repos/asf/james-project/blob/d730186e/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/MatcherSplitter.java
----------------------------------------------------------------------
diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/MatcherSplitter.java
b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/MatcherSplitter.java
index 36c77c2..64f00c1 100644
--- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/MatcherSplitter.java
+++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/camel/MatcherSplitter.java
@@ -20,7 +20,6 @@
 package org.apache.james.mailetcontainer.impl.camel;
 
 import java.io.Closeable;
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -32,18 +31,18 @@ import org.apache.camel.Body;
 import org.apache.camel.ExchangeProperty;
 import org.apache.camel.Handler;
 import org.apache.camel.InOnly;
-import org.apache.james.server.core.MailImpl;
+import org.apache.james.core.MailAddress;
 import org.apache.james.mailetcontainer.impl.ProcessorUtil;
 import org.apache.james.mailetcontainer.lib.AbstractStateMailetProcessor.MailetProcessorListener;
 import org.apache.james.metrics.api.MetricFactory;
 import org.apache.james.metrics.api.TimeMetric;
+import org.apache.james.server.core.MailImpl;
 import org.apache.james.util.MDCBuilder;
 import org.apache.mailet.Mail;
-import org.apache.james.core.MailAddress;
 import org.apache.mailet.Matcher;
 import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 
 /**
@@ -52,20 +51,21 @@ import com.google.common.collect.ImmutableList;
  */
 @InOnly
 public class MatcherSplitter {
+    private static final Logger LOGGER = LoggerFactory.getLogger(MatcherSplitter.class);
 
     /** Headername which is used to indicate that the matcher matched */
-    public final static String MATCHER_MATCHED_ATTRIBUTE = "matched";
+    public static final String MATCHER_MATCHED_ATTRIBUTE = "matched";
 
     /** Headername under which the matcher is stored */
-    public final static String MATCHER_PROPERTY = "matcher";
+    public static final String MATCHER_PROPERTY = "matcher";
 
-    public final static String ON_MATCH_EXCEPTION_PROPERTY = "onMatchException";
+    public static final String ON_MATCH_EXCEPTION_PROPERTY = "onMatchException";
 
-    public final static String LOGGER_PROPERTY = "logger";
+    public static final String LOGGER_PROPERTY = "logger";
 
-    public final static String MAILETCONTAINER_PROPERTY = "container";
+    public static final String MAILETCONTAINER_PROPERTY = "container";
 
-    public final static String METRIC_FACTORY = "metricFactory";
+    public static final String METRIC_FACTORY = "metricFactory";
 
     /**
      * Generate a List of MailMessage instances for the give @Body. This is done
@@ -89,7 +89,7 @@ public class MatcherSplitter {
         Collection<MailAddress> matchedRcpts = null;
         Collection<MailAddress> origRcpts = new ArrayList<>(mail.getRecipients());
         long start = System.currentTimeMillis();
-        MessagingException ex = null;
+        Exception ex = null;
         TimeMetric timeMetric = metricFactory.timer(matcher.getClass().getSimpleName());
 
         try {
@@ -120,7 +120,7 @@ public class MatcherSplitter {
                     ProcessorUtil.verifyMailAddresses(matchedRcpts);
                 }
 
-            } catch (MessagingException me) {
+            } catch (Exception me) {
                 ex = me;
                 if (onMatchException == null) {
                     onMatchException = Mail.ERROR;
@@ -130,15 +130,15 @@ public class MatcherSplitter {
                 if (onMatchException.compareTo("nomatch") == 0) {
                     // In case the matcher returned null, create an empty
                     // Collection
+                    LOGGER.warn("Encountered error while executing matcher {}. Matching none.",
matcher, ex);
                     matchedRcpts = new ArrayList<>(0);
                 } else if (onMatchException.compareTo("matchall") == 0) {
+                    LOGGER.warn("Encountered error while executing matcher {}. matching all.",
matcher, ex);
                     matchedRcpts = mail.getRecipients();
                     // no need to verify addresses
                 } else {
                     ProcessorUtil.handleException(me, mail, matcher.getMatcherConfig().getMatcherName(),
onMatchException, logger);
                 }
-            } catch (IOException e) {
-                throw Throwables.propagate(e);
             }
 
             // check if the matcher matched

http://git-wip-us.apache.org/repos/asf/james-project/blob/d730186e/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/jmx/JMXStateMailetProcessorListener.java
----------------------------------------------------------------------
diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/jmx/JMXStateMailetProcessorListener.java
b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/jmx/JMXStateMailetProcessorListener.java
index 9393490..496b109 100644
--- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/jmx/JMXStateMailetProcessorListener.java
+++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/impl/jmx/JMXStateMailetProcessorListener.java
@@ -26,17 +26,16 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
-import javax.mail.MessagingException;
 import javax.management.JMException;
 import javax.management.MBeanServer;
 import javax.management.MalformedObjectNameException;
 import javax.management.ObjectName;
 
+import org.apache.james.core.MailAddress;
 import org.apache.james.lifecycle.api.Disposable;
 import org.apache.james.mailetcontainer.impl.matchers.CompositeMatcher;
 import org.apache.james.mailetcontainer.lib.AbstractStateMailetProcessor;
 import org.apache.james.mailetcontainer.lib.AbstractStateMailetProcessor.MailetProcessorListener;
-import org.apache.james.core.MailAddress;
 import org.apache.mailet.Mailet;
 import org.apache.mailet.Matcher;
 
@@ -65,10 +64,9 @@ public class JMXStateMailetProcessorListener implements MailetProcessorListener,
     /**
      * @see
      * org.apache.james.mailetcontainer.lib.AbstractStateMailetProcessor.MailetProcessorListener
-     * #afterMailet(org.apache.mailet.Mailet, java.lang.String, java.lang.String, long,
-     * javax.mail.MessagingException)
+     * #afterMailet(org.apache.mailet.Mailet, java.lang.String, java.lang.String, long, java.lang.Exception)
      */
-    public void afterMailet(Mailet m, String mailName, String state, long processTime, MessagingException
e) {
+    public void afterMailet(Mailet m, String mailName, String state, long processTime, Exception
e) {
         MailetManagement mgmt = mailetMap.get(m);
         if (mgmt != null) {
             mgmt.update(processTime, e == null);
@@ -79,9 +77,9 @@ public class JMXStateMailetProcessorListener implements MailetProcessorListener,
      * @see
      * org.apache.james.mailetcontainer.lib.AbstractStateMailetProcessor.MailetProcessorListener
      * #afterMatcher(org.apache.mailet.Matcher, java.lang.String, java.util.Collection,
-     * java.util.Collection, long, javax.mail.MessagingException)
+     * java.util.Collection, long, java.lang.Exception)
      */
-    public void afterMatcher(Matcher m, String mailName, Collection<MailAddress> rcpts,
Collection<MailAddress> matches, long processTime, MessagingException e) {
+    public void afterMatcher(Matcher m, String mailName, Collection<MailAddress> rcpts,
Collection<MailAddress> matches, long processTime, Exception e) {
         MatcherManagement mgmt = matcherMap.get(m);
 
         if (mgmt != null) {

http://git-wip-us.apache.org/repos/asf/james-project/blob/d730186e/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessor.java
----------------------------------------------------------------------
diff --git a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessor.java
b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessor.java
index 5911261..960201e 100644
--- a/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessor.java
+++ b/server/mailet/mailetcontainer-camel/src/main/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessor.java
@@ -437,9 +437,9 @@ public abstract class AbstractStateMailetProcessor implements MailProcessor,
Con
          * @param processTime
          *            in ms
          * @param e
-         *            or null if no {@link MessagingException} was thrown
+         *            or null if no Exception was thrown
          */
-        void afterMailet(Mailet m, String mailName, String state, long processTime, MessagingException
e);
+        void afterMailet(Mailet m, String mailName, String state, long processTime, Exception
e);
 
         /**
          * Get called after each {@link Matcher} call was complete
@@ -451,9 +451,9 @@ public abstract class AbstractStateMailetProcessor implements MailProcessor,
Con
          * @param processTime
          *            in ms
          * @param e
-         *            or null if no {@link MessagingException} was thrown
+         *            or null if no Exception was thrown
          */
-        void afterMatcher(Matcher m, String mailName, Collection<MailAddress> recipients,
Collection<MailAddress> matches, long processTime, MessagingException e);
+        void afterMatcher(Matcher m, String mailName, Collection<MailAddress> recipients,
Collection<MailAddress> matches, long processTime, Exception e);
 
     }
 

http://git-wip-us.apache.org/repos/asf/james-project/blob/d730186e/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessorTest.java
----------------------------------------------------------------------
diff --git a/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessorTest.java
b/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessorTest.java
index 47a4baf..3a275b2 100644
--- a/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessorTest.java
+++ b/server/mailet/mailetcontainer-camel/src/test/java/org/apache/james/mailetcontainer/lib/AbstractStateMailetProcessorTest.java
@@ -18,32 +18,31 @@
  ****************************************************************/
 package org.apache.james.mailetcontainer.lib;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import java.io.ByteArrayInputStream;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.concurrent.CountDownLatch;
+
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.configuration.DefaultConfigurationBuilder;
 import org.apache.commons.configuration.HierarchicalConfiguration;
-import org.apache.james.server.core.MailImpl;
+import org.apache.james.core.MailAddress;
 import org.apache.james.mailetcontainer.api.mock.ExceptionThrowingMailet;
 import org.apache.james.mailetcontainer.api.mock.ExceptionThrowingMatcher;
 import org.apache.james.mailetcontainer.api.mock.MockMailet;
 import org.apache.james.mailetcontainer.api.mock.MockMatcher;
 import org.apache.james.mailetcontainer.lib.AbstractStateMailetProcessor.MailetProcessorListener;
+import org.apache.james.server.core.MailImpl;
 import org.apache.mailet.Mail;
-import org.apache.james.core.MailAddress;
 import org.apache.mailet.Mailet;
 import org.apache.mailet.Matcher;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
 import org.junit.Test;
 
-import javax.mail.MessagingException;
-import java.io.ByteArrayInputStream;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.concurrent.CountDownLatch;
-
 public abstract class AbstractStateMailetProcessorTest {
 
     protected abstract AbstractStateMailetProcessor createProcessor(HierarchicalConfiguration
configuration) throws
@@ -78,7 +77,7 @@ public abstract class AbstractStateMailetProcessorTest {
 
             @Override
             public void afterMatcher(Matcher m, String mailName, Collection<MailAddress>
recipients,
-                                     Collection<MailAddress> matches, long processTime,
MessagingException e) {
+                                     Collection<MailAddress> matches, long processTime,
Exception e) {
                 if (MockMatcher.class.equals(m.getClass())) {
                     assertEquals(mail.getName(), mailName);
                     // match one recipient
@@ -90,7 +89,7 @@ public abstract class AbstractStateMailetProcessorTest {
             }
 
             @Override
-            public void afterMailet(Mailet m, String mailName, String state, long processTime,
MessagingException e) {
+            public void afterMailet(Mailet m, String mailName, String state, long processTime,
Exception e) {
                 // check for class name as the terminating  mailet will kick in too
 
                 if (MockMailet.class.equals(m.getClass())) {
@@ -126,7 +125,7 @@ public abstract class AbstractStateMailetProcessorTest {
 
             @Override
             public void afterMatcher(Matcher m, String mailName, Collection<MailAddress>
recipients,
-                                     Collection<MailAddress> matches, long processTime,
MessagingException e) {
+                                     Collection<MailAddress> matches, long processTime,
Exception e) {
                 if (MockMatcher.class.equals(m.getClass())) {
                     assertEquals(mail.getName(), mailName);
                     // match all recipient
@@ -138,7 +137,7 @@ public abstract class AbstractStateMailetProcessorTest {
             }
 
             @Override
-            public void afterMailet(Mailet m, String mailName, String state, long processTime,
MessagingException e) {
+            public void afterMailet(Mailet m, String mailName, String state, long processTime,
Exception e) {
                 // check for class name as the terminating  mailet will kick in too
 
                 if (MockMailet.class.equals(m.getClass())) {
@@ -163,7 +162,7 @@ public abstract class AbstractStateMailetProcessorTest {
     }
 
     @Test
-    public void testMatcherThrowException() throws Exception {
+    public void matcherProcessingShouldNotResultInAnExceptionWhenMatcherThrows() throws Exception
{
         final CountDownLatch latch = new CountDownLatch(1);
         final MailImpl mail = new MailImpl();
         mail.setName(MailImpl.getId());
@@ -176,7 +175,7 @@ public abstract class AbstractStateMailetProcessorTest {
 
             @Override
             public void afterMatcher(Matcher m, String mailName, Collection<MailAddress>
recipients,
-                                     Collection<MailAddress> matches, long processTime,
MessagingException e) {
+                                     Collection<MailAddress> matches, long processTime,
Exception e) {
                 if (ExceptionThrowingMatcher.class.equals(m.getClass())) {
                     assertEquals(mail.getName(), mailName);
                     // match no recipient because of the error
@@ -188,20 +187,14 @@ public abstract class AbstractStateMailetProcessorTest {
             }
 
             @Override
-            public void afterMailet(Mailet m, String mailName, String state, long processTime,
MessagingException e) {
+            public void afterMailet(Mailet m, String mailName, String state, long processTime,
Exception e) {
                 throw new RuntimeException("Should not call any mailet!");
             }
         });
 
         assertEquals(Mail.DEFAULT, mail.getState());
 
-        boolean catched = false;
-        try {
-            processor.service(mail);
-        } catch (MessagingException e) {
-            catched = true;
-        }
-        assertTrue(catched);
+        processor.service(mail);
 
         // the source mail should have state error as the exception was thrown
         assertEquals(Mail.ERROR, mail.getState());
@@ -211,7 +204,7 @@ public abstract class AbstractStateMailetProcessorTest {
     }
 
     @Test
-    public void testMailetThrowException() throws Exception {
+    public void mailetProcessingShouldNotResultInAnExceptionWhenMailetThrows() throws Exception
{
         final CountDownLatch latch = new CountDownLatch(2);
         final MailImpl mail = new MailImpl();
         mail.setName(MailImpl.getId());
@@ -224,7 +217,7 @@ public abstract class AbstractStateMailetProcessorTest {
 
             @Override
             public void afterMatcher(Matcher m, String mailName, Collection<MailAddress>
recipients,
-                                     Collection<MailAddress> matches, long processTime,
MessagingException e) {
+                                     Collection<MailAddress> matches, long processTime,
Exception e) {
                 if (MockMatcher.class.equals(m.getClass())) {
                     assertEquals(mail.getName(), mailName);
                     // match one recipient
@@ -236,7 +229,7 @@ public abstract class AbstractStateMailetProcessorTest {
             }
 
             @Override
-            public void afterMailet(Mailet m, String mailName, String state, long processTime,
MessagingException e) {
+            public void afterMailet(Mailet m, String mailName, String state, long processTime,
Exception e) {
                 if (ExceptionThrowingMailet.class.equals(m.getClass())) {
                     // the name should be not the same as we have a part match
                     assertFalse(mail.getName().equals(mailName));
@@ -249,13 +242,7 @@ public abstract class AbstractStateMailetProcessorTest {
 
         assertEquals(Mail.DEFAULT, mail.getState());
 
-        boolean catched = false;
-        try {
-            processor.service(mail);
-        } catch (MessagingException e) {
-            catched = true;
-        }
-        assertTrue(catched);
+        processor.service(mail);
 
         latch.await();
         processor.destroy();


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message