You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2018/01/03 03:20:36 UTC
[2/9] james-project git commit: JAMES-2263 Correct mailetContainer
error handling
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 <ap...@gmail.com>
Authored: Mon Dec 18 17:03:03 2017 +0900
Committer: benwa <bt...@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