Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delivery DataManager fails when transaction.savepoint is called #35

Closed
mauritsvanrees opened this issue Dec 17, 2020 · 2 comments
Closed
Labels

Comments

@mauritsvanrees
Copy link
Member

mauritsvanrees commented Dec 17, 2020

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:

  • Plone 5.2.2 site, Python 3.
  • Add a content rule that triggers when a document changes.
  • Give this a mailer action.
  • The default Plone mailer action seems to call send with immediate=True, which does not trigger any error. So either hack this to use immediate=False or write your own mailer action.
  • Activate this rule on a document.
  • Go to the history tab, which is backed by CMFEditions.
  • Choose to revert to a previous version of this document.

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.

mauritsvanrees added a commit that referenced this issue Dec 17, 2020
@mgedmin mgedmin added the bug label Dec 18, 2020
mauritsvanrees added a commit that referenced this issue Dec 22, 2020
…a savepoint.

Fixes issue #35.

Note that we do not need to do anything special.
There are two cases.

1. Mail sent before savepoint.
We send a mail, but delivery is delayed until the end of the transaction.
A savepoint is made.
Other changes are made.
A rollback is done to the savepoint.
The rollback method of our datamanager is called, but it does not need to do anything to restore the correct situation:
the mail is rightfully still scheduled to be mailed at the end of the transaction, if it is committed.

2. Mail sent after savepoint.
A savepoint is made.
We send a mail, but delivery is delayed until the end of the transaction.
A rollback is done to the savepoint.
Our datamanager part of the transaction is automatically discarded.
@icemac
Copy link
Member

icemac commented Jan 18, 2021

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

@mauritsvanrees
Copy link
Member Author

Right. I close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants