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 ro...@apache.org on 2017/01/10 14:18:37 UTC
[22/50] [abbrv] james-project git commit: JAMES-1877 Centralize
decision making in DeliveryRunnable
JAMES-1877 Centralize decision making in DeliveryRunnable
All parts generates ExecutionResults.
All boucing / success / enqueuing decision should be centralized
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/bc52f337
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/bc52f337
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/bc52f337
Branch: refs/heads/master
Commit: bc52f3375231187e3f5093f43ee995c2dce365ce
Parents: c8a3dd9
Author: Benoit Tellier <bt...@linagora.com>
Authored: Thu Dec 1 18:14:25 2016 +0700
Committer: Benoit Tellier <bt...@linagora.com>
Committed: Tue Jan 10 15:12:50 2017 +0700
----------------------------------------------------------------------
.../remoteDelivery/DeliveryRunnable.java | 130 +++++++++----------
1 file changed, 62 insertions(+), 68 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/bc52f337/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java
----------------------------------------------------------------------
diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java
index 45169a1..a3e08ef 100644
--- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java
+++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/remoteDelivery/DeliveryRunnable.java
@@ -186,22 +186,44 @@ public class DeliveryRunnable implements Runnable {
outgoingMailsMetric.increment();
break;
case TEMPORARY_FAILURE:
- // TODO move the incrementing of retries here instead of in fail message
- // Something happened that will delay delivery. Store it back in the retry repository.
- long delay = getNextDelay(DeliveryRetriesHelper.retrieveRetries(mail));
-
- if (configuration.isUsePriority()) {
- // Use lowest priority for retries. See JAMES-1311
- mail.setAttribute(MailPrioritySupport.MAIL_PRIORITY, MailPrioritySupport.LOW_PRIORITY);
- }
- queue.enQueue(mail, delay, TimeUnit.MILLISECONDS);
+ handleTemporaryFailure(mail, executionResult);
break;
case PERMANENT_FAILURE:
- // TODO move boucing logic here
+ bounce(mail, executionResult.getException().orNull());
break;
}
}
+ private void handleTemporaryFailure(Mail mail, ExecutionResult executionResult) throws MailQueue.MailQueueException {
+ if (!mail.getState().equals(Mail.ERROR)) {
+ mail.setState(Mail.ERROR);
+ DeliveryRetriesHelper.initRetries(mail);
+ mail.setLastUpdated(new Date());
+ }
+ int retries = DeliveryRetriesHelper.retrieveRetries(mail);
+
+ if (retries < configuration.getMaxRetries()) {
+ reAttemptDelivery(mail, retries);
+ } else {
+ logger.debug("Bouncing message " + mail.getName() + " after " + retries + " retries");
+ bounce(mail, new Exception("Too many retries failure. Bouncing after " + retries + " retries.", executionResult.getException().orNull()));
+ }
+ }
+
+ private void reAttemptDelivery(Mail mail, int retries) throws MailQueue.MailQueueException {
+ logger.debug("Storing message " + mail.getName() + " into outgoing after " + retries + " retries");
+ DeliveryRetriesHelper.incrementRetries(mail);
+ mail.setLastUpdated(new Date());
+ // Something happened that will delay delivery. Store it back in the retry repository.
+ long delay = getNextDelay(DeliveryRetriesHelper.retrieveRetries(mail));
+
+ if (configuration.isUsePriority()) {
+ // Use lowest priority for retries. See JAMES-1311
+ mail.setAttribute(MailPrioritySupport.MAIL_PRIORITY, MailPrioritySupport.LOW_PRIORITY);
+ }
+ queue.enQueue(mail, delay, TimeUnit.MILLISECONDS);
+ }
+
/**
* We can assume that the recipients of this message are all going to the
* same mail server. We will now rely on the DNS server to do DNS MX record
@@ -215,14 +237,7 @@ public class DeliveryRunnable implements Runnable {
*/
private ExecutionResult deliver(Mail mail, Session session) {
try {
- return Optional.fromNullable(tryDeliver(mail, session))
- .or(failMessage(mail, new MessagingException("No mail server(s) available at this time."), false));
- /*
- * If we get here, we've exhausted the loop of servers without sending
- * the message or throwing an exception. One case where this might
- * happen is if we get a MessagingException on each transport.connect(),
- * e.g., if there is only one server and we get a connect exception.
- */
+ return tryDeliver(mail, session);
} catch (SendFailedException sfe) {
return handleSenderFailedException(mail, sfe);
} catch (MessagingException ex) {
@@ -234,11 +249,15 @@ public class DeliveryRunnable implements Runnable {
// We check whether this is a 5xx error message, which indicates a permanent failure (like account doesn't exist
// or mailbox is full or domain is setup wrong). We fail permanently if this was a 5xx error
- return failMessage(mail, ex, ('5' == ex.getMessage().charAt(0)));
+
+ boolean isPermanent = '5' == ex.getMessage().charAt(0);
+ logger.debug(messageComposer.composeFailLogMessage(mail, ex, isPermanent));
+ return onFailure(isPermanent, ex);
} catch (Exception ex) {
logger.error("Generic exception = permanent failure: " + ex.getMessage(), ex);
// Generic exception = permanent failure
- return failMessage(mail, ex, PERMANENT_FAILURE);
+ logger.debug(messageComposer.composeFailLogMessage(mail, ex, PERMANENT_FAILURE));
+ return permanentFailure(ex);
}
}
@@ -301,7 +320,6 @@ public class DeliveryRunnable implements Runnable {
}
private boolean tryDeliveryToHost(Mail mail, Session session, MimeMessage message, InternetAddress[] addr, HostAddress outgoingMailServer) throws MessagingException {
- boolean success = false;
Properties props = session.getProperties();
if (mail.getSender() == null) {
props.put("mail.smtp.from", "<>");
@@ -325,16 +343,15 @@ public class DeliveryRunnable implements Runnable {
transport = (SMTPTransport) session.getTransport(outgoingMailServer);
transport.setLocalHost( props.getProperty("mail.smtp.localhost", configuration.getHeloNameProvider().getHeloName()) );
if (!connect(outgoingMailServer, transport)) {
- success = false;
+ return false;
}
transport.sendMessage(adaptToTransport(message, transport), addr);
- success = true;
logger.debug("Mail (" + mail.getName() + ") sent successfully to " + outgoingMailServer.getHostName() +
" at " + outgoingMailServer.getHost() + " from " + props.get("mail.smtp.from") + " for " + mail.getRecipients());
+ return true;
} finally {
closeTransport(mail, outgoingMailServer, transport);
}
- return success;
}
private MessagingException handleMessagingException(Mail mail, MessagingException me) throws MessagingException {
@@ -440,7 +457,8 @@ public class DeliveryRunnable implements Runnable {
if (configuration.isDebug())
logger.debug("Invalid recipients: " + recipients);
- deleteMessage = failMessage(mail, sfe, true);
+ logger.debug(messageComposer.composeFailLogMessage(mail, sfe, true));
+ deleteMessage = permanentFailure(sfe);
}
}
@@ -462,11 +480,15 @@ public class DeliveryRunnable implements Runnable {
mail.setRecipients(recipients);
if (configuration.isDebug())
logger.debug("Unsent recipients: " + recipients);
+
if (sfe.getClass().getName().endsWith(".SMTPSendFailedException")) {
int returnCode = (Integer) invokeGetter(sfe, "getReturnCode");
- deleteMessage = failMessage(mail, sfe, returnCode >= 500 && returnCode <= 599);
+ boolean isPermanent = returnCode >= 500 && returnCode <= 599;
+ logger.debug(messageComposer.composeFailLogMessage(mail, sfe, isPermanent));
+ deleteMessage = onFailure(isPermanent, sfe);
} else {
- deleteMessage = failMessage(mail, sfe, false);
+ logger.debug(messageComposer.composeFailLogMessage(mail, sfe, false));
+ deleteMessage = temporaryFailure(sfe);
}
}
}
@@ -593,11 +615,9 @@ public class DeliveryRunnable implements Runnable {
}
private ExecutionResult handleTemporaryResolutionException(Mail mail, String host) {
- logger.info("Temporary problem looking up mail server for host: " + host);
- // temporary problems
- return failMessage(mail,
- new MessagingException("Temporary problem looking up mail server for host: " + host + ". I cannot determine where to send this message."),
- false);
+ MessagingException messagingException = new MessagingException("Temporary problem looking up mail server for host: " + host + ". I cannot determine where to send this message.");
+ logger.debug(messageComposer.composeFailLogMessage(mail, messagingException, false));
+ return temporaryFailure(messagingException);
}
private ExecutionResult handleNoTargetServer(Mail mail, String host) {
@@ -607,9 +627,13 @@ public class DeliveryRunnable implements Runnable {
int retry = DeliveryRetriesHelper.retrieveRetries(mail);
if (retry == 0 || retry > configuration.getDnsProblemRetry()) {
// The domain has no dns entry.. Return a permanent error
- return failMessage(mail, new MessagingException(exceptionBuffer), true);
+ MessagingException messagingException = new MessagingException(exceptionBuffer);
+ logger.debug(messageComposer.composeFailLogMessage(mail, messagingException, true));
+ return permanentFailure(messagingException);
} else {
- return failMessage(mail, new MessagingException(exceptionBuffer), false);
+ MessagingException messagingException = new MessagingException(exceptionBuffer);
+ logger.debug(messageComposer.composeFailLogMessage(mail, messagingException, false));
+ return temporaryFailure(messagingException);
}
}
@@ -687,41 +711,13 @@ public class DeliveryRunnable implements Runnable {
}
}
- /**
- * @param mail org.apache.james.core.MailImpl
- * @param ex javax.mail.MessagingException
- * @param permanent
- * @return boolean Whether the message failed fully and can be deleted
- */
- private ExecutionResult failMessage(Mail mail, Exception ex, boolean permanent) {
- logger.debug(messageComposer.composeFailLogMessage(mail, ex, permanent));
- if (!permanent) {
- if (!mail.getState().equals(Mail.ERROR)) {
- mail.setState(Mail.ERROR);
- DeliveryRetriesHelper.initRetries(mail);
- mail.setLastUpdated(new Date());
- }
-
- int retries = DeliveryRetriesHelper.retrieveRetries(mail);
-
- if (retries < configuration.getMaxRetries()) {
- logger.debug("Storing message " + mail.getName() + " into outgoing after " + retries + " retries");
- DeliveryRetriesHelper.incrementRetries(mail);
- mail.setLastUpdated(new Date());
- return temporaryFailure();
- } else {
- logger.debug("Bouncing message " + mail.getName() + " after " + retries + " retries");
- }
- }
-
+ private void bounce(Mail mail, Exception ex) {
if (mail.getSender() == null) {
logger.debug("Null Sender: no bounce will be generated for " + mail.getName());
- return permanentFailure(ex);
}
if (configuration.getBounceProcessor() != null) {
- // do the new DSN bounce
- // setting attributes for DSN mailet
+ // do the new DSN bounce setting attributes for DSN mailet
mail.setAttribute("delivery-error", messageComposer.getErrorMsg(ex));
mail.setState(configuration.getBounceProcessor());
// re-insert the mail into the spool for getting it passed to the dsn-processor
@@ -732,14 +728,12 @@ public class DeliveryRunnable implements Runnable {
logger.debug("Exception re-inserting failed mail: ", e);
}
} else {
- // do an old style bounce
- bounce(mail, ex);
+ bounceWithMailetContext(mail, ex);
}
- return permanentFailure(ex);
}
- private void bounce(Mail mail, Exception ex) {
+ private void bounceWithMailetContext(Mail mail, Exception ex) {
logger.debug("Sending failure message " + mail.getName());
try {
mailetContext.bounce(mail, messageComposer.composeForBounce(mail, ex));
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org