archive/phabricator
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Prepare mail transmission to support failover across multiple mailers

Browse Source 
Summary:
Ref T13053. Ref T12677. This restructures the calls and error handling logic so that we can pass in a list of multiple mailers and get retry logic.

This doesn't actually ever use multiple mailers yet, and shouldn't change any behavior. I'll add multiple-mailer coverage a little further in, since there's currently no way to effectively test which of several mailers ended up transmitting a message.

Test Plan:
  - This has test coverage; tests still pass.
  - Poked around locally doing things that send mail, saw mail appear to send. I'm not attached to a real mailer though so my confidence in local testing is only so-so.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13053, T12677

Differential Revision: https://secure.phabricator.com/D18998
This commit is contained in:
epriestley
2018-02-05 14:22:54 -08:00
parent 150a04791c
commit 1485debcbd
2 changed files with 367 additions and 334 deletions
Download Patch File Download Diff File Expand all files Collapse all files

143
src/applications/metamta/storage/PhabricatorMetaMTAMail.php
View File

@@ -461,28 +461,95 @@ final class PhabricatorMetaMTAMail
/**
* Attempt to deliver an email immediately, in this process.
*
* @param bool Try to deliver this email even if it has already been
* delivered or is in backoff after a failed delivery attempt.
* @param PhabricatorMailImplementationAdapter Use a specific mail adapter,
* instead of the default.
*
* @return void
*/
public function sendNow(
$force_send = false,
PhabricatorMailImplementationAdapter $mailer = null) {
if ($mailer === null) {
$mailer = $this->buildDefaultMailer();
}
if (!$force_send) {
public function sendNow() {
if ($this->getStatus() != PhabricatorMailOutboundStatus::STATUS_QUEUE) {
throw new Exception(pht('Trying to send an already-sent mail!'));
}
$mailers = array(
$this->buildDefaultMailer(),
);
return $this->sendWithMailers($mailers);
}
public function sendWithMailers(array $mailers) {
$exceptions = array();
foreach ($mailers as $template_mailer) {
$mailer = null;
try {
$mailer = $this->buildMailer($template_mailer);
} catch (Exception $ex) {
$exceptions[] = $ex;
continue;
}
if (!$mailer) {
// If we don't get a mailer back, that means the mail doesn't
// actually need to be sent (for example, because recipients have
// declined to receive the mail). Void it and return.
return $this
->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID)
->save();
}
try {
$ok = $mailer->send();
if (!$ok) {
// TODO: At some point, we should clean this up and make all mailers
// throw.
throw new Exception(
pht(
'Mail adapter encountered an unexpected, unspecified '.
'failure.'));
}
} catch (PhabricatorMetaMTAPermanentFailureException $ex) {
// If any mailer raises a permanent failure, stop trying to send the
// mail with other mailers.
$this
->setStatus(PhabricatorMailOutboundStatus::STATUS_FAIL)
->setMessage($ex->getMessage())
->save();
throw $ex;
} catch (Exception $ex) {
$exceptions[] = $ex;
continue;
}
return $this
->setStatus(PhabricatorMailOutboundStatus::STATUS_SENT)
->save();
}
// If we make it here, no mailer could send the mail but no mailer failed
// permanently either. We update the error message for the mail, but leave
// it in the current status (usually, STATUS_QUEUE) and try again later.
$messages = array();
foreach ($exceptions as $ex) {
$messages[] = $ex->getMessage();
}
$messages = implode("\n\n", $messages);
$this
->setMessage($messages)
->save();
if (count($exceptions) === 1) {
throw head($exceptions);
}
throw new PhutilAggregateException(
pht('Encountered multiple exceptions while transmitting mail.'),
$exceptions);
}
private function buildMailer(PhabricatorMailImplementationAdapter $mailer) {
$headers = $this->generateHeaders();
$params = $this->parameters;
@@ -726,35 +793,35 @@ final class PhabricatorMetaMTAMail
$this->setParam('routingmap.sent', $this->getRoutingRuleMap());
if (!$add_to && !$add_cc) {
$this->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID);
$this->setMessage(
pht(
'Message has no valid recipients: all To/Cc are disabled, '.
'invalid, or configured not to receive this mail.'));
return $this->save();
return null;
}
if ($this->getIsErrorEmail()) {
$all_recipients = array_merge($add_to, $add_cc);
if ($this->shouldRateLimitMail($all_recipients)) {
$this->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID);
$this->setMessage(
pht(
'This is an error email, but one or more recipients have '.
'exceeded the error email rate limit. Declining to deliver '.
'message.'));
return $this->save();
return null;
}
}
if (PhabricatorEnv::getEnvConfig('phabricator.silent')) {
$this->setStatus(PhabricatorMailOutboundStatus::STATUS_VOID);
$this->setMessage(
pht(
'Phabricator is running in silent mode. See `%s` '.
'in the configuration to change this setting.',
'phabricator.silent'));
return $this->save();
return null;
}
// Some mailers require a valid "To:" in order to deliver mail. If we
@@ -778,42 +845,8 @@ final class PhabricatorMetaMTAMail
if ($add_cc) {
$mailer->addCCs($add_cc);
}
} catch (Exception $ex) {
$this
->setStatus(PhabricatorMailOutboundStatus::STATUS_FAIL)
->setMessage($ex->getMessage())
->save();
throw $ex;
}
try {
$ok = $mailer->send();
if (!$ok) {
// TODO: At some point, we should clean this up and make all mailers
// throw.
throw new Exception(
pht('Mail adapter encountered an unexpected, unspecified failure.'));
}
$this->setStatus(PhabricatorMailOutboundStatus::STATUS_SENT);
$this->save();
return $this;
} catch (PhabricatorMetaMTAPermanentFailureException $ex) {
$this
->setStatus(PhabricatorMailOutboundStatus::STATUS_FAIL)
->setMessage($ex->getMessage())
->save();
throw $ex;
} catch (Exception $ex) {
$this
->setMessage($ex->getMessage()."\n".$ex->getTraceAsString())
->save();
throw $ex;
}
return $mailer;
}
private function generateThreadIndex($seed, $is_first_mail) {

8
src/applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php
View File

@@ -18,7 +18,7 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
$mail->addTos(array($phid));
$mailer = new PhabricatorMailImplementationTestAdapter();
$mail->sendNow($force = true, $mailer);
$mail->sendWithMailers(array($mailer));
$this->assertEqual(
PhabricatorMailOutboundStatus::STATUS_SENT,
$mail->getStatus());
@@ -31,7 +31,7 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
$mailer = new PhabricatorMailImplementationTestAdapter();
$mailer->setFailTemporarily(true);
try {
$mail->sendNow($force = true, $mailer);
$mail->sendWithMailers(array($mailer));
} catch (Exception $ex) {
// Ignore.
}
@@ -47,7 +47,7 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
$mailer = new PhabricatorMailImplementationTestAdapter();
$mailer->setFailPermanently(true);
try {
$mail->sendNow($force = true, $mailer);
$mail->sendWithMailers(array($mailer));
} catch (Exception $ex) {
// Ignore.
}
@@ -191,7 +191,7 @@ final class PhabricatorMetaMTAMailTestCase extends PhabricatorTestCase {
$mail = new PhabricatorMetaMTAMail();
$mail->setThreadID($thread_id, $is_first_mail);
$mail->sendNow($force = true, $mailer);
$mail->sendWithMailers(array($mailer));
$guts = $mailer->getGuts();
$dict = ipull($guts['headers'], 1, 0);