django-dbconn-retry icon indicating copy to clipboard operation
django-dbconn-retry copied to clipboard

TransactionManagementError: The rollback flag doesn't work outside of an 'atomic' block.

Open blueyed opened this issue 6 years ago • 4 comments

Using this app causes an error during teardown in one of our tests.

The test is triggering warnings like the following, which are likely causing the atomic block to be left:

2018-03-06 20:18:13,527:WARNING::drf_yasg.inspectors.base: 'default' on schema for HiddenField(default=CurrentUserDefault()) will not be set because to_representation raised an exception
Traceback (most recent call last):
  File "…/project/.venv/lib/python3.6/site-packages/drf_yasg/inspectors/base.py", line 223, in SwaggerType
    default = field.to_representation(default)
  File "…/Vcs/django-rest-framework/rest_framework/fields.py", line 573, in to_representation
    field_name=self.field_name,
NotImplementedError: HiddenField.to_representation() must be implemented for field own_field. If you do not need to support write operations you probably want to subclass `ReadOnlyField` instead.

The error during teardown (using pytest and pytest-django):

ERROR project/app/tests/test_api_docs.py::test_docs_media_types_and_index                                                                                                                                                
                                                                                                                                      
_____ ERROR at teardown of test_docs_media_types_and_index _____
../../../Vcs/django/django/test/testcases.py:893: in _post_teardown
    self._fixture_teardown()
../../../Vcs/django/django/test/testcases.py:1043: in _fixture_teardown
    self._rollback_atomics(self.atomics)
../../../Vcs/django/django/test/testcases.py:984: in _rollback_atomics
    transaction.set_rollback(True, using=db_name)
../../../Vcs/django/django/db/transaction.py:92: in set_rollback
    return get_connection(using).set_rollback(rollback)
../../../Vcs/django/django/db/backends/base/base.py:425: in set_rollback
    "The rollback flag doesn't work outside of an 'atomic' block.")
E   django.db.transaction.TransactionManagementError: The rollback flag doesn't work outside of an 'atomic' block.

blueyed avatar Mar 06 '18 20:03 blueyed

I have looked into this for a little bit and I have a theory why this happens, but no real solution right now.

I can reproduce this error by commenting out the overridden tearDownClass() in django_dbconn_retry.tests.ReconnectTests.

This will produce

======================================================================
ERROR: tearDownClass (django_dbconn_retry.tests.ReconnectTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/django-dbconn-retry/.env/lib/python3.5/site-packages/django/test/testcases.py", line 1050, in tearDownClass
    cls._rollback_atomics(cls.cls_atomics)
  File "/home/vagrant/django-dbconn-retry/.env/lib/python3.5/site-packages/django/test/testcases.py", line 1020, in _rollback_atomics
    transaction.set_rollback(True, using=db_name)
  File "/home/vagrant/django-dbconn-retry/.env/lib/python3.5/site-packages/django/db/transaction.py", line 103, in set_rollback
    return get_connection(using).set_rollback(rollback)
  File "/home/vagrant/django-dbconn-retry/.env/lib/python3.5/site-packages/django/db/backends/base/base.py", line 434, in set_rollback
    "The rollback flag doesn't work outside of an 'atomic' block.")
django.db.transaction.TransactionManagementError: The rollback flag doesn't work outside of an 'atomic' block.

----------------------------------------------------------------------

The commented out tearDownClass() previously had a comment that points to the root cause: Django's TestCase tries to rollback changes to the testing database between test. I think somewhere in-between the connection gets marked as unusable. When any code path then accesses a connection, ensure_connection will be called, but the monkey-patched version will reinitialize the connection. This will lead to a valid connection without an atomic block having been entered. Removing that behavior by overriding tearDownClass is not a problem for django-dbconn-retry's tests as those don't write to the database (yet), but obviously it breaks other tests that deliberately fail, like yours and you'll need changes to be rolled back.

I believe Django's TestCase wraps each test into an atomic block and I assume that this also points to django-dbconn-retry being broken with ATOMIC_REQUESTS=True.

Long story short: django-dbconn-retry will need to learn how to handle atomic blocks correctly and I need to study the semantics more. I mean, this project is marked as version 0.1 for a reason ;).

jdelic avatar Mar 08 '18 11:03 jdelic

I encountered the same problem

ZhuoZhuoCrayon avatar Oct 29 '21 03:10 ZhuoZhuoCrayon

Hi, I'm having the same issue. Is there any solution? Also, could it be a problem in production? Assuming our ATOMIC_REQUESTS flag is False.

moranReh avatar Nov 04 '21 08:11 moranReh

@pytest.mark.django_db(transaction=True) for pytest-django gets rid of the tear down error for me, I assume using TransactionTestCase would too

but it'd be good to get to the bottom of it

anentropic avatar Dec 01 '23 15:12 anentropic