zopefoundation / zope.sendmail

Zope sendmail support.
Other
3 stars 6 forks source link

Delivery DataManager fails when transaction.savepoint is called #35

Closed mauritsvanrees closed 3 years ago

mauritsvanrees commented 3 years ago

BUG/PROBLEM REPORT (OR OTHER COMMON ISSUE)

When a mail is not immediately sent, it is sent at the end of the transaction. When some code calls transaction.savepoint, this fails with a traceback, because our MailDataManager has no savepoint.

What I did:

I have a PR ready with a fix, and a testcase that fails without the fix. Simplified from the test, here is some code that would fail:

mailer = MailerStub()
DirectMailDelivery(mailer)
delivery.send(fromaddr, toaddrs, opt_headers + message)
mailer.sent_messages == []
savepoint = transaction.savepoint()

My real use case is much harder to reproduce, and requires Plone knowledge:

What I expect to happen:

A mail is sent, because the content rule and mail action are triggered.

What actually happened:

I get a traceback, because CMFEditions needs to call transaction.savepoint in order to work:

Traceback (innermost last):

Module ZPublisher.WSGIPublisher, line 162, in transaction_pubevents
Module ZPublisher.WSGIPublisher, line 359, in publish_module
Module ZPublisher.WSGIPublisher, line 254, in publish
Module ZPublisher.mapply, line 85, in mapply
Module ZPublisher.WSGIPublisher, line 63, in call_object
Module Products.CMFCore.FSPythonScript, line 133, in __call__
Module Shared.DC.Scripts.Bindings, line 335, in __call__
Module Shared.DC.Scripts.Bindings, line 372, in _bindAndExec
Module Products.PythonScripts.PythonScript, line 351, in _exec
Module script, line 26, in revertversion
<FSPythonScript at /plone_portal/.../revertversion>
Line 26
Module Products.CMFEditions.CopyModifyMergeRepositoryTool, line 332, in save
Module ugent.patches.products_cmfeditions, line 9, in _recursiveSave
Module Products.CMFEditions.ArchivistTool, line 267, in prepare
Module Products.CMFEditions.ModifierRegistryTool, line 135, in getReferencedAttributes
Module plone.app.versioningbehavior.modifiers, line 117, in getReferencedAttributes
Module Products.CMFEditions.CopyModifyMergeRepositoryTool, line 410, in retrieve
Module Products.CMFEditions.CopyModifyMergeRepositoryTool, line 557, in _retrieve
Module transaction._manager, line 272, in savepoint
Module transaction._manager, line 151, in savepoint
Module transaction._transaction, line 227, in savepoint
Module transaction._transaction, line 316, in _saveAndRaiseCommitishError
Module transaction._compat, line 45, in reraise
Module transaction._transaction, line 224, in savepoint
Module transaction._transaction, line 634, in __init__
TypeError: ('Savepoints unsupported', <zope.sendmail.delivery.MailDataManager object at 0x7fb19e8b3c10>)

Or in the testcase:

Error in test testSavepoint (zope.sendmail.tests.test_delivery.TestDirectMailDelivery)
Traceback (most recent call last):
  File "/Users/maurits/.pyenv/versions/2.7.17/lib/python2.7/unittest/case.py", line 329, in run
    testMethod()
  File "/Users/maurits/community/plone-coredev/py3/src/zope.sendmail/.tox/py27/lib/python2.7/site-packages/zope/sendmail/tests/test_delivery.py", line 286, in testSavepoint
    transaction.savepoint()
  File "/Users/maurits/community/plone-coredev/py3/src/zope.sendmail/.tox/py27/lib/python2.7/site-packages/transaction/_manager.py", line 272, in savepoint
    return self.manager.savepoint(optimistic)
  File "/Users/maurits/community/plone-coredev/py3/src/zope.sendmail/.tox/py27/lib/python2.7/site-packages/transaction/_manager.py", line 150, in savepoint
    return self.get().savepoint(optimistic)
  File "/Users/maurits/community/plone-coredev/py3/src/zope.sendmail/.tox/py27/lib/python2.7/site-packages/transaction/_transaction.py", line 228, in savepoint
    self._saveAndRaiseCommitishError()  # reraises!
  File "/Users/maurits/community/plone-coredev/py3/src/zope.sendmail/.tox/py27/lib/python2.7/site-packages/transaction/_transaction.py", line 316, in _saveAndRaiseCommitishError
    reraise(t, v, tb)
  File "/Users/maurits/community/plone-coredev/py3/src/zope.sendmail/.tox/py27/lib/python2.7/site-packages/transaction/_transaction.py", line 225, in savepoint
    savepoint = Savepoint(self, optimistic, *self._resources)
  File "/Users/maurits/community/plone-coredev/py3/src/zope.sendmail/.tox/py27/lib/python2.7/site-packages/transaction/_transaction.py", line 623, in __init__
    raise TypeError("Savepoints unsupported", datamanager)
TypeError: ('Savepoints unsupported', <zope.sendmail.delivery.MailDataManager object at 0x109044810>)

What version of Python and Zope/Addons I am using:

Plone 5.2.2, Zope 4.5.1, both Python 2 and 3, zope.sendmail 5.1.

As said, I have a branch ready with a fix. This adds a savepoint implementation that basically does nothing. I took the code from a comment on aplone.namedfile branch with a similar error.

Alternatively, I wonder if it might be possible to add proper support. At least I can imagine this scenario in one transaction:

  1. Do stuff
  2. Package X creates savepoint A
  3. Send mail A
  4. Unrelated stuff in package X goes wrong.
  5. Package X does a rollback to savepoint A and tries something else.
  6. Send mail B.
  7. Unrelated stuff now goes right.
  8. Transaction commit.

In this case, only mail B should be sent. I have not tried this scenario, but I guess currently both mail A and B would be sent.

But this seems much more difficult than the simple fix I have ready, so I will just push that branch.

icemac commented 3 years ago

@mauritsvanrees This issue seems to be fixed by #36, right?

mauritsvanrees commented 3 years ago

Right. I close.