zope.sendmail and critical transaction errors.
Laurence Rowe <[email protected]>
| Newsgroups | gmane.comp.web.zope.devel |
|---|---|
| Message-ID | <[email protected]> |
With Zope2's MailHost now using zope.sendmail, we're seeing some critical errors when sending mail when the mail server domain name is misconfigured. http://dev.plone.org/plone/ticket/10675 (these are triggered by a password reset mail, the registration mail is sent immediately). This is because zope.sendmail.delivery.MailDataManager sends mail in tpc_finish when using DirectMailDelivery. While MailDataManager makes sense for QueuedMailDelivery (msg.commit should never fail) for DirectMailDelivery it seems wrong. To fix this, DirectMailDelivery should use a commit hook - there are two options: * After Commit Hook Ensures mail is only sent once in event of a request being retried, but errors are swallowed so no feedback that there is a problem to the browser. * Before Commit Hook Mail me be sent multiple times in event of a request being retried due to a conflict error, but errors propagate to the browser. I think the Before Commit Hook option is probably best here. DirectMailDelivery should only be used for testing anyway, or at least only on very small sites in production - QueuedMailDelivery will scale better. For Zope 2.12 / Plone 4.0 we have the additional problem that Zope 2.12 is incompatible with zope.sendmail 3.7.x / trunk due to a zope.component 3.8 dependency. I think this issue is serious enough to warrant backporting this fix to the zope.sendmail 3.6.x branch. Patches attached for comment. Laurence _______________________________________________ Zope-Dev maillist - [email protected] https://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - https://mail.zope.org/mailman/listinfo/zope-announce https://mail.zope.org/mailman/listinfo/zope )
aftercommit.diff
(application/octet-stream, 2.3 KB)
Index: CHANGES.txt
===================================================================
--- CHANGES.txt (revision 113766)
+++ CHANGES.txt (working copy)
@@ -5,6 +5,8 @@
3.7.3 (unreleased)
------------------
+- Use an after commit hook for direct mail delivery to avoid critical
+ transaction errors. http://dev.plone.org/plone/ticket/10675
3.7.2 (2010-04-30)
------------------
Index: src/zope/sendmail/delivery.py
===================================================================
--- src/zope/sendmail/delivery.py (revision 113766)
+++ src/zope/sendmail/delivery.py (working copy)
@@ -97,11 +97,17 @@
else:
messageid = self.newMessageId()
message = 'Message-Id: <%s>\n%s' % (messageid, message)
- transaction.get().join(
- self.createDataManager(fromaddr, toaddrs, message))
+ self.deliver(fromaddr, toaddrs, message)
return messageid
+def direct_mail_delivery_hook(status, mailer, fromaddr, toaddrs, message):
+ """After Commit Hook to send mail on successful commit.
+ """
+ if status:
+ mailer.send(fromaddr, toaddrs, message)
+
+
class DirectMailDelivery(AbstractMailDelivery):
__doc__ = IDirectMailDelivery.__doc__
@@ -110,9 +116,9 @@
def __init__(self, mailer):
self.mailer = mailer
- def createDataManager(self, fromaddr, toaddrs, message):
- return MailDataManager(self.mailer.send,
- args=(fromaddr, toaddrs, message))
+ def deliver(self, fromaddr, toaddrs, message):
+ transaction.get().addAfterCommitHook(direct_mail_delivery_hook,
+ args=(self.mailer, fromaddr, toaddrs, message))
class QueuedMailDelivery(AbstractMailDelivery):
@@ -125,11 +131,11 @@
queuePath = property(lambda self: self._queuePath)
- def createDataManager(self, fromaddr, toaddrs, message):
+ def deliver(self, fromaddr, toaddrs, message):
maildir = Maildir(self.queuePath, True)
msg = maildir.newMessage()
msg.write('X-Zope-From: %s\n' % fromaddr)
msg.write('X-Zope-To: %s\n' % ", ".join(toaddrs))
msg.write(message)
msg.close()
- return MailDataManager(msg.commit, onAbort=msg.abort)
+ transaction.get().join(MailDataManager(msg.commit, onAbort=msg.abort))
beforecommit.diff
(application/octet-stream, 2 KB)
Index: CHANGES.txt
===================================================================
--- CHANGES.txt (revision 113766)
+++ CHANGES.txt (working copy)
@@ -5,6 +5,8 @@
3.7.3 (unreleased)
------------------
+- Use a before commit hook for direct mail delivery to avoid critical
+ transaction errors. http://dev.plone.org/plone/ticket/10675
3.7.2 (2010-04-30)
------------------
Index: src/zope/sendmail/delivery.py
===================================================================
--- src/zope/sendmail/delivery.py (revision 113766)
+++ src/zope/sendmail/delivery.py (working copy)
@@ -97,8 +97,7 @@
else:
messageid = self.newMessageId()
message = 'Message-Id: <%s>\n%s' % (messageid, message)
- transaction.get().join(
- self.createDataManager(fromaddr, toaddrs, message))
+ self.deliver(fromaddr, toaddrs, message)
return messageid
@@ -110,9 +109,9 @@
def __init__(self, mailer):
self.mailer = mailer
- def createDataManager(self, fromaddr, toaddrs, message):
- return MailDataManager(self.mailer.send,
- args=(fromaddr, toaddrs, message))
+ def deliver(self, fromaddr, toaddrs, message):
+ transaction.get().addBeforeCommitHook(self.mailer.send,
+ args=(fromaddr, toaddrs, message))
class QueuedMailDelivery(AbstractMailDelivery):
@@ -125,11 +124,11 @@
queuePath = property(lambda self: self._queuePath)
- def createDataManager(self, fromaddr, toaddrs, message):
+ def deliver(self, fromaddr, toaddrs, message):
maildir = Maildir(self.queuePath, True)
msg = maildir.newMessage()
msg.write('X-Zope-From: %s\n' % fromaddr)
msg.write('X-Zope-To: %s\n' % ", ".join(toaddrs))
msg.write(message)
msg.close()
- return MailDataManager(msg.commit, onAbort=msg.abort)
+ transaction.get().join(MailDataManager(msg.commit, onAbort=msg.abort))