[Checkins] SVN: zope.sendmail/trunk/src/zope/sendmail/ Fix for
https://bugs.launchpad.net/zope3/+bug/157104: move aside mail
Albertas Agejevas
alga at pov.lt
Wed Dec 19 10:47:43 EST 2007
Log message for revision 82358:
Fix for https://bugs.launchpad.net/zope3/+bug/157104: move aside mail
that's causing 5xx server responses.
Changed:
U zope.sendmail/trunk/src/zope/sendmail/README.txt
U zope.sendmail/trunk/src/zope/sendmail/delivery.py
U zope.sendmail/trunk/src/zope/sendmail/tests/test_delivery.py
-=-
Modified: zope.sendmail/trunk/src/zope/sendmail/README.txt
===================================================================
--- zope.sendmail/trunk/src/zope/sendmail/README.txt 2007-12-19 13:09:28 UTC (rev 82357)
+++ zope.sendmail/trunk/src/zope/sendmail/README.txt 2007-12-19 15:47:43 UTC (rev 82358)
@@ -123,9 +123,5 @@
* The configuration should be done in zope.conf, not in ZCML.
-* If the SMTP server rejects a message (for example, when the sender or
- recipient address is malformed), that email stays in the queue forever
- (https://bugs.launchpad.net/zope3/+bug/157104).
-
* The IMailSentEvent and IMailErrorEvent events aren't used and can't be used
(you don't want to send emails during the commit phase).
Modified: zope.sendmail/trunk/src/zope/sendmail/delivery.py
===================================================================
--- zope.sendmail/trunk/src/zope/sendmail/delivery.py 2007-12-19 13:09:28 UTC (rev 82357)
+++ zope.sendmail/trunk/src/zope/sendmail/delivery.py 2007-12-19 15:47:43 UTC (rev 82358)
@@ -24,6 +24,7 @@
import os
import os.path
import rfc822
+import smtplib
import stat
import threading
import time
@@ -263,13 +264,14 @@
toaddrs = ()
head, tail = os.path.split(filename)
tmp_filename = os.path.join(head, '.sending-' + tail)
+ rejected_filename = os.path.join(head, '.rejected-' + tail)
try:
# perform a series of operations in an attempt to ensure
# that no two threads/processes send this message
# simultaneously as well as attempting to not generate
# spurious failure messages in the log; a diagram that
- # represents these operations is included in
- # send-mail-states.txt
+ # represents these operations is included in a
+ # comment above this class
try:
# find the age of the tmp file (if it exists)
age = None
@@ -336,7 +338,19 @@
message = file.read()
file.close()
fromaddr, toaddrs, message = self._parseMessage(message)
- self.mailer.send(fromaddr, toaddrs, message)
+ try:
+ self.mailer.send(fromaddr, toaddrs, message)
+ except smtplib.SMTPResponseException, e:
+ if 500 <= e.smtp_code <= 599:
+ # permanent error, ditch the message
+ self.log.error(
+ "Discarding email from %s to %s due to"
+ " a permanent error: %s",
+ fromaddr, ", ".join(toaddrs), str(e))
+ os.link(filename, rejected_filename)
+ else:
+ # Log an error and retry later
+ raise
try:
os.unlink(filename)
Modified: zope.sendmail/trunk/src/zope/sendmail/tests/test_delivery.py
===================================================================
--- zope.sendmail/trunk/src/zope/sendmail/tests/test_delivery.py 2007-12-19 13:09:28 UTC (rev 82357)
+++ zope.sendmail/trunk/src/zope/sendmail/tests/test_delivery.py 2007-12-19 15:47:43 UTC (rev 82358)
@@ -19,7 +19,9 @@
"""
import os.path
-from tempfile import mktemp
+import shutil
+import smtplib
+from tempfile import mkdtemp
from unittest import TestCase, TestSuite, makeSuite
import transaction
@@ -193,6 +195,7 @@
self.msgs.append(m)
return m
+
class LoggerStub(object):
def __init__(self):
@@ -208,9 +211,11 @@
def info(self, msg, *args, **kwargs):
self.infos.append((msg, args, kwargs))
+
class BizzarreMailError(IOError):
pass
+
class BrokenMailerStub(object):
implements(IMailer)
@@ -220,6 +225,17 @@
def send(self, fromaddr, toaddrs, message):
raise BizzarreMailError("bad things happened while sending mail")
+
+class SMTPResponseExceptionMailerStub(object):
+
+ implements(IMailer)
+ def __init__(self, code):
+ self.code = code
+
+ def send(self, fromaddr, toaddrs, message):
+ raise smtplib.SMTPResponseException(self.code, 'Serious Error')
+
+
class TestQueuedMailDelivery(TestCase):
def setUp(self):
@@ -298,7 +314,11 @@
self.mailer = MailerStub()
self.thread.setMailer(self.mailer)
self.thread.log = LoggerStub()
+ self.dir = mkdtemp()
+ def tearDown(self):
+ shutil.rmtree(self.dir)
+
def test_parseMessage(self):
hdr = ('X-Zope-From: foo at example.com\n'
'X-Zope-To: bar at example.com, baz at example.com\n')
@@ -311,7 +331,7 @@
self.assertEquals(m, msg)
def test_deliveration(self):
- self.filename = mktemp()
+ self.filename = os.path.join(self.dir, 'message')
temp = open(self.filename, "w+b")
temp.write('X-Zope-From: foo at example.com\n'
'X-Zope-To: bar at example.com, baz at example.com\n'
@@ -332,7 +352,7 @@
def test_error_logging(self):
self.thread.setMailer(BrokenMailerStub())
- self.filename = mktemp()
+ self.filename = os.path.join(self.dir, 'message')
temp = open(self.filename, "w+b")
temp.write('X-Zope-From: foo at example.com\n'
'X-Zope-To: bar at example.com, baz at example.com\n'
@@ -346,7 +366,50 @@
'bar at example.com, baz at example.com'),
{'exc_info': 1})])
+ def test_smtp_response_error_transient(self):
+ # Test a transient error
+ self.thread.setMailer(SMTPResponseExceptionMailerStub(451))
+ self.filename = os.path.join(self.dir, 'message')
+ temp = open(self.filename, "w+b")
+ temp.write('X-Zope-From: foo at example.com\n'
+ 'X-Zope-To: bar at example.com, baz at example.com\n'
+ 'Header: value\n\nBody\n')
+ temp.close()
+ self.md.files.append(self.filename)
+ self.thread.run(forever=False)
+ # File must remail were it was, so it will be retried
+ self.failUnless(os.path.exists(self.filename))
+ self.assertEquals(self.thread.log.errors,
+ [('Error while sending mail from %s to %s.',
+ ('foo at example.com',
+ 'bar at example.com, baz at example.com'),
+ {'exc_info': 1})])
+
+ def test_smtp_response_error_permanent(self):
+ # Test a permanent error
+ self.thread.setMailer(SMTPResponseExceptionMailerStub(550))
+ self.filename = os.path.join(self.dir, 'message')
+ temp = open(self.filename, "w+b")
+ temp.write('X-Zope-From: foo at example.com\n'
+ 'X-Zope-To: bar at example.com, baz at example.com\n'
+ 'Header: value\n\nBody\n')
+ temp.close()
+ self.md.files.append(self.filename)
+ self.thread.run(forever=False)
+
+ # File must be moved aside
+ self.failIf(os.path.exists(self.filename))
+ self.failUnless(os.path.exists(os.path.join(self.dir,
+ '.rejected-message')))
+ self.assertEquals(self.thread.log.errors,
+ [('Discarding email from %s to %s due to a '
+ 'permanent error: %s',
+ ('foo at example.com',
+ 'bar at example.com, baz at example.com',
+ "(550, 'Serious Error')"), {})])
+
+
def test_suite():
return TestSuite((
makeSuite(TestMailDataManager),
More information about the Checkins
mailing list