Merge pull request #85 from zagy/main

Fix retrying transaction when a datamanger marks an exception as retryable

CHANGES.rst

@@ -10,6 +10,9 @@ Unreleased (2024-11-14)
 
 - rename "master" to "main"
 
+- Fix retrying transactions with `pyramid_retry` when using veto and a datamanger
+  marks the exception as retryable.
+
 2.5 (2022-03-12)
 ^^^^^^^^^^^^^^^^
 

CONTRIBUTORS.txt

@@ -112,3 +112,4 @@ Contributors
 - Sean Hammond, 2019/09/27
 - Jon Betts, 2021/04/19
 - Jonathan Vanasco, 2022/11/14
+- Christian Zagrodnick, 2024/11/04

src/pyramid_tm/__init__.py

@@ -149,13 +149,14 @@ def tm_tween(request):
             if commit_veto is not None:
                 if commit_veto(request, response):
                     raise AbortWithResponse(response)
-            else:
-                # check for a squashed exception and handle it
-                # this would happen if an exception view was invoked and
-                # rendered an error response
-                exc_info = getattr(request, 'exc_info', None)
-                if exc_info is not None:
-                    maybe_tag_retryable(request, exc_info)
+
+            # check for a squashed exception and handle it
+            # this would happen if an exception view was invoked and
+            # rendered an error response
+            exc_info = getattr(request, 'exc_info', None)
+            if exc_info is not None:
+                maybe_tag_retryable(request, exc_info)
+                if commit_veto is None:
                     raise AbortWithResponse(response)
 
             return _finish(request, manager.commit, response)

tests/test_it.py

@@ -506,6 +506,50 @@ def view(request):
         self.assertEqual(calls, ['fail', 'ok'])
         self.assertEqual(result.body, b'ok')
 
+    @skip_if_missing('pyramid_retry')
+    def test_error_is_retried_with_commit_veto_and_error_view_and_retry_data_manager(
+        self,
+    ):
+        class Conflict(Exception):
+            """This is not a Transient error."""
+
+        class RetryDataManager(DummyDataManager):
+            """The datamanager wants the Conflict to be retried."""
+
+            def should_retry(self, exception):
+                return isinstance(exception, Conflict)
+
+        def exc_view(request):
+            return 'failure'
+
+        config = self.config
+        config.add_settings(
+            {
+                'retry.attempts': 2,
+                'tm.commit_veto': lambda request, response: False,
+            }
+        )
+        config.include('pyramid_retry')
+        config.add_view(exc_view, context=Exception, renderer='string')
+
+        calls = []
+
+        def view(request):
+            print(calls)
+            dm = RetryDataManager()
+            dm.bind(request.tm)
+            if len(calls) < 1:
+                calls.append('fail')
+                raise Conflict
+            calls.append('ok')
+            return 'ok'
+
+        config.add_view(view, renderer='string')
+        app = self._makeApp()
+        result = app.get('/')
+        self.assertEqual(calls, ['fail', 'ok'])
+        self.assertEqual(result.body, b'ok')
+
     def test_unhandled_error_aborts(self):
         config = self.config
         dm = DummyDataManager()