Skip to content

Commit

Permalink
Add a vote method the the mailer interface and implement it for SMTPM…
Browse files Browse the repository at this point in the history
…ailer. Attempts to create the connection and HELO while in transaction vote rather than finish, so exceptions in making the connection will abort the current transaction safely.

The vote method needs to be provided to the MailDataManager, but if a Mailer implementation doesn't provide one DirectMailDelivery will provide a noop replacement and make a deprecation warning.

Bump version to 3.8 branch
  • Loading branch information
MatthewWilkes committed Jun 29, 2010
1 parent e66ef1b commit 185a7cc
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 12 deletions.
6 changes: 5 additions & 1 deletion CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@
CHANGES
=======

3.7.3 (unreleased)
3.8.0 (unreleased)
------------------

- Add a vote method to Mailer implementations to allow them to abort a
transaction if it is known to be unsafe.

- Prevent fatal errors in mail delivery causing potential database corruption.

3.7.2 (2010-04-30)
------------------
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@


setup(name='zope.sendmail',
version = '3.7.3dev',
version = '3.8.0dev',
url='http://pypi.python.org/pypi/zope.sendmail',
license='ZPL 2.1',
description='Zope sendmail',
Expand Down
33 changes: 29 additions & 4 deletions src/zope/sendmail/delivery.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import os
import rfc822
import logging
import warnings
from cStringIO import StringIO
from random import randrange
from time import strftime
Expand All @@ -34,12 +36,15 @@
# zope.sendmail which defined QueueProcessorThread in this module
from zope.sendmail.queue import QueueProcessorThread

log = logging.getLogger("MailDataManager")

class MailDataManager(object):
implements(IDataManager)

def __init__(self, callable, args=(), onAbort=None):
def __init__(self, callable, args=(), vote=None, onAbort=None):
self.callable = callable
self.args = args
self.vote = vote
self.onAbort = onAbort
# Use the default thread transaction manager.
self.transaction_manager = transaction.manager
Expand Down Expand Up @@ -69,10 +74,18 @@ def tpc_begin(self, transaction, subtransaction=False):
assert not subtransaction

def tpc_vote(self, transaction):
pass
if self.vote is not None:
return self.vote(*self.args)

This comment has been minimized.

Copy link
@lrowe

lrowe Mar 4, 2013

Sending mail during tpc_vote is the right thing to do, but you need to ensure that the MailDataManager sortKey() sorts last or mail may be sent before another data manager raises an exception in tpc_vote and the transaction is retried. See the 1PC variant of the zope.sqlalchemy DataManager: https://github.com/zopefoundation/zope.sqlalchemy/blob/2c5d7a844831ba9ad00cfb39f4d4016bc81c580c/src/zope/sqlalchemy/datamanager.py#L109

This comment has been minimized.

Copy link
@mgedmin

mgedmin Mar 4, 2013

Member

A line note on a 3-year-old commit is perhaps not the best place for this. Would you care to open an issue (or provide a pull request)?


def tpc_finish(self, transaction):
self.callable(*self.args)
try:
self.callable(*self.args)
except Exception, e:
# Any exceptions here can cause database corruption.
# Better to protect the data and potentially miss emails than
# leave a database in an inconsistent state which requires a
# guru to fix.
log.exception(e)

tpc_abort = abort

Expand Down Expand Up @@ -111,8 +124,20 @@ def __init__(self, mailer):
self.mailer = mailer

def createDataManager(self, fromaddr, toaddrs, message):
try:
vote = self.mailer.vote
except AttributeError:
# We've got an old mailer, just pass through voting
warnings.warn("The mailer %s does not provide a vote method"
% (repr(self.mailer)), DeprecationWarning)

def vote(*args, **kwargs):
pass

return MailDataManager(self.mailer.send,
args=(fromaddr, toaddrs, message))
args=(fromaddr, toaddrs, message),
vote=vote,
onAbort=self.mailer.abort)


class QueuedMailDelivery(AbstractMailDelivery):
Expand Down
9 changes: 8 additions & 1 deletion src/zope/sendmail/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,18 @@ def send(fromaddr, toaddrs, message):
2822. It should contain at least Date, From, To, and Message-Id
headers.
Messages are sent immediatelly.
Messages are sent immediately.
Dispatches an `IMailSentEvent` on successful delivery, otherwise an
`IMailErrorEvent`.
"""

def abort():
"""Abort sending the message for asynchronous subclasses."""

def vote(fromaddr, toaddrs, message):
"""Raise an exception if there is a known reason why the message
cannot be sent."""


class ISMTPMailer(IMailer):
Expand Down
31 changes: 26 additions & 5 deletions src/zope/sendmail/mailer.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,38 @@ def __init__(self, hostname='localhost', port=25,
self.password = password
self.force_tls = force_tls
self.no_tls = no_tls
self.connection = None

def send(self, fromaddr, toaddrs, message):
connection = self.smtp(self.hostname, str(self.port))
def vote(self, fromaddr, toaddrs, message):
self.connection = self.smtp(self.hostname, str(self.port))

This comment has been minimized.

Copy link
@mgedmin

mgedmin Mar 3, 2015

Member

Looks like this change broke queued email delivery, which reuses the same SMTP mailer object for multiple deliveries.

This comment has been minimized.

Copy link
@mgedmin

mgedmin Mar 3, 2015

Member

Oops, forgot to link to the bug: #1

This comment has been minimized.

Copy link
@MatthewWilkes

MatthewWilkes Mar 3, 2015

Author Member

Oh, fun. Darn, that could be tricky, if a lot of emails are going to be sent() then we potentially need to check them all in vote()


# send EHLO
code, response = connection.ehlo()
code, response = self.connection.ehlo()
if code < 200 or code >= 300:
code, response = connection.helo()
code, response = self.connection.helo()
if code < 200 or code >= 300:
raise RuntimeError('Error sending HELO to the SMTP server '
'(code=%s, response=%s)' % (code, response))

self.code, self.response = code, response


def abort(self):
if self.connection is None:
return

try:
self.connection.quit()
except socket.sslerror:
#something weird happened while quiting
self.connection.close()

def send(self, fromaddr, toaddrs, message):
connection = getattr(self, 'connection', None)
if connection is None:
self.vote(fromaddr, toaddrs, message)

connection, code, response = self.connection, self.code, self.response


# encryption support
have_tls = connection.has_extn('starttls')
Expand Down

0 comments on commit 185a7cc

Please sign in to comment.